From 1910d613ee1cfc7ffb957cae5d6a2dd95f7296fe Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 21:43:53 -0500 Subject: [PATCH] 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. --- .../sidebar/projects/project-menu.test.tsx | 24 ++++----------- .../projects/workspace-header.test.tsx | 4 +-- .../sidebar/session-actions-menu.test.tsx | 29 ++++--------------- .../src/app/chat/sidebar/session-row.test.tsx | 25 +++++----------- apps/desktop/src/app/overlays/panel.test.tsx | 5 ++-- 5 files changed, 22 insertions(+), 65 deletions(-) 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 index 9ceb3b7be385..54f23f5bcf12 100644 --- a/apps/desktop/src/app/chat/sidebar/projects/project-menu.test.tsx +++ b/apps/desktop/src/app/chat/sidebar/projects/project-menu.test.tsx @@ -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() 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 `{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). + // 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() 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 index 418db785730c..f28aa84fb54b 100644 --- a/apps/desktop/src/app/chat/sidebar/projects/workspace-header.test.tsx +++ b/apps/desktop/src/app/chat/sidebar/projects/workspace-header.test.tsx @@ -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() const button = screen.getByRole('button', { name: 'Project actions' }) - expect(tipTrigger(button)).toBeTruthy() + expect(tipTrigger(button)).toBeNull() }) }) 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 index ae4d7b6db4ef..0a257af2054b 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx @@ -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 -// {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. +// 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( - + @@ -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() diff --git a/apps/desktop/src/app/chat/sidebar/session-row.test.tsx b/apps/desktop/src/app/chat/sidebar/session-row.test.tsx index 96ddec809148..561dd5cc5d75 100644 --- a/apps/desktop/src/app/chat/sidebar/session-row.test.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-row.test.tsx @@ -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 }) => ( -
- {children} -
- ), + 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( { /> ) - 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', () => { diff --git a/apps/desktop/src/app/overlays/panel.test.tsx b/apps/desktop/src/app/overlays/panel.test.tsx index aed5d106e972..1fb7e167b22e 100644 --- a/apps/desktop/src/app/overlays/panel.test.tsx +++ b/apps/desktop/src/app/overlays/panel.test.tsx @@ -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() 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' }))