mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 08:37:49 +00:00
fix(ui): use focusable tooltip menu triggers (#9595)
* fix(ui): use focusable tooltip menu triggers * test(tooltips): give each menu surface its own remount check Keep all hover, focus, description, keyboard, pointer and two-remount assertions while avoiding a shared 120-second budget across three surfaces. The failed CI trace passed five full exercises before global teardown. --------- Co-authored-by: Yuris Auzins <zuz666@users.noreply.github.com> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
co-authored by
Yuris Auzins
Neil
parent
2b33fed6a5
commit
8ef0e99a7c
@@ -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(<Harness />)
|
||||
})
|
||||
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(
|
||||
<StrictMode>
|
||||
<Harness compactMode={cycle % 2 === 1} />
|
||||
</StrictMode>
|
||||
)
|
||||
})
|
||||
const trigger = container.querySelector<HTMLButtonElement>(
|
||||
'button[aria-label="Thread list options"]'
|
||||
)
|
||||
if (!trigger) {
|
||||
throw new Error('Thread list options button missing')
|
||||
}
|
||||
|
||||
const trigger = container.querySelector<HTMLButtonElement>(
|
||||
'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)(
|
||||
|
||||
@@ -99,21 +99,18 @@ export function ActivityThreadOptionsMenu({
|
||||
<DropdownMenu>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
{/* Why: keep Tooltip and Dropdown from composing refs onto the same button (Radix setRef crash loop). */}
|
||||
<span className="inline-flex shrink-0">
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
className="relative text-muted-foreground"
|
||||
aria-label={optionsLabel}
|
||||
>
|
||||
<SlidersHorizontal className="size-3.5" strokeWidth={2.25} />
|
||||
<OptionsFilterCountBadge count={activeFilterCount} />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
</span>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
className="relative text-muted-foreground"
|
||||
aria-label={optionsLabel}
|
||||
>
|
||||
<SlidersHorizontal className="size-3.5" strokeWidth={2.25} />
|
||||
<OptionsFilterCountBadge count={activeFilterCount} />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" sideOffset={6}>
|
||||
{activeFilterCount > 0
|
||||
|
||||
+19
-21
@@ -55,27 +55,25 @@ export function CommitActionMenu({
|
||||
<DropdownMenu>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span className="inline-flex shrink-0">
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
size="xs"
|
||||
className={cn(
|
||||
'rounded-l-none border-l border-border px-1.5 shrink-0',
|
||||
// Why: mirror the primary's disabled dimming for a unified look, but the chevron stays clickable (its push/fetch/pull stay valid when Commit is disabled).
|
||||
primaryAction.disabled && 'opacity-50'
|
||||
)}
|
||||
aria-label={moreCommitAndRemoteActionsLabel}
|
||||
>
|
||||
{showChevronSpinner ? (
|
||||
<Loader2 className="size-3.5 animate-spin" />
|
||||
) : (
|
||||
<ChevronDown className="size-3.5" />
|
||||
)}
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
</span>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
size="xs"
|
||||
className={cn(
|
||||
'rounded-l-none border-l border-border px-1.5 shrink-0',
|
||||
// Why: mirror the primary's disabled dimming for a unified look, but the chevron stays clickable (its push/fetch/pull stay valid when Commit is disabled).
|
||||
primaryAction.disabled && 'opacity-50'
|
||||
)}
|
||||
aria-label={moreCommitAndRemoteActionsLabel}
|
||||
>
|
||||
{showChevronSpinner ? (
|
||||
<Loader2 className="size-3.5 animate-spin" />
|
||||
) : (
|
||||
<ChevronDown className="size-3.5" />
|
||||
)}
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="top" sideOffset={6}>
|
||||
{moreCommitAndRemoteActionsLabel}
|
||||
|
||||
+14
-16
@@ -40,22 +40,20 @@ export function SourceControlHeaderOverflowMenu({
|
||||
<DropdownMenu>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span className="inline-flex shrink-0">
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
className="size-7 text-muted-foreground hover:text-foreground"
|
||||
aria-label={translate(
|
||||
'auto.components.right.sidebar.SourceControl.f71c4a8d90',
|
||||
'More source control actions'
|
||||
)}
|
||||
>
|
||||
<MoreHorizontal className="size-3.5" />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
</span>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
className="size-7 text-muted-foreground hover:text-foreground"
|
||||
aria-label={translate(
|
||||
'auto.components.right.sidebar.SourceControl.f71c4a8d90',
|
||||
'More source control actions'
|
||||
)}
|
||||
>
|
||||
<MoreHorizontal className="size-3.5" />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" sideOffset={6}>
|
||||
{translate(
|
||||
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user