diff --git a/src/renderer/src/components/activity/ActivityThreadOptionsMenu.test.tsx b/src/renderer/src/components/activity/ActivityThreadOptionsMenu.test.tsx index 544101f7ff2..2f905623efa 100644 --- a/src/renderer/src/components/activity/ActivityThreadOptionsMenu.test.tsx +++ b/src/renderer/src/components/activity/ActivityThreadOptionsMenu.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment happy-dom -import { act, type ReactElement } from 'react' +import { act, StrictMode, type ReactElement } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -150,23 +150,40 @@ describe('ActivityThreadOptionsMenu', () => { } }) - it('opens without recursively updating composed Radix trigger refs', async () => { - await act(async () => { - root.render() - }) + it('keeps the focused menu button described through repeated opens and rerenders', async () => { + for (let cycle = 0; cycle < 3; cycle += 1) { + await act(async () => { + root.render( + + + + ) + }) + const trigger = container.querySelector( + 'button[aria-label="Thread list options"]' + ) + if (!trigger) { + throw new Error('Thread list options button missing') + } - const trigger = container.querySelector( - 'button[aria-label="Thread list options"]' - ) - - expect(trigger).not.toBeNull() - expect(trigger?.parentElement?.tagName).toBe('SPAN') - - await act(async () => { - trigger?.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' })) - }) - - expect(document.body.textContent).toContain('Compact mode') + await act(async () => trigger.focus()) + await vi.waitFor(() => { + const descriptionId = trigger.getAttribute('aria-describedby') + expect(descriptionId).toBeTruthy() + expect(document.getElementById(descriptionId ?? '')?.textContent).toBe('Activity options') + }) + await act(async () => { + trigger.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Enter' })) + }) + expect(document.body.textContent).toContain('Compact mode') + await act(async () => { + document + .querySelector('[role="menu"]') + ?.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: 'Escape' })) + trigger.blur() + }) + await vi.waitFor(() => expect(document.querySelector('[role="menu"]')).toBeNull()) + } }) it.each(['removed host', 'single project', 'stale project'] as const)( diff --git a/src/renderer/src/components/activity/activity-thread-options-menu.tsx b/src/renderer/src/components/activity/activity-thread-options-menu.tsx index 0ee6dfa5689..f3a39aef0bb 100644 --- a/src/renderer/src/components/activity/activity-thread-options-menu.tsx +++ b/src/renderer/src/components/activity/activity-thread-options-menu.tsx @@ -99,21 +99,18 @@ export function ActivityThreadOptionsMenu({ - {/* Why: keep Tooltip and Dropdown from composing refs onto the same button (Radix setRef crash loop). */} - - - - - + + + {activeFilterCount > 0 diff --git a/src/renderer/src/components/right-sidebar/source-control/commit/commit-action-menu.tsx b/src/renderer/src/components/right-sidebar/source-control/commit/commit-action-menu.tsx index 84e4f221c97..ec9dd19be1b 100644 --- a/src/renderer/src/components/right-sidebar/source-control/commit/commit-action-menu.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/commit/commit-action-menu.tsx @@ -55,27 +55,25 @@ export function CommitActionMenu({ - - - - - + + + {moreCommitAndRemoteActionsLabel} diff --git a/src/renderer/src/components/right-sidebar/source-control/panel/header-overflow-menu.tsx b/src/renderer/src/components/right-sidebar/source-control/panel/header-overflow-menu.tsx index 3d5e9b6ac80..0febb01cc49 100644 --- a/src/renderer/src/components/right-sidebar/source-control/panel/header-overflow-menu.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/panel/header-overflow-menu.tsx @@ -40,22 +40,20 @@ export function SourceControlHeaderOverflowMenu({ - - - - - + + + {translate( diff --git a/tests/e2e/tooltip-dropdown-trigger-stability.spec.ts b/tests/e2e/tooltip-dropdown-trigger-stability.spec.ts new file mode 100644 index 00000000000..2999b622905 --- /dev/null +++ b/tests/e2e/tooltip-dropdown-trigger-stability.spec.ts @@ -0,0 +1,112 @@ +import type { Locator, Page } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' + +async function expectTooltipClosed(page: Page, text: string): Promise { + const content = page.locator('[data-slot="tooltip-content"]').filter({ hasText: text }) + await expect + .poll(() => + content.evaluateAll((elements) => + elements.every((element) => element.getAttribute('data-state') === 'closed') + ) + ) + .toBe(true) + await expect(content).toHaveCount(0) +} + +async function exerciseMenuTrigger(page: Page, button: Locator, text: string): Promise { + await page.mouse.move(400, 400) + await button.blur() + await expectTooltipClosed(page, text) + await button.hover() + await expect(page.getByRole('tooltip').filter({ hasText: text })).toBeVisible() + await page.mouse.move(400, 400) + await expectTooltipClosed(page, text) + + await button.focus() + await expect(button).toBeFocused() + await expect(button).toHaveAccessibleDescription(text) + await expect(page.getByRole('tooltip').filter({ hasText: text })).toBeVisible() + + await button.blur() + await expectTooltipClosed(page, text) + + for (const input of ['keyboard', 'pointer'] as const) { + if (input === 'keyboard') { + await button.focus() + await page.keyboard.press('Enter') + } else { + await button.click() + } + const menu = page.getByRole('menu') + await expect(menu).toBeVisible() + await expect + .poll(() => menu.evaluate((element) => element.contains(document.activeElement))) + .toBe(true) + // A menu item's tooltip can consume the first Escape. + await page.keyboard.press('Escape') + if ( + await menu.evaluateAll((elements) => + elements.some((element) => element.getAttribute('data-state') === 'open') + ) + ) { + await expect(page.locator('[data-slot="tooltip-content"]')).toHaveCount(0) + await page.keyboard.press('Escape') + } + await expect(page.getByRole('menu')).toBeHidden() + await expect(button).toBeFocused() + } +} + +for (const surface of [ + { + name: 'source control header', + openButton: /^Source Control/, + closeButton: /^Explorer/, + triggerLabel: 'More source control actions', + tooltip: 'More source control actions' + }, + { + name: 'commit actions', + openButton: /^Source Control/, + closeButton: /^Explorer/, + triggerLabel: 'More commit and remote actions', + tooltip: 'More commit and remote actions' + }, + { + name: 'activity options', + openButton: 'View activity', + closeButton: 'Turn off activity view', + triggerLabel: 'Thread list options', + tooltip: 'Activity options' + } +] as const) { + test(`${surface.name} menu button retains tooltip descriptions through focus, hover and remounts`, async ({ + orcaPage: page + }) => { + await waitForSessionReady(page) + await waitForActiveWorktree(page) + const errors: string[] = [] + const captureError = (error: Error): void => { + errors.push(error.message) + } + page.on('pageerror', captureError) + try { + for (let cycle = 0; cycle < 2; cycle += 1) { + await page.getByRole('button', { name: surface.openButton, exact: true }).click() + await exerciseMenuTrigger( + page, + page.getByRole('button', { name: surface.triggerLabel, exact: true }), + surface.tooltip + ) + await page.getByRole('button', { name: surface.closeButton, exact: true }).click() + await expect( + page.getByRole('button', { name: surface.triggerLabel, exact: true }) + ).toHaveCount(0) + expect(errors).toEqual([]) + } + } finally { + page.off('pageerror', captureError) + } + }) +}