diff --git a/desktop/src/components/shared/MultiSelectDropdown.test.tsx b/desktop/src/components/shared/MultiSelectDropdown.test.tsx new file mode 100644 index 00000000..443e4d9e --- /dev/null +++ b/desktop/src/components/shared/MultiSelectDropdown.test.tsx @@ -0,0 +1,81 @@ +import { describe, it, expect, vi } from 'vitest' +import { render, screen, fireEvent } from '@testing-library/react' +import '@testing-library/jest-dom' + +import { MultiSelectDropdown } from './MultiSelectDropdown' + +type Item = { value: string; label: string } + +function renderDropdown(props: { + values: string[] + items: Item[] + onToggle?: (v: string) => void +}) { + return render( + {})} + items={props.items} + trigger={} + />, + ) +} + +describe('MultiSelectDropdown', () => { + it('hides items until the trigger is clicked', () => { + renderDropdown({ + values: [], + items: [{ value: 'a', label: 'Alpha' }, { value: 'b', label: 'Beta' }], + }) + expect(screen.queryByText('Alpha')).not.toBeInTheDocument() + + fireEvent.click(screen.getByText('open')) + expect(screen.getByText('Alpha')).toBeInTheDocument() + expect(screen.getByText('Beta')).toBeInTheDocument() + }) + + it('exposes each row as menuitemcheckbox with correct aria-checked', () => { + renderDropdown({ + values: ['a'], + items: [{ value: 'a', label: 'Alpha' }, { value: 'b', label: 'Beta' }], + }) + fireEvent.click(screen.getByText('open')) + + const rows = screen.getAllByRole('menuitemcheckbox') + expect(rows).toHaveLength(2) + expect(rows[0]).toHaveAttribute('aria-checked', 'true') + expect(rows[1]).toHaveAttribute('aria-checked', 'false') + }) + + it('keeps the popover open after toggling so multiple items can be chosen', () => { + const onToggle = vi.fn() + renderDropdown({ + values: [], + items: [{ value: 'a', label: 'Alpha' }, { value: 'b', label: 'Beta' }], + onToggle, + }) + fireEvent.click(screen.getByText('open')) + + fireEvent.click(screen.getByText('Alpha')) + fireEvent.click(screen.getByText('Beta')) + + // Both fires AND the menu items are still on screen — popover never auto-closed. + expect(onToggle).toHaveBeenCalledTimes(2) + expect(onToggle).toHaveBeenNthCalledWith(1, 'a') + expect(onToggle).toHaveBeenNthCalledWith(2, 'b') + expect(screen.getByText('Alpha')).toBeInTheDocument() + expect(screen.getByText('Beta')).toBeInTheDocument() + }) + + it('closes when Escape is pressed', () => { + renderDropdown({ + values: [], + items: [{ value: 'a', label: 'Alpha' }], + }) + fireEvent.click(screen.getByText('open')) + expect(screen.getByText('Alpha')).toBeInTheDocument() + + fireEvent.keyDown(document, { key: 'Escape' }) + expect(screen.queryByText('Alpha')).not.toBeInTheDocument() + }) +}) diff --git a/desktop/src/components/shared/MultiSelectDropdown.tsx b/desktop/src/components/shared/MultiSelectDropdown.tsx new file mode 100644 index 00000000..045564f6 --- /dev/null +++ b/desktop/src/components/shared/MultiSelectDropdown.tsx @@ -0,0 +1,158 @@ +import { useState, useRef, useEffect, type CSSProperties, type ReactNode } from 'react' + +type MultiSelectDropdownItem = { + value: T + label: string + description?: string + icon?: ReactNode +} + +type MultiSelectDropdownProps = { + items: MultiSelectDropdownItem[] + /** Currently selected values. Items not in this array are rendered unchecked. */ + values: T[] + /** Toggle a single item. The popover stays open so multiple items can be selected without reopening. */ + onToggle: (value: T) => void + trigger: ReactNode + width?: CSSProperties['width'] + maxHeight?: CSSProperties['maxHeight'] + align?: 'left' | 'right' + className?: string + /** Optional bulk shortcuts shown above the items. */ + selectAllLabel?: string + clearLabel?: string + onSelectAll?: () => void + onClear?: () => void +} + +/** + * Multi-select cousin of Dropdown. Click outside / press Escape to close. + * Clicking an item toggles it without closing — bulk-selection without + * reopening the popover for each pick. The shape mirrors Dropdown so a + * caller can swap them without learning a different API surface. + */ +export function MultiSelectDropdown({ + items, + values, + onToggle, + trigger, + width = 320, + maxHeight, + align = 'left', + className = '', + selectAllLabel, + clearLabel, + onSelectAll, + onClear, +}: MultiSelectDropdownProps) { + const [open, setOpen] = useState(false) + const ref = useRef(null) + + useEffect(() => { + if (!open) return + const handleClick = (e: MouseEvent) => { + if (ref.current && !ref.current.contains(e.target as Node)) { + setOpen(false) + } + } + const handleEsc = (e: KeyboardEvent) => { + if (e.key === 'Escape') setOpen(false) + } + document.addEventListener('mousedown', handleClick) + document.addEventListener('keydown', handleEsc) + return () => { + document.removeEventListener('mousedown', handleClick) + document.removeEventListener('keydown', handleEsc) + } + }, [open]) + + const selectedSet = new Set(values) + const showHeader = (selectAllLabel && onSelectAll) || (clearLabel && onClear) + + return ( +
+
setOpen(!open)} className="cursor-pointer"> + {trigger} +
+ + {open && ( +
+ {showHeader && ( +
+ {selectAllLabel && onSelectAll && ( + + )} + {clearLabel && onClear && ( + + )} +
+ )} + + {items.map((item, i) => { + const checked = selectedSet.has(item.value) + return ( + + ) + })} +
+ )} +
+ ) +} diff --git a/desktop/src/components/skills/SkillDetail.test.tsx b/desktop/src/components/skills/SkillDetail.test.tsx index 5a3101bc..05e7b3c6 100644 --- a/desktop/src/components/skills/SkillDetail.test.tsx +++ b/desktop/src/components/skills/SkillDetail.test.tsx @@ -176,4 +176,29 @@ describe('SkillActivationScope', () => { // "Applies to:" label appears only under project scope with resolved projects. expect(await screen.findByText(/Applies to/i)).toBeInTheDocument() }) + + it('treats multiple projects as independently active and surfaces the count', async () => { + // Skill is active in 2 of 3 projects — the trigger should show "2 projects". + getActiveSkills.mockImplementation((scope: string, cwd?: string) => { + if (scope !== 'project') return Promise.resolve({ activeSkills: [] }) + if (cwd === '/work/alpha' || cwd === '/work/beta') { + return Promise.resolve({ activeSkills: ['multi-skill'] }) + } + return Promise.resolve({ activeSkills: [] }) + }) + apiGet.mockResolvedValue({ + projects: [ + { id: 'p1', label: '/work/alpha', projectPath: '/work/alpha', isCurrent: true }, + { id: 'p2', label: '/work/beta', projectPath: '/work/beta', isCurrent: false }, + { id: 'p3', label: '/work/gamma', projectPath: '/work/gamma', isCurrent: false }, + ], + }) + selectSkill('multi-skill') + render() + + // Trigger label uses the {count} key once 2+ projects are active. + expect(await screen.findByText(/2 projects/i)).toBeInTheDocument() + // The activated badge is still shown. + expect(screen.getByText(/Active/)).toBeInTheDocument() + }) }) diff --git a/desktop/src/components/skills/SkillDetail.tsx b/desktop/src/components/skills/SkillDetail.tsx index f0497fce..857e753d 100644 --- a/desktop/src/components/skills/SkillDetail.tsx +++ b/desktop/src/components/skills/SkillDetail.tsx @@ -8,7 +8,7 @@ import { useUIStore } from '../../stores/uiStore' import { skillsApi } from '../../api/skills' import { api } from '../../api/client' import { useSessionStore } from '../../stores/sessionStore' -import { Dropdown } from '../shared/Dropdown' +import { MultiSelectDropdown } from '../shared/MultiSelectDropdown' const META_PRIORITY = [ 'description', @@ -256,9 +256,11 @@ type ProjectChoice = { function SkillActivationScope({ skillName }: { skillName: string }) { const t = useTranslation() const [globalSkills, setGlobalSkills] = useState([]) - const [projectSkills, setProjectSkills] = useState([]) + // Set of project paths where this skill is currently active. Replaces the + // old single "selectedProjectPath" model: the skill can now be active in + // any number of projects independently. + const [activeProjects, setActiveProjects] = useState>(new Set()) const [projects, setProjects] = useState([]) - const [selectedProjectPath, setSelectedProjectPath] = useState(undefined) const [saving, setSaving] = useState(false) const sessions = useSessionStore((s) => s.sessions) const activeSessionId = useSessionStore((s) => s.activeSessionId) @@ -266,75 +268,155 @@ function SkillActivationScope({ skillName }: { skillName: string }) { const cwd = activeSession?.workDir || activeSession?.projectPath || undefined // Load the project list (reuse the project-rules endpoint) so the user can - // pick WHICH project the skill applies to under 'project' scope. + // pick WHICH projects the skill applies to under 'project' scope. useEffect(() => { const query = cwd ? `?cwd=${encodeURIComponent(cwd)}` : '' api.get<{ projects: ProjectChoice[] }>(`/api/project-rules${query}`) .then((res) => { - const resolved = res.projects.filter((p) => p.projectPath) - setProjects(resolved) - setSelectedProjectPath((prev) => { - if (prev && resolved.some((p) => p.projectPath === prev)) return prev - const current = resolved.find((p) => p.isCurrent) - return current?.projectPath ?? resolved[0]?.projectPath ?? undefined - }) + setProjects(res.projects.filter((p) => p.projectPath)) }) .catch(() => {}) }, [cwd]) - // Load activation state for global + the currently selected project. + // Global is a single list; load it once. useEffect(() => { skillsApi.getActiveSkills('global').then(res => setGlobalSkills(res.activeSkills)).catch(() => {}) }, []) + // Project state is per-project. Fan out one read per project and collect + // those whose activeSkills contains this skill into a Set. Re-run when the + // resolved project list changes. useEffect(() => { - skillsApi.getActiveSkills('project', selectedProjectPath).then(res => setProjectSkills(res.activeSkills)).catch(() => {}) - }, [selectedProjectPath]) + let cancelled = false + const projectPaths = projects.map(p => p.projectPath).filter((p): p is string => Boolean(p)) + if (projectPaths.length === 0) { + setActiveProjects(new Set()) + return + } + Promise.all( + projectPaths.map(async (path) => { + try { + const res = await skillsApi.getActiveSkills('project', path) + return { path, active: res.activeSkills.includes(skillName) } + } catch { + return { path, active: false } + } + }), + ).then((results) => { + if (cancelled) return + const next = new Set() + for (const { path, active } of results) { + if (active) next.add(path) + } + setActiveProjects(next) + }) + return () => { cancelled = true } + }, [projects, skillName]) + const isGlobal = globalSkills.includes(skillName) const currentScope: ActivationScope = - projectSkills.includes(skillName) ? 'project' - : globalSkills.includes(skillName) ? 'global' + activeProjects.size > 0 ? 'project' + : isGlobal ? 'global' : 'off' - const handleChange = async (scope: ActivationScope) => { + const writeProjectActivation = async (projectPath: string, shouldBeActive: boolean) => { + // Read-modify-write that single project's activeSkills, leaving every + // other project untouched. This keeps each toggle independent — a + // failure on one project does not corrupt the others. + const res = await skillsApi.getActiveSkills('project', projectPath) + const current = res.activeSkills + const has = current.includes(skillName) + if (shouldBeActive === has) return + const next = shouldBeActive + ? [...current, skillName] + : current.filter(s => s !== skillName) + await skillsApi.setActiveSkills(next, 'project', projectPath) + } + + // Bulk apply for the high-level Off / Global / Project buttons. + const handleScopeChange = async (scope: ActivationScope) => { setSaving(true) try { - const newGlobal = globalSkills.filter(s => s !== skillName) - const newProject = projectSkills.filter(s => s !== skillName) - - if (scope === 'global') { - newGlobal.push(skillName) - } else if (scope === 'project') { - newProject.push(skillName) + if (scope === 'off') { + // Clear from global + every active project. + const promises: Promise[] = [] + if (isGlobal) { + promises.push(skillsApi.setActiveSkills(globalSkills.filter(s => s !== skillName), 'global')) + } + for (const p of activeProjects) { + promises.push(writeProjectActivation(p, false)) + } + await Promise.all(promises) + setGlobalSkills(g => g.filter(s => s !== skillName)) + setActiveProjects(new Set()) + return } - await Promise.all([ - skillsApi.setActiveSkills(newGlobal, 'global'), - skillsApi.setActiveSkills(newProject, 'project', selectedProjectPath), - ]) - setGlobalSkills(newGlobal) - setProjectSkills(newProject) + if (scope === 'global') { + const promises: Promise[] = [] + if (!isGlobal) { + promises.push(skillsApi.setActiveSkills([...globalSkills, skillName], 'global')) + } + // Switching to global clears project-level entries to avoid double-injection. + for (const p of activeProjects) { + promises.push(writeProjectActivation(p, false)) + } + await Promise.all(promises) + setGlobalSkills(g => (g.includes(skillName) ? g : [...g, skillName])) + setActiveProjects(new Set()) + return + } + + // scope === 'project': default-activate the current project. If we're + // already in project mode, this button is a no-op and the user should + // use the multi-select dropdown instead. + if (activeProjects.size === 0) { + const defaultPath = + projects.find(p => p.isCurrent)?.projectPath ?? + projects[0]?.projectPath + if (!defaultPath) return + const promises: Promise[] = [] + if (isGlobal) { + promises.push(skillsApi.setActiveSkills(globalSkills.filter(s => s !== skillName), 'global')) + } + promises.push(writeProjectActivation(defaultPath, true)) + await Promise.all(promises) + setGlobalSkills(g => g.filter(s => s !== skillName)) + setActiveProjects(new Set([defaultPath])) + } } catch { - // ignore + // ignore — UI state stays in sync with what we last successfully wrote } finally { setSaving(false) } } - // When the user switches the project dropdown, re-evaluate whether the skill - // should be re-applied to the newly selected project (if scope was 'project'). - const handleProjectSwitch = async (projectPath: string) => { - const wasProjectScoped = currentScope === 'project' - setSelectedProjectPath(projectPath) - // Fetch the new project's active skills to reflect its status immediately. + // Per-project toggle from the multi-select dropdown. + const handleToggleProject = async (projectPath: string) => { + if (saving) return + const wantActive = !activeProjects.has(projectPath) + setSaving(true) try { - const res = await skillsApi.getActiveSkills('project', projectPath) - setProjectSkills(res.activeSkills) - // If the skill was project-scoped on the previous project, do NOT auto-move - // it — let the user explicitly click 'project' again for the new one. - void wasProjectScoped + // Activating any project also clears the global flag, since the two + // scopes are mutually exclusive in the high-level UI summary. + const promises: Promise[] = [writeProjectActivation(projectPath, wantActive)] + if (wantActive && isGlobal) { + promises.push(skillsApi.setActiveSkills(globalSkills.filter(s => s !== skillName), 'global')) + } + await Promise.all(promises) + setActiveProjects(prev => { + const next = new Set(prev) + if (wantActive) next.add(projectPath) + else next.delete(projectPath) + return next + }) + if (wantActive && isGlobal) { + setGlobalSkills(g => g.filter(s => s !== skillName)) + } } catch { // ignore + } finally { + setSaving(false) } } @@ -366,7 +448,7 @@ function SkillActivationScope({ skillName }: { skillName: string }) { return (