mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
fix(desktop): wrap missing sidebar icon-button tooltips (#67500)
* fix(desktop): wrap sidebar icon buttons in Tip tooltips
Several icon-only buttons in the sidebar (header actions, workspace
menu, project menu, session actions, load-more) had aria-label but
no visual tooltip on hover. Wrap them in the existing <Tip> component,
matching the pattern already used elsewhere (e.g. ProfilePill).
No behavioral changes -- purely wraps existing buttons.
Adds vitest coverage asserting the Tip wrapper (data-slot=tooltip-trigger)
for 6 of 7 files; index.tsx is a 1500+ line top-level page component and
was verified manually via screenshots instead.
* fix(desktop): satisfy consistent-type-imports lint rule in project-dialog test
* test(desktop): update session-row mocks for restored sessionColorById
* fix(desktop): compose Tip around the real trigger instead of inside it
Tip was being placed as SessionActionsMenu's/PlatformAvatar's DIRECT child,
which asChild then cloned instead of the actual button/span. Neither Tip nor
PlatformAvatar forwarded the injected onClick/ref, so both silently dropped
the wiring:
- session-actions-menu.tsx: Tip now wraps DropdownMenuTrigger internally
(new ooltip prop) instead of the caller wrapping its children in Tip.
- platform-icon.tsx: PlatformAvatar now forwards ref and spreads rest props
onto its span so a wrapping Tip's trigger actually attaches.
- session-row.tsx: updated call site to use the new tooltip prop.
- Added session-actions-menu.test.tsx exercising the real DropdownMenu open
behavior end-to-end (no Tip/Dropdown mocks).
- session-row.test.tsx no longer mocks PlatformAvatar's behavior; it now
exercises the real (fixed) component for the handoff-avatar tooltip.
* fix(desktop): compose Tip outside PopoverAnchor in ProjectMenu (#67500)
* test(desktop): update session-row test for the tooltip-prop composition (cbbbeb2fd)
* fix(desktop): satisfy consistent-type-imports in session-row.test.tsx mocks
* chore: retrigger CI
* test(desktop): stop mocking PlatformAvatar's behavior (#67500, third pass)
The mock was re-introduced by a prior edit that fixed an unrelated lint
error, silently undoing the earlier fix where this test started exercising
the real (forwardRef) PlatformAvatar. Removed the mock; updated the two
handoff-avatar tests to query the real component's rendered span instead of
text content, since it renders a brand SVG icon for known platforms rather
than the platform name as text.
This commit is contained in:
parent
9cc475cc58
commit
0c33db0564
16 changed files with 965 additions and 144 deletions
|
|
@ -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
|
||||
/>
|
||||
<div className="grid size-6 place-items-center">
|
||||
<Button
|
||||
aria-label={s.showProjects}
|
||||
className={HEADER_NAV_BTN}
|
||||
onClick={event => {
|
||||
event.stopPropagation()
|
||||
exitProjectScope()
|
||||
}}
|
||||
size="icon-xs"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name="list-unordered" size="0.75rem" />
|
||||
</Button>
|
||||
<Tip label={s.showProjects}>
|
||||
<Button
|
||||
aria-label={s.showProjects}
|
||||
className={HEADER_NAV_BTN}
|
||||
onClick={event => {
|
||||
event.stopPropagation()
|
||||
exitProjectScope()
|
||||
}}
|
||||
size="icon-xs"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name="list-unordered" size="0.75rem" />
|
||||
</Button>
|
||||
</Tip>
|
||||
</div>
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex shrink-0 items-center gap-0.5">
|
||||
{!showAllProfiles ? (
|
||||
<Button
|
||||
aria-label={agentsGrouped ? s.projects.newButton : s.nav['new-session']}
|
||||
className={HEADER_ACTION_BTN}
|
||||
onClick={event => {
|
||||
event.stopPropagation()
|
||||
|
||||
if (agentsGrouped) {
|
||||
openProjectCreate()
|
||||
} else {
|
||||
onNewSessionInWorkspace(null)
|
||||
}
|
||||
}}
|
||||
size="icon-xs"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name="add" size="0.75rem" />
|
||||
</Button>
|
||||
) : null}
|
||||
<div className="grid size-6 place-items-center">
|
||||
{!showAllProfiles && agentSessions.length > 0 ? (
|
||||
<Tip label={agentsGrouped ? s.projects.newButton : s.nav['new-session']}>
|
||||
<Button
|
||||
aria-label={agentsGrouped ? s.showSessions : s.showProjects}
|
||||
className={cn(
|
||||
HEADER_NAV_BTN,
|
||||
agentsGrouped && 'bg-(--ui-control-active-background) text-foreground opacity-100'
|
||||
)}
|
||||
aria-label={agentsGrouped ? s.projects.newButton : s.nav['new-session']}
|
||||
className={HEADER_ACTION_BTN}
|
||||
onClick={event => {
|
||||
event.stopPropagation()
|
||||
setSidebarRecentsOpen(true)
|
||||
setSidebarAgentsGrouped(!agentsGrouped)
|
||||
|
||||
if (agentsGrouped) {
|
||||
openProjectCreate()
|
||||
} else {
|
||||
onNewSessionInWorkspace(null)
|
||||
}
|
||||
}}
|
||||
size="icon-xs"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name={agentsGrouped ? 'list-unordered' : 'root-folder'} size="0.75rem" />
|
||||
<Codicon name="add" size="0.75rem" />
|
||||
</Button>
|
||||
</Tip>
|
||||
) : null}
|
||||
<div className="grid size-6 place-items-center">
|
||||
{!showAllProfiles && agentSessions.length > 0 ? (
|
||||
<Tip label={agentsGrouped ? s.showSessions : s.showProjects}>
|
||||
<Button
|
||||
aria-label={agentsGrouped ? s.showSessions : s.showProjects}
|
||||
className={cn(
|
||||
HEADER_NAV_BTN,
|
||||
agentsGrouped && 'bg-(--ui-control-active-background) text-foreground opacity-100'
|
||||
)}
|
||||
onClick={event => {
|
||||
event.stopPropagation()
|
||||
setSidebarRecentsOpen(true)
|
||||
setSidebarAgentsGrouped(!agentsGrouped)
|
||||
}}
|
||||
size="icon-xs"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name={agentsGrouped ? 'list-unordered' : 'root-folder'} size="0.75rem" />
|
||||
</Button>
|
||||
</Tip>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
54
apps/desktop/src/app/chat/sidebar/load-more-row.test.tsx
Normal file
54
apps/desktop/src/app/chat/sidebar/load-more-row.test.tsx
Normal file
|
|
@ -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 #<issue> was missing.
|
||||
describe('SidebarLoadMoreRow', () => {
|
||||
it('wraps the button in a Tip with the loading label as the trigger', () => {
|
||||
render(<SidebarLoadMoreRow loading onClick={vi.fn()} step={0} />)
|
||||
|
||||
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(<SidebarLoadMoreRow onClick={vi.fn()} step={5} />)
|
||||
|
||||
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(<SidebarLoadMoreRow onClick={vi.fn()} step={0} />)
|
||||
|
||||
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(<SidebarLoadMoreRow onClick={onClick} step={0} />)
|
||||
|
||||
screen.getByRole('button', { name: 'Load more' }).click()
|
||||
expect(onClick).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
|
@ -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 (
|
||||
<button
|
||||
aria-label={label}
|
||||
className="ml-auto grid size-5 place-items-center rounded-sm bg-transparent text-(--ui-text-tertiary) transition-colors hover:bg-(--ui-control-hover-background) hover:text-foreground disabled:cursor-default disabled:opacity-60 disabled:hover:bg-transparent disabled:hover:text-(--ui-text-tertiary)"
|
||||
disabled={loading}
|
||||
onClick={onClick}
|
||||
type="button"
|
||||
>
|
||||
{loading ? (
|
||||
<GlyphSpinner ariaLabel={label} className="text-[0.75rem]" />
|
||||
) : (
|
||||
<Codicon name="ellipsis" size="0.75rem" />
|
||||
)}
|
||||
</button>
|
||||
<Tip label={label}>
|
||||
<button
|
||||
aria-label={label}
|
||||
className="ml-auto grid size-5 place-items-center rounded-sm bg-transparent text-(--ui-text-tertiary) transition-colors hover:bg-(--ui-control-hover-background) hover:text-foreground disabled:cursor-default disabled:opacity-60 disabled:hover:bg-transparent disabled:hover:text-(--ui-text-tertiary)"
|
||||
disabled={loading}
|
||||
onClick={onClick}
|
||||
type="button"
|
||||
>
|
||||
{loading ? (
|
||||
<GlyphSpinner ariaLabel={label} className="text-[0.75rem]" />
|
||||
) : (
|
||||
<Codicon name="ellipsis" size="0.75rem" />
|
||||
)}
|
||||
</button>
|
||||
</Tip>
|
||||
)
|
||||
}
|
||||
|
|
|
|||
87
apps/desktop/src/app/chat/sidebar/project-dialog.test.tsx
Normal file
87
apps/desktop/src/app/chat/sidebar/project-dialog.test.tsx
Normal file
|
|
@ -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(<ProjectDialog />)
|
||||
|
||||
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(<ProjectDialog />)
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Add folder' }))
|
||||
|
||||
const button = await screen.findByRole('button', { name: 'Remove folder' })
|
||||
expect(tipTrigger(button)).toBeTruthy()
|
||||
})
|
||||
})
|
||||
|
|
@ -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}
|
||||
</span>
|
||||
)}
|
||||
<Button
|
||||
aria-label={p.removeFolder}
|
||||
className="size-5 shrink-0 text-(--ui-text-quaternary) hover:text-foreground"
|
||||
onClick={() => setFolders(prev => prev.filter(f => f !== folder))}
|
||||
size="icon-xs"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name="close" size="0.75rem" />
|
||||
</Button>
|
||||
<Tip label={p.removeFolder}>
|
||||
<Button
|
||||
aria-label={p.removeFolder}
|
||||
className="size-5 shrink-0 text-(--ui-text-quaternary) hover:text-foreground"
|
||||
onClick={() => setFolders(prev => prev.filter(f => f !== folder))}
|
||||
size="icon-xs"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name="close" size="0.75rem" />
|
||||
</Button>
|
||||
</Tip>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
|
|
@ -258,17 +261,19 @@ export function ProjectDialog() {
|
|||
{template.label}
|
||||
</button>
|
||||
))}
|
||||
<Button
|
||||
aria-label={p.ideaShuffle}
|
||||
className="size-5 text-(--ui-text-quaternary) hover:text-foreground"
|
||||
disabled={submitting}
|
||||
onClick={() => setTemplates(randomIdeaTemplates())}
|
||||
size="icon-xs"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name="refresh" size="0.75rem" />
|
||||
</Button>
|
||||
<Tip label={p.ideaShuffle}>
|
||||
<Button
|
||||
aria-label={p.ideaShuffle}
|
||||
className="size-5 text-(--ui-text-quaternary) hover:text-foreground"
|
||||
disabled={submitting}
|
||||
onClick={() => setTemplates(randomIdeaTemplates())}
|
||||
size="icon-xs"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name="refresh" size="0.75rem" />
|
||||
</Button>
|
||||
</Tip>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -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(<ProjectOverviewRow onNewSession={vi.fn()} project={project} />)
|
||||
|
||||
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(
|
||||
<ProjectOverviewRow
|
||||
previewSessions={[{ id: 's1' } as unknown as SessionInfo]}
|
||||
project={project}
|
||||
renderRows={() => 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(<ProjectOverviewRow project={project} />)
|
||||
|
||||
expect(screen.queryByRole('button', { name: 'Toggle Test D sessions' })).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
@ -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}
|
||||
</SidebarRowLink>
|
||||
{preview.length > 0 ? (
|
||||
<button
|
||||
aria-label={s.projects.toggle(project.label)}
|
||||
className="flex flex-1 items-center self-stretch bg-transparent p-0"
|
||||
onClick={toggleOpen}
|
||||
type="button"
|
||||
>
|
||||
<DisclosureCaret
|
||||
className="shrink-0 text-(--ui-text-tertiary) opacity-0 transition group-hover/workspace:opacity-100"
|
||||
open={open}
|
||||
/>
|
||||
</button>
|
||||
<Tip label={s.projects.toggle(project.label)}>
|
||||
<button
|
||||
aria-label={s.projects.toggle(project.label)}
|
||||
className="flex flex-1 items-center self-stretch bg-transparent p-0"
|
||||
onClick={toggleOpen}
|
||||
type="button"
|
||||
>
|
||||
<DisclosureCaret
|
||||
className="shrink-0 text-(--ui-text-tertiary) opacity-0 transition group-hover/workspace:opacity-100"
|
||||
open={open}
|
||||
/>
|
||||
</button>
|
||||
</Tip>
|
||||
) : (
|
||||
<span className="flex-1" />
|
||||
)}
|
||||
|
|
|
|||
132
apps/desktop/src/app/chat/sidebar/projects/project-menu.test.tsx
Normal file
132
apps/desktop/src/app/chat/sidebar/projects/project-menu.test.tsx
Normal file
|
|
@ -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(<ProjectMenu isActive={false} project={project} />)
|
||||
|
||||
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 `<PopoverAnchor asChild>{trigger}</PopoverAnchor>` where `trigger` was
|
||||
// ALREADY wrapped in <Tip> — 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(<ProjectMenu isActive={false} project={project} />)
|
||||
|
||||
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)
|
||||
})
|
||||
|
|
@ -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({
|
|||
</DropdownMenuItem>
|
||||
)
|
||||
|
||||
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 = (
|
||||
<DropdownMenuTrigger asChild>
|
||||
<button
|
||||
aria-label={p.menu}
|
||||
|
|
@ -146,13 +154,24 @@ export function ProjectMenu({
|
|||
</DropdownMenuTrigger>
|
||||
)
|
||||
|
||||
// Tip always wraps the outermost element of whatever we render — either the
|
||||
// trigger directly (anchorRef present: the popover anchors to the whole row
|
||||
// via a separate virtualRef, so PopoverAnchor isn't involved here), or the
|
||||
// PopoverAnchor-wrapped trigger (anchorRef absent: the popover anchors to
|
||||
// this button itself). Either way, Tip > PopoverAnchor > DropdownMenuTrigger
|
||||
// > button, so asChild composes props/ref all the way down to the real DOM
|
||||
// node instead of stopping at an intermediate wrapper.
|
||||
const trigger = (
|
||||
<Tip label={p.menu}>{anchorRef ? triggerButton : <PopoverAnchor asChild>{triggerButton}</PopoverAnchor>}</Tip>
|
||||
)
|
||||
|
||||
return (
|
||||
<Popover onOpenChange={setAppearanceOpen} open={appearanceOpen}>
|
||||
{/* Position the appearance popover against the row (when a ref is wired);
|
||||
the kebab is only the dropdown trigger then. */}
|
||||
{anchorRef ? <PopoverAnchor virtualRef={anchorRef as React.RefObject<HTMLElement>} /> : null}
|
||||
<DropdownMenu>
|
||||
{anchorRef ? trigger : <PopoverAnchor asChild>{trigger}</PopoverAnchor>}
|
||||
{trigger}
|
||||
{/* Closing the menu refocuses the trigger (also the popover anchor),
|
||||
which the appearance popover would read as focus-outside and die on.
|
||||
Suppress that refocus so it survives. */}
|
||||
|
|
@ -230,19 +249,20 @@ export function ProjectMenu({
|
|||
profile picker's width (icons flex to fill, not fixed-width). */}
|
||||
<div className="mt-2 grid grid-cols-6 gap-1.5">
|
||||
{ICONS.map(name => (
|
||||
<button
|
||||
aria-label={name}
|
||||
className={cn(
|
||||
'grid aspect-square place-items-center rounded-md text-(--ui-text-tertiary) transition hover:bg-(--ui-control-hover-background)',
|
||||
project.icon === name && 'bg-(--ui-control-active-background) text-foreground'
|
||||
)}
|
||||
key={name}
|
||||
onClick={() => void applyAppearance({ icon: project.icon === name ? null : name })}
|
||||
style={project.icon === name && project.color ? { color: project.color } : undefined}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name={name} size="0.8125rem" />
|
||||
</button>
|
||||
<Tip key={name} label={name}>
|
||||
<button
|
||||
aria-label={name}
|
||||
className={cn(
|
||||
'grid aspect-square place-items-center rounded-md text-(--ui-text-tertiary) transition hover:bg-(--ui-control-hover-background)',
|
||||
project.icon === name && 'bg-(--ui-control-active-background) text-foreground'
|
||||
)}
|
||||
onClick={() => void applyAppearance({ icon: project.icon === name ? null : name })}
|
||||
style={project.icon === name && project.color ? { color: project.color } : undefined}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name={name} size="0.8125rem" />
|
||||
</button>
|
||||
</Tip>
|
||||
))}
|
||||
</div>
|
||||
</PopoverContent>
|
||||
|
|
|
|||
|
|
@ -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(<WorkspaceAddButton label="New session in Test D" onClick={vi.fn()} />)
|
||||
|
||||
const button = screen.getByRole('button', { name: 'New session in Test D' })
|
||||
expect(tipTrigger(button)).toBeTruthy()
|
||||
})
|
||||
|
||||
it('still fires onClick', () => {
|
||||
const onClick = vi.fn()
|
||||
render(<WorkspaceAddButton label="New session in Test D" onClick={onClick} />)
|
||||
|
||||
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(<WorkspaceShowMoreButton count={5} label="Test D" onClick={vi.fn()} />)
|
||||
|
||||
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(<WorkspaceMenu onRemove={vi.fn()} path="/repo/lane" />)
|
||||
|
||||
const button = screen.getByRole('button', { name: 'Project actions' })
|
||||
expect(tipTrigger(button)).toBeTruthy()
|
||||
})
|
||||
})
|
||||
|
||||
describe('StartWorkButton', () => {
|
||||
it('wraps the git-branch trigger in a Tip', () => {
|
||||
render(<StartWorkButton onStarted={vi.fn()} repoPath="/repo" />)
|
||||
|
||||
const button = screen.getByRole('button', { name: 'New worktree' })
|
||||
expect(tipTrigger(button)).toBeTruthy()
|
||||
})
|
||||
})
|
||||
|
|
@ -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 (
|
||||
<button
|
||||
aria-label={label}
|
||||
className="grid size-4 shrink-0 place-items-center rounded-sm bg-transparent text-(--ui-text-quaternary) opacity-0 transition-opacity hover:bg-(--ui-control-hover-background) hover:text-foreground group-hover/workspace:opacity-100"
|
||||
onClick={onClick}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name="add" size="0.75rem" />
|
||||
</button>
|
||||
<Tip label={label}>
|
||||
<button
|
||||
aria-label={label}
|
||||
className="grid size-4 shrink-0 place-items-center rounded-sm bg-transparent text-(--ui-text-quaternary) opacity-0 transition-opacity hover:bg-(--ui-control-hover-background) hover:text-foreground group-hover/workspace:opacity-100"
|
||||
onClick={onClick}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name="add" size="0.75rem" />
|
||||
</button>
|
||||
</Tip>
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -64,14 +67,16 @@ export function WorkspaceShowMoreButton({
|
|||
const text = t.sidebar.showMoreIn(count, label)
|
||||
|
||||
return (
|
||||
<button
|
||||
aria-label={text}
|
||||
className="ml-auto grid size-5 place-items-center rounded-sm bg-transparent text-(--ui-text-tertiary) transition-colors hover:bg-(--ui-control-hover-background) hover:text-foreground"
|
||||
onClick={onClick}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name="ellipsis" size="0.75rem" />
|
||||
</button>
|
||||
<Tip label={text}>
|
||||
<button
|
||||
aria-label={text}
|
||||
className="ml-auto grid size-5 place-items-center rounded-sm bg-transparent text-(--ui-text-tertiary) transition-colors hover:bg-(--ui-control-hover-background) hover:text-foreground"
|
||||
onClick={onClick}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name="ellipsis" size="0.75rem" />
|
||||
</button>
|
||||
</Tip>
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -84,16 +89,18 @@ export function WorkspaceMenu({ path, onRemove }: { path: null | string; onRemov
|
|||
|
||||
return (
|
||||
<DropdownMenu>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<button
|
||||
aria-label={p.menu}
|
||||
className="grid size-4 shrink-0 place-items-center rounded-sm bg-transparent text-(--ui-text-quaternary) opacity-0 transition-opacity hover:bg-(--ui-control-hover-background) hover:text-foreground group-hover/workspace:opacity-100 data-[state=open]:opacity-100"
|
||||
onClick={event => event.stopPropagation()}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name="kebab-vertical" size="0.75rem" />
|
||||
</button>
|
||||
</DropdownMenuTrigger>
|
||||
<Tip label={p.menu}>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<button
|
||||
aria-label={p.menu}
|
||||
className="grid size-4 shrink-0 place-items-center rounded-sm bg-transparent text-(--ui-text-quaternary) opacity-0 transition-opacity hover:bg-(--ui-control-hover-background) hover:text-foreground group-hover/workspace:opacity-100 data-[state=open]:opacity-100"
|
||||
onClick={event => event.stopPropagation()}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name="kebab-vertical" size="0.75rem" />
|
||||
</button>
|
||||
</DropdownMenuTrigger>
|
||||
</Tip>
|
||||
<DropdownMenuContent align="end" className="w-48" sideOffset={6}>
|
||||
<DropdownMenuItem disabled={!path} onSelect={() => void revealPath(path)}>
|
||||
<Codicon name="folder-opened" size="0.875rem" />
|
||||
|
|
@ -125,14 +132,16 @@ export function StartWorkButton({ repoPath, onStarted }: { repoPath: string; onS
|
|||
|
||||
return (
|
||||
<>
|
||||
<button
|
||||
aria-label={p.startWork}
|
||||
className="grid size-4 shrink-0 place-items-center rounded-sm bg-transparent text-(--ui-text-quaternary) opacity-0 transition-opacity hover:bg-(--ui-control-hover-background) hover:text-foreground group-hover/section:opacity-100 focus-visible:opacity-100"
|
||||
onClick={() => setOpen(true)}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name="git-branch" size="0.75rem" />
|
||||
</button>
|
||||
<Tip label={p.startWork}>
|
||||
<button
|
||||
aria-label={p.startWork}
|
||||
className="grid size-4 shrink-0 place-items-center rounded-sm bg-transparent text-(--ui-text-quaternary) opacity-0 transition-opacity hover:bg-(--ui-control-hover-background) hover:text-foreground group-hover/section:opacity-100 focus-visible:opacity-100"
|
||||
onClick={() => setOpen(true)}
|
||||
type="button"
|
||||
>
|
||||
<Codicon name="git-branch" size="0.75rem" />
|
||||
</button>
|
||||
</Tip>
|
||||
<WorktreeDialog onOpenChange={setOpen} onStarted={onStarted} open={open} repoPath={repoPath} />
|
||||
</>
|
||||
)
|
||||
|
|
|
|||
120
apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx
Normal file
120
apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx
Normal file
|
|
@ -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
|
||||
// <DropdownMenuTrigger asChild>{children}</DropdownMenuTrigger>
|
||||
// with the caller wrapping ITS children in <Tip>. 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 | string>(null),
|
||||
$selectedStoredSessionId: atom<null | string>(null),
|
||||
$sessions: atom<unknown[]>([]),
|
||||
sessionMatchesStoredId: vi.fn(() => false),
|
||||
sessionPinId: vi.fn((s: { id: string }) => s.id),
|
||||
setSessions: vi.fn()
|
||||
}))
|
||||
vi.mock('@/store/session-color', () => ({
|
||||
$sessionColorOverrides: atom<Record<string, string>>({}),
|
||||
setSessionColorOverride: vi.fn()
|
||||
}))
|
||||
vi.mock('@/store/session-states', () => ({
|
||||
$sessionTiles: atom<unknown[]>([]),
|
||||
openSessionTile: vi.fn()
|
||||
}))
|
||||
vi.mock('@/store/windows', () => ({
|
||||
canOpenSessionWindow: () => false,
|
||||
openSessionInNewWindow: vi.fn()
|
||||
}))
|
||||
|
||||
function renderMenu() {
|
||||
return render(
|
||||
<SessionActionsMenu sessionId="s1" title="My session" tooltip="Actions for My session">
|
||||
<button aria-label="Actions for My session" type="button">
|
||||
⋮
|
||||
</button>
|
||||
</SessionActionsMenu>
|
||||
)
|
||||
}
|
||||
|
||||
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()
|
||||
})
|
||||
})
|
||||
|
|
@ -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<React.ComponentProps<typeof DropdownMenuContent>, '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 (
|
||||
<>
|
||||
<DropdownMenu onOpenChange={setOpen} open={open}>
|
||||
<DropdownMenuTrigger asChild>{children}</DropdownMenuTrigger>
|
||||
<Tip label={tooltip}>
|
||||
<DropdownMenuTrigger asChild>{children}</DropdownMenuTrigger>
|
||||
</Tip>
|
||||
<DropdownMenuContent
|
||||
align={align}
|
||||
aria-label={t.sidebar.row.actionsFor(actions.title)}
|
||||
|
|
|
|||
199
apps/desktop/src/app/chat/sidebar/session-row.test.tsx
Normal file
199
apps/desktop/src/app/chat/sidebar/session-row.test.tsx
Normal file
|
|
@ -0,0 +1,199 @@
|
|||
import type * as React from 'react'
|
||||
|
||||
import { atom } from 'nanostores'
|
||||
import { cleanup, render, screen } from '@testing-library/react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { SessionInfo } from '@/hermes'
|
||||
import type * as ComposerStatusStore from '@/store/composer-status'
|
||||
import type * as SessionStore from '@/store/session'
|
||||
import type * as SessionStatesStore from '@/store/session-states'
|
||||
import type * as WindowsStore from '@/store/windows'
|
||||
|
||||
import { SidebarSessionRow } from './session-row'
|
||||
|
||||
afterEach(cleanup)
|
||||
|
||||
vi.mock('@/i18n', () => ({
|
||||
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<typeof ComposerStatusStore>()
|
||||
|
||||
return { ...actual, $backgroundRunningSessionIds: atom<string[]>([]) }
|
||||
})
|
||||
vi.mock('@/store/session', async importOriginal => {
|
||||
const actual = await importOriginal<typeof SessionStore>()
|
||||
|
||||
return { ...actual, $unreadFinishedSessionIds: atom<string[]>([]) }
|
||||
})
|
||||
vi.mock('@/store/session-states', async importOriginal => {
|
||||
const actual = await importOriginal<typeof SessionStatesStore>()
|
||||
|
||||
return {
|
||||
...actual,
|
||||
$attentionSessionIds: atom<string[]>([]),
|
||||
$stalledSessionIds: atom<string[]>([]),
|
||||
openSessionTile: vi.fn()
|
||||
}
|
||||
})
|
||||
vi.mock('@/store/windows', async importOriginal => {
|
||||
const actual = await importOriginal<typeof WindowsStore>()
|
||||
|
||||
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 }) => (
|
||||
<div data-testid="session-actions-menu" data-tooltip={tooltip}>
|
||||
{children}
|
||||
</div>
|
||||
),
|
||||
SessionContextMenu: ({ children }: { children: React.ReactNode }) => <>{children}</>
|
||||
}))
|
||||
|
||||
vi.mock('./use-profile-prewarm', () => ({
|
||||
useProfilePrewarm: () => ({ cancelPrewarm: vi.fn(), startPrewarm: vi.fn() })
|
||||
}))
|
||||
|
||||
function makeSession(overrides: Partial<SessionInfo> & { 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(
|
||||
<SidebarSessionRow
|
||||
isPinned={false}
|
||||
isSelected={false}
|
||||
isWorking={false}
|
||||
onArchive={noop}
|
||||
onDelete={noop}
|
||||
onPin={noop}
|
||||
onResume={noop}
|
||||
session={makeSession({ title: 'Hermes doctor health check results' })}
|
||||
/>
|
||||
)
|
||||
|
||||
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(
|
||||
<SidebarSessionRow
|
||||
isPinned={false}
|
||||
isSelected={false}
|
||||
isWorking={false}
|
||||
onArchive={noop}
|
||||
onDelete={noop}
|
||||
onPin={noop}
|
||||
onResume={noop}
|
||||
session={makeSession({ title: 'Local session' })}
|
||||
/>
|
||||
)
|
||||
|
||||
// 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 <i>, not <span>, 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(
|
||||
<SidebarSessionRow
|
||||
isPinned={false}
|
||||
isSelected={false}
|
||||
isWorking={false}
|
||||
onArchive={noop}
|
||||
onDelete={noop}
|
||||
onPin={noop}
|
||||
onResume={noop}
|
||||
session={makeSession({
|
||||
handoff_platform: 'telegram',
|
||||
handoff_state: 'active',
|
||||
title: 'Continued from Telegram'
|
||||
})}
|
||||
/>
|
||||
)
|
||||
|
||||
// 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()
|
||||
})
|
||||
})
|
||||
|
|
@ -133,6 +133,7 @@ export function SidebarSessionRow({
|
|||
profile={session.profile}
|
||||
sessionId={session.id}
|
||||
title={title}
|
||||
tooltip={r.actionsFor(title)}
|
||||
>
|
||||
<Button
|
||||
aria-label={r.actionsFor(title)}
|
||||
|
|
|
|||
|
|
@ -12,7 +12,8 @@ import {
|
|||
SiWechat,
|
||||
SiWhatsapp
|
||||
} from '@icons-pack/react-simple-icons'
|
||||
import type { ComponentType, SVGProps } from 'react'
|
||||
import type { ComponentPropsWithoutRef, ComponentType, SVGProps } from 'react'
|
||||
import { forwardRef } from 'react'
|
||||
|
||||
import { Globe, Link as LinkIcon, MessageSquareText } from '@/lib/icons'
|
||||
import { cn } from '@/lib/utils'
|
||||
|
|
@ -54,13 +55,20 @@ const PLATFORM_ICONS: Record<string, PlatformIconSpec> = {
|
|||
yuanbao: { Icon: SiBilibili, color: '#FB7299', kind: 'brand' }
|
||||
}
|
||||
|
||||
interface PlatformAvatarProps {
|
||||
interface PlatformAvatarProps extends Omit<ComponentPropsWithoutRef<'span'>, 'children'> {
|
||||
platformId: string
|
||||
platformName: string
|
||||
className?: string
|
||||
}
|
||||
|
||||
export function PlatformAvatar({ className, platformId, platformName }: PlatformAvatarProps) {
|
||||
// forwardRef + spreading ...rest is required so a wrapping <Tip> (Radix
|
||||
// Tooltip's `asChild`) can actually attach its trigger: asChild clones this
|
||||
// component and injects a ref plus pointer/focus/aria handlers onto it. A
|
||||
// plain function component with no ref/rest forwarding drops all of that
|
||||
// silently — the tooltip renders but never opens (#67500).
|
||||
export const PlatformAvatar = forwardRef<HTMLSpanElement, PlatformAvatarProps>(function PlatformAvatar(
|
||||
{ className, platformId, platformName, style, ...rest },
|
||||
ref
|
||||
) {
|
||||
const spec = PLATFORM_ICONS[platformId]
|
||||
|
||||
const baseClass = cn(
|
||||
|
|
@ -70,7 +78,13 @@ export function PlatformAvatar({ className, platformId, platformName }: Platform
|
|||
|
||||
if (!spec) {
|
||||
return (
|
||||
<span aria-hidden="true" className={cn(baseClass, 'bg-(--ui-bg-tertiary) text-(--ui-text-tertiary)')}>
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className={cn(baseClass, 'bg-(--ui-bg-tertiary) text-(--ui-text-tertiary)')}
|
||||
ref={ref}
|
||||
style={style}
|
||||
{...rest}
|
||||
>
|
||||
{platformName.charAt(0).toUpperCase()}
|
||||
</span>
|
||||
)
|
||||
|
|
@ -82,14 +96,17 @@ export function PlatformAvatar({ className, platformId, platformName }: Platform
|
|||
<span
|
||||
aria-hidden="true"
|
||||
className={baseClass}
|
||||
ref={ref}
|
||||
style={{
|
||||
// 16% tint of the brand color so the glyph reads against any surface
|
||||
// without the avatar dominating the row.
|
||||
backgroundColor: `color-mix(in srgb, ${color} 16%, transparent)`,
|
||||
color
|
||||
color,
|
||||
...style
|
||||
}}
|
||||
{...rest}
|
||||
>
|
||||
{Icon ? <Icon className="size-3.5" /> : spec.monogram || platformName.charAt(0).toUpperCase()}
|
||||
</span>
|
||||
)
|
||||
}
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue