test(desktop): assert kebabs are not wrapped in Tip

Flip the #67500 structure tests that required tooltip-trigger on menu
kebabs; keep coverage that the menus still open on click.
This commit is contained in:
Brooklyn Nicholson 2026-07-27 21:43:53 -05:00
parent 26413bbf57
commit 1910d613ee
5 changed files with 22 additions and 65 deletions

View file

@ -88,30 +88,16 @@ const openTriggerMenu = (trigger: HTMLElement) => {
}
describe('ProjectMenu', () => {
it('wraps the kebab trigger in a Tip', () => {
it('does not wrap the kebab trigger in a Tip', () => {
render(<ProjectMenu isActive={false} project={project} />)
const button = screen.getByRole('button', { name: 'Project actions' })
expect(tipTrigger(button)).toBeTruthy()
expect(tipTrigger(button)).toBeNull()
})
// #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).
// When anchorRef is absent, PopoverAnchor wraps the dropdown trigger so the
// appearance popover positions against the kebab. asChild must still reach
// the real button (no non-forwarding wrappers inside the chain — #67500).
it('opens the appearance popover through the kebab trigger when anchorRef is absent', async () => {
render(<ProjectMenu isActive={false} project={project} />)

View file

@ -63,11 +63,11 @@ describe('WorkspaceShowMoreButton', () => {
})
describe('WorkspaceMenu', () => {
it('wraps the kebab trigger in a Tip', () => {
it('does not wrap 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()
expect(tipTrigger(button)).toBeNull()
})
})

View file

@ -6,16 +6,9 @@ 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.
// Exercises the real SessionActionsMenu end-to-end (no DropdownMenu mock) so
// a broken asChild composition on the kebab trigger fails here — the menu
// must still open on click.
vi.mock('@/components/pane-shell/tree/store', () => ({
closeAllTreeTabs: vi.fn(),
@ -80,7 +73,7 @@ vi.mock('@/store/windows', () => ({
function renderMenu() {
return render(
<SessionActionsMenu sessionId="s1" title="My session" tooltip="Actions for My session">
<SessionActionsMenu sessionId="s1" title="My session">
<button aria-label="Actions for My session" type="button">
</button>
@ -89,18 +82,12 @@ function renderMenu() {
}
describe('SessionActionsMenu', () => {
it('shows the tooltip label wired to the real trigger button', () => {
it('opens the dropdown on click without a tooltip on the kebab', async () => {
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' })
expect(trigger.closest('[data-slot="tooltip-trigger"]')).toBeNull()
// Radix's dropdown trigger opens on pointerdown (not on the synthetic
// 'click' fireEvent alone would dispatch), so fire the full mouse
@ -109,10 +96,6 @@ describe('SessionActionsMenu', () => {
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()

View file

@ -88,21 +88,11 @@ vi.mock('@/store/windows', async importOriginal => {
}
})
// 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.
// SessionActionsMenu open behavior is covered in session-actions-menu.test.tsx
// against the real component. Stub it here so this file stays focused on the
// row chrome (handoff avatar tip, etc.).
vi.mock('./session-actions-menu', () => ({
SessionActionsMenu: ({ children, tooltip }: { children: React.ReactNode; tooltip?: string }) => (
<div data-testid="session-actions-menu" data-tooltip={tooltip}>
{children}
</div>
),
SessionActionsMenu: ({ children }: { children: React.ReactNode }) => <>{children}</>,
SessionContextMenu: ({ children }: { children: React.ReactNode }) => <>{children}</>
}))
@ -127,7 +117,7 @@ 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', () => {
it('keeps an aria-label on the kebab without wrapping it in a Tip', () => {
render(
<SidebarSessionRow
isPinned={false}
@ -141,9 +131,8 @@ describe('SidebarSessionRow', () => {
/>
)
expect(screen.getByTestId('session-actions-menu').getAttribute('data-tooltip')).toBe(
'Actions for Hermes doctor health check results'
)
const kebab = screen.getByRole('button', { name: 'Actions for Hermes doctor health check results' })
expect(tipTrigger(kebab)).toBeNull()
})
it('does not render a handoff avatar for a locally-started session', () => {

View file

@ -15,15 +15,14 @@ describe('PanelRowMenu', () => {
cleanup()
})
it('opens its actions menu when the trigger has a tooltip', async () => {
it('opens its actions menu from the kebab without a tooltip', async () => {
const onSelect = vi.fn()
render(<PanelRowMenu items={[{ label: 'Rename', onSelect }]} />)
const trigger = screen.getByRole('button', { name: 'Actions' })
fireEvent.pointerMove(trigger, { pointerType: 'mouse' })
expect((await screen.findByRole('tooltip')).textContent).toContain('Actions')
expect(trigger.closest('[data-slot="tooltip-trigger"]')).toBeNull()
fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false, pointerType: 'mouse' })
fireEvent.click(await screen.findByRole('menuitem', { name: 'Rename' }))