diff --git a/apps/desktop/src/app/chat/sidebar/index.tsx b/apps/desktop/src/app/chat/sidebar/index.tsx index 8add602e8c7f..5b3f2b4b30c2 100644 --- a/apps/desktop/src/app/chat/sidebar/index.tsx +++ b/apps/desktop/src/app/chat/sidebar/index.tsx @@ -21,7 +21,7 @@ import { SidebarMenuButton, SidebarMenuItem } from '@/components/ui/sidebar' -import { TipKeybindLabel } from '@/components/ui/tooltip' +import { Tip, TipKeybindLabel } from '@/components/ui/tooltip' import { useContributions } from '@/contrib/react/use-contributions' import { searchSessions, type SessionInfo, type SessionSearchResult } from '@/hermes' import { useI18n } from '@/i18n' @@ -1315,59 +1315,65 @@ export function ChatSidebar({ scoped />
- + + +
) : (
{!showAllProfiles ? ( - - ) : null} -
- {!showAllProfiles && agentSessions.length > 0 ? ( + + + ) : null} +
+ {!showAllProfiles && agentSessions.length > 0 ? ( + + + ) : null}
diff --git a/apps/desktop/src/app/chat/sidebar/load-more-row.test.tsx b/apps/desktop/src/app/chat/sidebar/load-more-row.test.tsx new file mode 100644 index 000000000000..02978ee2adf2 --- /dev/null +++ b/apps/desktop/src/app/chat/sidebar/load-more-row.test.tsx @@ -0,0 +1,54 @@ +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { SidebarLoadMoreRow } from './load-more-row' + +afterEach(cleanup) + +vi.mock('@/i18n', () => ({ + useI18n: () => ({ + t: { + sidebar: { + loadCount: (n: number) => `Load ${n} more`, + loadMore: 'Load more', + loading: 'Loading…' + } + } + }) +})) + +// The tooltip's open transition rides a real, un-act()-wrapped Radix timer +// that reliably never fires on the Linux CI runner (see dialog.test.tsx's +// skipped hover test) — so instead of hovering and waiting for the tip to +// open, we assert the structural fix directly: the button is now wrapped in +// a Tip (data-slot="tooltip-trigger"), which is what # was missing. +describe('SidebarLoadMoreRow', () => { + it('wraps the button in a Tip with the loading label as the trigger', () => { + render() + + const button = screen.getByRole('button', { name: 'Loading…' }) + expect(button.closest('[data-slot="tooltip-trigger"]')).toBeTruthy() + }) + + it('wraps the button in a Tip with the count label when a step is given', () => { + render() + + const button = screen.getByRole('button', { name: 'Load 5 more' }) + expect(button.closest('[data-slot="tooltip-trigger"]')).toBeTruthy() + }) + + it('wraps the button in a Tip with the generic label when step is 0', () => { + render() + + const button = screen.getByRole('button', { name: 'Load more' }) + expect(button.closest('[data-slot="tooltip-trigger"]')).toBeTruthy() + }) + + it('still fires onClick (Tip does not intercept the trigger interaction)', () => { + const onClick = vi.fn() + render() + + screen.getByRole('button', { name: 'Load more' }).click() + expect(onClick).toHaveBeenCalledOnce() + }) +}) diff --git a/apps/desktop/src/app/chat/sidebar/load-more-row.tsx b/apps/desktop/src/app/chat/sidebar/load-more-row.tsx index e0085fdb5879..617bad917265 100644 --- a/apps/desktop/src/app/chat/sidebar/load-more-row.tsx +++ b/apps/desktop/src/app/chat/sidebar/load-more-row.tsx @@ -1,5 +1,6 @@ import { Codicon } from '@/components/ui/codicon' import { GlyphSpinner } from '@/components/ui/glyph-spinner' +import { Tip } from '@/components/ui/tooltip' import { useI18n } from '@/i18n' interface SidebarLoadMoreRowProps { @@ -16,18 +17,20 @@ export function SidebarLoadMoreRow({ step, onClick, loading = false }: SidebarLo const label = loading ? t.sidebar.loading : step > 0 ? t.sidebar.loadCount(step) : t.sidebar.loadMore return ( - + + + ) } diff --git a/apps/desktop/src/app/chat/sidebar/project-dialog.test.tsx b/apps/desktop/src/app/chat/sidebar/project-dialog.test.tsx new file mode 100644 index 000000000000..17bc5dd9c7bb --- /dev/null +++ b/apps/desktop/src/app/chat/sidebar/project-dialog.test.tsx @@ -0,0 +1,87 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import type * as Nanostores from 'nanostores' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { ProjectDialog } from './project-dialog' + +afterEach(cleanup) + +vi.mock('@/i18n', () => ({ + useI18n: () => ({ + t: { + common: { cancel: 'Cancel', save: 'Save' }, + sidebar: { + projects: { + addFolder: 'Add folder', + create: 'Create', + createDesc: 'Create a new project', + createFailed: 'Failed to create project', + createTitle: 'New project', + foldersLabel: 'Folders', + ideaGenerate: 'Generate', + ideaGenerating: 'Generating…', + ideaLabel: 'Idea', + ideaPlaceholder: 'What are you building?', + ideaShuffle: 'Shuffle ideas', + namePlaceholder: 'Project name', + noFolders: 'No folders yet', + primaryBadge: 'Primary', + removeFolder: 'Remove folder' + } + } + } + }) +})) + +// $projectDialog is a real nanostore atom in the app; recreate it here so +// useStore behaves identically without pulling in the rest of the projects +// store (backend calls, project list, etc.) which is irrelevant to the Tip fix. +// vi.mock factories are hoisted above the rest of the file, so the atom must +// be created inside vi.hoisted to exist by the time the factory runs. +const { $projectDialog } = vi.hoisted(() => { + const { atom } = require('nanostores') as typeof Nanostores + + return { + $projectDialog: atom<{ mode: 'create' | 'rename' | 'add-folder'; name?: string; projectId?: string } | null>({ + mode: 'create' + }) + } +}) + +vi.mock('@/store/projects', () => ({ + $projectDialog, + addProjectFolder: vi.fn(), + closeProjectDialog: vi.fn(), + createProject: vi.fn(), + generateProjectIdea: vi.fn(), + pickProjectFolder: vi.fn(async () => '/Users/test/my-folder'), + renameProject: vi.fn() +})) + +vi.mock('@/store/notifications', () => ({ + notifyError: vi.fn() +})) + +vi.mock('@/lib/project-idea-templates', () => ({ + randomIdeaTemplates: () => [{ emoji: '🚀', idea: 'A rocket tracker', label: 'Rocket tracker' }] +})) + +const tipTrigger = (el: HTMLElement) => el.closest('[data-slot="tooltip-trigger"]') + +describe('ProjectDialog', () => { + it('wraps the "shuffle idea" button in a Tip', () => { + render() + + const button = screen.getByRole('button', { name: 'Shuffle ideas' }) + expect(tipTrigger(button)).toBeTruthy() + }) + + it('wraps the "remove folder" button in a Tip once a folder is added', async () => { + render() + + fireEvent.click(screen.getByRole('button', { name: 'Add folder' })) + + const button = await screen.findByRole('button', { name: 'Remove folder' }) + expect(tipTrigger(button)).toBeTruthy() + }) +}) diff --git a/apps/desktop/src/app/chat/sidebar/project-dialog.tsx b/apps/desktop/src/app/chat/sidebar/project-dialog.tsx index 5d0fc29dba11..a6254a518b58 100644 --- a/apps/desktop/src/app/chat/sidebar/project-dialog.tsx +++ b/apps/desktop/src/app/chat/sidebar/project-dialog.tsx @@ -14,6 +14,7 @@ import { import { GenerateButton } from '@/components/ui/generate-button' import { Input } from '@/components/ui/input' import { Textarea } from '@/components/ui/textarea' +import { Tip } from '@/components/ui/tooltip' import { useI18n } from '@/i18n' import { type ProjectIdeaTemplate, randomIdeaTemplates } from '@/lib/project-idea-templates' import { cn } from '@/lib/utils' @@ -197,16 +198,18 @@ export function ProjectDialog() { {p.primaryBadge} )} - + + + ))} @@ -258,17 +261,19 @@ export function ProjectDialog() { {template.label} ))} - + + +
)} diff --git a/apps/desktop/src/app/chat/sidebar/projects/overview-row.test.tsx b/apps/desktop/src/app/chat/sidebar/projects/overview-row.test.tsx new file mode 100644 index 000000000000..a4b6064ffd5b --- /dev/null +++ b/apps/desktop/src/app/chat/sidebar/projects/overview-row.test.tsx @@ -0,0 +1,69 @@ +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import type { SessionInfo } from '@/hermes' + +import { ProjectOverviewRow } from './overview-row' +import type { SidebarProjectTree } from './workspace-groups' + +afterEach(cleanup) + +vi.mock('@/i18n', () => ({ + useI18n: () => ({ + t: { + sidebar: { + newSessionIn: (label: string) => `New session in ${label}`, + projects: { + enter: (label: string) => `Enter ${label}`, + reorder: (label: string) => `Reorder ${label}`, + toggle: (label: string) => `Toggle ${label} sessions` + } + } + } + }) +})) + +vi.mock('./model', () => ({ + PROJECT_PREVIEW_COUNT: 3, + latestProjectSessions: () => [], + useWorkspaceNodeOpen: () => [false, vi.fn()] +})) + +// ProjectMenu (the kebab) has its own dedicated test file — stub it here so +// this file only exercises overview-row's own Tip usage (the disclosure +// toggle) plus the WorkspaceAddButton wiring. +vi.mock('./project-menu', () => ({ + ProjectMenu: () => null +})) + +const project = { id: 'p1', label: 'Test D' } as unknown as SidebarProjectTree + +const tipTrigger = (el: HTMLElement) => el.closest('[data-slot="tooltip-trigger"]') + +describe('ProjectOverviewRow', () => { + it('wraps the "new session" add button in a Tip with the project-scoped label', () => { + render() + + const button = screen.getByRole('button', { name: 'New session in Test D' }) + expect(tipTrigger(button)).toBeTruthy() + }) + + it('wraps the disclosure toggle in a Tip when there are preview sessions', () => { + render( + null} + /> + ) + + const button = screen.getByRole('button', { name: 'Toggle Test D sessions' }) + expect(tipTrigger(button)).toBeTruthy() + }) + + it('does not render the disclosure toggle when there is nothing to preview', () => { + render() + + expect(screen.queryByRole('button', { name: 'Toggle Test D sessions' })).toBeNull() + }) +}) diff --git a/apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx b/apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx index b3f779f2f2e3..c4aeefb2d55b 100644 --- a/apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx +++ b/apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx @@ -3,6 +3,7 @@ import { useRef } from 'react' import { Codicon } from '@/components/ui/codicon' import { DisclosureCaret } from '@/components/ui/disclosure-caret' +import { Tip } from '@/components/ui/tooltip' import type { SessionInfo } from '@/hermes' import { useI18n } from '@/i18n' import { cn } from '@/lib/utils' @@ -135,17 +136,19 @@ export function ProjectOverviewRow({ {project.label} {preview.length > 0 ? ( - + + + ) : ( )} diff --git a/apps/desktop/src/app/chat/sidebar/projects/project-menu.test.tsx b/apps/desktop/src/app/chat/sidebar/projects/project-menu.test.tsx new file mode 100644 index 000000000000..9ceb3b7be385 --- /dev/null +++ b/apps/desktop/src/app/chat/sidebar/projects/project-menu.test.tsx @@ -0,0 +1,132 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest' + +import { ProjectMenu } from './project-menu' +import type { SidebarProjectTree } from './workspace-groups' + +afterEach(cleanup) + +// jsdom doesn't implement ResizeObserver; Radix's PopoverContent/Arrow use it +// (via @radix-ui/react-use-size) to measure the arrow once the popover is +// actually mounted. The kebab-only test above never opens a Popover, so it +// doesn't need this — only the appearance-popover test below does. +beforeAll(() => { + vi.stubGlobal( + 'ResizeObserver', + class { + observe() {} + unobserve() {} + disconnect() {} + } + ) +}) + +vi.mock('@/i18n', () => ({ + useI18n: () => ({ + t: { + common: { cancel: 'Cancel', confirm: 'Confirm', done: 'Done', loading: 'Loading…' }, + sidebar: { + projects: { + copyPath: 'Copy path', + deleteConfirm: 'This cannot be undone.', + menu: 'Project actions', + menuAddFolder: 'Add folder', + menuAppearance: 'Appearance', + menuDelete: 'Delete', + menuRename: 'Rename', + menuSetActive: 'Set active', + noColor: 'No color', + removeFromSidebar: 'Remove from sidebar', + reveal: 'Reveal in file manager' + } + } + } + }) +})) + +vi.mock('@/store/layout', () => ({ + $panesFlipped: { + get: () => false, + listen: () => () => {}, + subscribe: (fn: (v: boolean) => void) => { + fn(false) + + return () => {} + } + }, + dismissAutoProject: vi.fn() +})) + +vi.mock('@/store/projects', () => ({ + copyPath: vi.fn(), + deleteProject: vi.fn(), + openProjectAddFolder: vi.fn(), + openProjectRename: vi.fn(), + revealPath: vi.fn(), + setActiveProject: vi.fn(), + setProjectAppearance: vi.fn().mockResolvedValue(false) +})) + +const project = { + color: null, + icon: null, + id: 'p1', + isAuto: false, + label: 'Test D', + path: '/repo' +} as unknown as SidebarProjectTree + +const tipTrigger = (el: HTMLElement) => el.closest('[data-slot="tooltip-trigger"]') + +const openTriggerMenu = (trigger: HTMLElement) => { + // Radix's dropdown trigger opens on pointerdown (a synthetic 'click' fireEvent + // alone won't do it), so fire the full mouse sequence a real click produces — + // same technique as session-actions-menu.test.tsx (#67500). + fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.click(trigger) +} + +describe('ProjectMenu', () => { + it('wraps the kebab trigger in a Tip', () => { + render() + + const button = screen.getByRole('button', { name: 'Project actions' }) + expect(tipTrigger(button)).toBeTruthy() + }) + + // #67500 (Gille, second pass): when anchorRef is absent, the trigger used to + // be `{trigger}` where `trigger` was + // ALREADY wrapped in — so PopoverAnchor's asChild cloned Tip itself + // (Tip doesn't forward extra props to its children), and the popover's + // real-DOM anchor ref never reached the button. Composing Tip OUTSIDE + // PopoverAnchor (Tip > PopoverAnchor > DropdownMenuTrigger > button) fixes + // that ref delivery. + // + // What this test can't verify: jsdom has no layout engine, so the actual + // POSITIONING the anchor ref enables isn't observable here — same + // limitation already noted above for the icon grid. What it does verify: + // the 3-deep asChild chain doesn't regress into the same silent-drop + // failure as the original bug (#67500, first pass) — the trigger stays a + // real, clickable element that opens the menu and reaches the Appearance + // popover end-to-end, for the anchorRef-absent path specifically (the + // anchorRef-present path never touches PopoverAnchor and is covered by the + // kebab test above). + it('opens the appearance popover through the kebab trigger when anchorRef is absent', async () => { + render() + + const trigger = screen.getByRole('button', { name: 'Project actions' }) + + openTriggerMenu(trigger) + + const appearanceItem = await screen.findByRole('menuitem', { name: 'Appearance' }) + + fireEvent.click(appearanceItem) + + // The color-swatch "No color" clear option only renders once the + // appearance Popover is actually open — proving the click reached the + // real button through the full Tip > PopoverAnchor > DropdownMenuTrigger + // chain rather than getting silently dropped on an intermediate wrapper. + expect(await screen.findByRole('button', { name: 'No color' })).toBeTruthy() + }, 15000) +}) diff --git a/apps/desktop/src/app/chat/sidebar/projects/project-menu.tsx b/apps/desktop/src/app/chat/sidebar/projects/project-menu.tsx index 19d86a11cd62..6eaeaab12a4e 100644 --- a/apps/desktop/src/app/chat/sidebar/projects/project-menu.tsx +++ b/apps/desktop/src/app/chat/sidebar/projects/project-menu.tsx @@ -13,6 +13,7 @@ import { DropdownMenuTrigger } from '@/components/ui/dropdown-menu' import { Popover, PopoverAnchor, PopoverContent } from '@/components/ui/popover' +import { Tip } from '@/components/ui/tooltip' import { useI18n } from '@/i18n' import { PROFILE_SWATCHES } from '@/lib/profile-color' import { cn } from '@/lib/utils' @@ -128,7 +129,14 @@ export function ProjectMenu({ ) - const trigger = ( + // The bare trigger button (no Tip, no anchor) — composed with whichever of + // Tip / PopoverAnchor apply below, always OUTSIDE the asChild chain that + // ends at this button, never wrapping it directly. asChild clones only its + // immediate child, so any of these wrappers placed inside another + // asChild-consuming component (instead of around it) would have its + // injected props silently swallowed by that inner component instead of + // reaching the real DOM button (see #67500). + const triggerButton = ( + + + ))} diff --git a/apps/desktop/src/app/chat/sidebar/projects/workspace-header.test.tsx b/apps/desktop/src/app/chat/sidebar/projects/workspace-header.test.tsx new file mode 100644 index 000000000000..418db785730c --- /dev/null +++ b/apps/desktop/src/app/chat/sidebar/projects/workspace-header.test.tsx @@ -0,0 +1,81 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { StartWorkButton, WorkspaceAddButton, WorkspaceMenu, WorkspaceShowMoreButton } from './workspace-header' + +afterEach(cleanup) + +vi.mock('@/i18n', () => ({ + useI18n: () => ({ + t: { + sidebar: { + projects: { + copyPath: 'Copy path', + menu: 'Project actions', + removeWorktree: 'Remove worktree', + reveal: 'Reveal in file manager', + startWork: 'New worktree' + }, + showMoreIn: (n: number, label: string) => `Show ${n} more in ${label}` + } + } + }) +})) + +vi.mock('@/store/projects', () => ({ + copyPath: vi.fn(), + revealPath: vi.fn() +})) + +// StartWorkButton renders the full WorktreeDialog (branch picker, git combobox, +// etc.) as soon as it's open — none of that is relevant to the tooltip fix, so +// stub it to keep this test focused on the trigger button. +vi.mock('./worktree-dialog', () => ({ + WorktreeDialog: () => null +})) + +const tipTrigger = (button: HTMLElement) => button.closest('[data-slot="tooltip-trigger"]') + +describe('WorkspaceAddButton', () => { + it('wraps the "+" button in a Tip', () => { + render() + + const button = screen.getByRole('button', { name: 'New session in Test D' }) + expect(tipTrigger(button)).toBeTruthy() + }) + + it('still fires onClick', () => { + const onClick = vi.fn() + render() + + fireEvent.click(screen.getByRole('button', { name: 'New session in Test D' })) + expect(onClick).toHaveBeenCalledOnce() + }) +}) + +describe('WorkspaceShowMoreButton', () => { + it('wraps the ellipsis button in a Tip with the composed label', () => { + render() + + const button = screen.getByRole('button', { name: 'Show 5 more in Test D' }) + expect(tipTrigger(button)).toBeTruthy() + }) +}) + +describe('WorkspaceMenu', () => { + it('wraps the kebab trigger in a Tip', () => { + render() + + const button = screen.getByRole('button', { name: 'Project actions' }) + expect(tipTrigger(button)).toBeTruthy() + }) +}) + +describe('StartWorkButton', () => { + it('wraps the git-branch trigger in a Tip', () => { + render() + + const button = screen.getByRole('button', { name: 'New worktree' }) + expect(tipTrigger(button)).toBeTruthy() + }) +}) diff --git a/apps/desktop/src/app/chat/sidebar/projects/workspace-header.tsx b/apps/desktop/src/app/chat/sidebar/projects/workspace-header.tsx index 0446800d25ab..da3431cf2400 100644 --- a/apps/desktop/src/app/chat/sidebar/projects/workspace-header.tsx +++ b/apps/desktop/src/app/chat/sidebar/projects/workspace-header.tsx @@ -10,6 +10,7 @@ import { DropdownMenuSeparator, DropdownMenuTrigger } from '@/components/ui/dropdown-menu' +import { Tip } from '@/components/ui/tooltip' import { useI18n } from '@/i18n' import { cn } from '@/lib/utils' import { copyPath, revealPath } from '@/store/projects' @@ -39,14 +40,16 @@ function LaneLabel({ label, title }: { label: string; title?: string }) { // "+" affordance shared by repo and worktree headers — reveals on header hover. export function WorkspaceAddButton({ label, onClick }: { label: string; onClick: () => void }) { return ( - + + + ) } @@ -64,14 +67,16 @@ export function WorkspaceShowMoreButton({ const text = t.sidebar.showMoreIn(count, label) return ( - + + + ) } @@ -84,16 +89,18 @@ export function WorkspaceMenu({ path, onRemove }: { path: null | string; onRemov return ( - - - + + + + + void revealPath(path)}> @@ -125,14 +132,16 @@ export function StartWorkButton({ repoPath, onStarted }: { repoPath: string; onS return ( <> - + + + ) diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx new file mode 100644 index 000000000000..ef989a4d643a --- /dev/null +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx @@ -0,0 +1,120 @@ +import { atom } from 'nanostores' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { SessionActionsMenu } from './session-actions-menu' + +afterEach(cleanup) + +// This file exists specifically to catch the regression flagged in #67500: +// SessionActionsMenu used to be composed as +// {children} +// with the caller wrapping ITS children in . Radix's `asChild` clones +// its single child and injects onClick/aria-haspopup/ref onto it — but Tip +// doesn't forward those extra props to whatever it wraps, so they were +// silently dropped and the menu could stop opening. Tip has since moved +// inside this component (wrapping DropdownMenuTrigger itself, not the other +// way around) — these tests exercise the REAL component end-to-end (no mock +// of DropdownMenu/Tip) so a future regression of this composition fails here. + +vi.mock('@/components/pane-shell/tree/store', () => ({ + closeAllTreeTabs: vi.fn(), + closeOtherTreeTabs: vi.fn(), + closeTreeTabsToRight: vi.fn(), + treeTabCloseTargets: vi.fn(() => null) +})) +vi.mock('@/hermes', () => ({ renameSession: vi.fn() })) +vi.mock('@/i18n', () => ({ + useI18n: () => ({ + t: { + common: { cancel: 'Cancel', close: 'Close', delete: 'Delete', save: 'Save' }, + sidebar: { + projects: { menuAppearance: 'Appearance', noColor: 'No color' }, + row: { + actionsFor: (title: string) => `Actions for ${title}`, + archive: 'Archive', + branchFrom: 'Branch from here', + copyId: 'Copy ID', + copyIdFailed: 'Failed to copy ID', + export: 'Export', + hideTabBar: 'Hide tab bar', + pin: 'Pin', + rename: 'Rename', + renameDesc: 'Rename this session', + renameFailed: 'Rename failed', + renameTitle: 'Rename session', + renamed: 'Renamed', + unpin: 'Unpin', + untitledPlaceholder: 'Untitled' + } + }, + zones: { closeAll: 'Close all', closeOthers: 'Close others', closeToRight: 'Close to the right' } + } + }) +})) +vi.mock('@/lib/haptics', () => ({ triggerHaptic: vi.fn() })) +vi.mock('@/lib/profile-color', () => ({ PROFILE_SWATCHES: [] })) +vi.mock('@/lib/session-export', () => ({ exportSession: vi.fn() })) +vi.mock('@/store/gateway', () => ({ activeGateway: vi.fn(() => null) })) +vi.mock('@/store/notifications', () => ({ notify: vi.fn(), notifyError: vi.fn() })) +vi.mock('@/store/session', () => ({ + $activeSessionId: atom(null), + $selectedStoredSessionId: atom(null), + $sessions: atom([]), + sessionMatchesStoredId: vi.fn(() => false), + sessionPinId: vi.fn((s: { id: string }) => s.id), + setSessions: vi.fn() +})) +vi.mock('@/store/session-color', () => ({ + $sessionColorOverrides: atom>({}), + setSessionColorOverride: vi.fn() +})) +vi.mock('@/store/session-states', () => ({ + $sessionTiles: atom([]), + openSessionTile: vi.fn() +})) +vi.mock('@/store/windows', () => ({ + canOpenSessionWindow: () => false, + openSessionInNewWindow: vi.fn() +})) + +function renderMenu() { + return render( + + + + ) +} + +describe('SessionActionsMenu', () => { + it('shows the tooltip label wired to the real trigger button', () => { + renderMenu() + + const trigger = screen.getByRole('button', { name: 'Actions for My session' }) + + expect(trigger.closest('[data-slot="tooltip-trigger"]')).toBeTruthy() + }) + + it('still opens the dropdown on click with the trigger wrapped in a Tip (#67500)', async () => { + renderMenu() + + const trigger = screen.getByRole('button', { name: 'Actions for My session' }) + + // Radix's dropdown trigger opens on pointerdown (not on the synthetic + // 'click' fireEvent alone would dispatch), so fire the full mouse + // sequence a real click produces. + fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.click(trigger) + + // If Tip (now composed around DropdownMenuTrigger, not the other way + // round) ever stopped forwarding the asChild-injected props again, this + // menu would never open and these queries would throw instead of + // resolving. + expect(await screen.findByRole('menu')).toBeTruthy() + expect(screen.getByRole('menuitem', { name: /rename/i })).toBeTruthy() + expect(screen.getByRole('menuitem', { name: /archive/i })).toBeTruthy() + }) +}) diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx index 889ebd4e0d6a..43bde9b516fc 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx @@ -41,6 +41,7 @@ import { DropdownMenuTrigger } from '@/components/ui/dropdown-menu' import { Input } from '@/components/ui/input' +import { Tip } from '@/components/ui/tooltip' import { renameSession } from '@/hermes' import { useI18n } from '@/i18n' import { triggerHaptic } from '@/lib/haptics' @@ -441,9 +442,21 @@ function useSessionActions({ interface SessionActionsMenuProps extends SessionActions, Pick, 'align' | 'sideOffset'> { children: React.ReactNode + /** Tooltip label for the trigger. Composed INSIDE the dropdown trigger + * (Tip wraps DropdownMenuTrigger, not the other way around) — Tip doesn't + * forward the extra props/ref an `asChild` clone injects, so putting it as + * the trigger's direct child silently drops onClick/aria-haspopup/ref and + * the menu stops opening (#67500). */ + tooltip?: React.ReactNode } -export function SessionActionsMenu({ children, align = 'end', sideOffset = 6, ...actions }: SessionActionsMenuProps) { +export function SessionActionsMenu({ + children, + tooltip, + align = 'end', + sideOffset = 6, + ...actions +}: SessionActionsMenuProps) { const { t } = useI18n() const { renameDialog, renderItems } = useSessionActions(actions) const [open, setOpen] = useState(false) @@ -451,7 +464,9 @@ export function SessionActionsMenu({ children, align = 'end', sideOffset = 6, .. return ( <> - {children} + + {children} + ({ + useI18n: () => ({ + t: { + sidebar: { + row: { + actionsFor: (title: string) => `Actions for ${title}`, + ageMin: 'm', + ageNow: 'now', + backgroundRunning: 'Running in background', + finishedUnread: 'Finished', + handoffOrigin: (platform: string) => `Started on ${platform}`, + needsInput: 'Needs input', + sessionRunning: 'Running', + waitingForAnswer: 'Waiting for answer' + } + } + } + }) +})) + +vi.mock('@/app/chat/profile-tag', () => ({ ProfileTag: () => null })) +vi.mock('@/app/chat/session-drag', () => ({ startSessionDrag: vi.fn() })) +// PlatformAvatar is intentionally NOT mocked (do not reintroduce this — see +// #67500, Gille's third pass): it's a forwardRef component that spreads its +// props onto the rendered span, and mocking it with a stand-in that spreads +// props itself only proves the MOCK forwards them, not that the real +// component does. This file exercises the actual production component so a +// regression in its ref/prop forwarding fails here again. +vi.mock('@/lib/chat-runtime', () => ({ sessionTitle: (s: SessionInfo) => (s as unknown as { title: string }).title })) +vi.mock('@/lib/haptics', () => ({ triggerHaptic: vi.fn() })) +vi.mock('@/lib/session-source', () => ({ + handoffOriginSource: (state?: string, platform?: string) => (state && platform ? platform : null), + sessionSourceLabel: (source: string) => source +})) +vi.mock('@/lib/time', () => ({ coarseElapsed: () => ({ unit: 'minute' as const, value: 5 }) })) + +// These mocks use importOriginal rather than replacing the module wholesale: +// session-row.tsx (and its transitive imports, e.g. session-color.ts) reads +// several store exports beyond the ones this file cares about, and that set +// keeps growing as the app evolves upstream. A wholesale replacement mock +// silently turns every export it doesn't list into `undefined`, which then +// crashes nanostores' `computed()` the moment a new dependency is added +// upstream (as happened twice already: $stalledSessionIds, then $sessions). +// Overriding only the named atoms we actually control keeps this test +// resilient to that drift. +vi.mock('@/store/composer-status', async importOriginal => { + const actual = await importOriginal() + + return { ...actual, $backgroundRunningSessionIds: atom([]) } +}) +vi.mock('@/store/session', async importOriginal => { + const actual = await importOriginal() + + return { ...actual, $unreadFinishedSessionIds: atom([]) } +}) +vi.mock('@/store/session-states', async importOriginal => { + const actual = await importOriginal() + + return { + ...actual, + $attentionSessionIds: atom([]), + $stalledSessionIds: atom([]), + openSessionTile: vi.fn() + } +}) +vi.mock('@/store/windows', async importOriginal => { + const actual = await importOriginal() + + return { + ...actual, + canOpenSessionWindow: () => false, + openSessionInNewWindow: vi.fn() + } +}) + +// SessionActionsMenu owns the Tip-around-DropdownMenuTrigger composition +// itself now (see session-actions-menu.test.tsx, which exercises that real, +// unmocked end-to-end) — testing it again here via the mock would just +// duplicate that coverage and silently stop testing anything the moment the +// mock's shape drifts from the real component's props (as happened when +// `tooltip` was introduced). This file only needs to confirm session-row +// wires the right tooltip text into the `tooltip` prop, so the mock renders +// it in a way we can assert on directly instead of re-deriving Tip's +// internal DOM structure. +vi.mock('./session-actions-menu', () => ({ + SessionActionsMenu: ({ children, tooltip }: { children: React.ReactNode; tooltip?: string }) => ( +
+ {children} +
+ ), + SessionContextMenu: ({ children }: { children: React.ReactNode }) => <>{children} +})) + +vi.mock('./use-profile-prewarm', () => ({ + useProfilePrewarm: () => ({ cancelPrewarm: vi.fn(), startPrewarm: vi.fn() }) +})) + +function makeSession(overrides: Partial & { title: string }): SessionInfo { + return { + handoff_platform: null, + handoff_state: null, + id: 's1', + last_active: 0, + profile: 'default', + started_at: 0, + ...overrides + } as unknown as SessionInfo +} + +const tipTrigger = (el: HTMLElement) => el.closest('[data-slot="tooltip-trigger"]') + +const noop = vi.fn() + +describe('SidebarSessionRow', () => { + it('wires the actions kebab tooltip text through to SessionActionsMenu', () => { + render( + + ) + + expect(screen.getByTestId('session-actions-menu').getAttribute('data-tooltip')).toBe( + 'Actions for Hermes doctor health check results' + ) + }) + + it('does not render a handoff avatar for a locally-started session', () => { + const { container } = render( + + ) + + // PlatformAvatar's span is the only aria-hidden SPAN this row ever + // renders (idle dot / arc-border / branch-stem are all inactive here) — + // Codicon icons (e.g. the kebab trigger) are also aria-hidden but render + // as , not , so this selector doesn't accidentally match them. + expect(container.querySelector('span[aria-hidden="true"]')).toBeNull() + }) + + it('wraps the handoff platform avatar in a Tip for a session started on another platform', () => { + const { container } = render( + + ) + + // PlatformAvatar is the REAL component here (see the note above the vi.mock + // block, #67500 third pass) — it renders the Telegram brand SVG rather + // than the platform name as text, so query the avatar span itself (the + // row's only aria-hidden span in this state) rather than text content, + // and confirm its tooltip trigger actually attaches to it — proving the + // real forwardRef/...rest path works, not a mock that fakes it. + const avatar = container.querySelector('span[aria-hidden="true"]') + expect(avatar).toBeTruthy() + expect(tipTrigger(avatar as HTMLElement)).toBeTruthy() + }) +}) diff --git a/apps/desktop/src/app/chat/sidebar/session-row.tsx b/apps/desktop/src/app/chat/sidebar/session-row.tsx index 895d18bedbf8..7b1fb92007b8 100644 --- a/apps/desktop/src/app/chat/sidebar/session-row.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-row.tsx @@ -133,6 +133,7 @@ export function SidebarSessionRow({ profile={session.profile} sessionId={session.id} title={title} + tooltip={r.actionsFor(title)} >