mirror of
https://github.com/stablyai/orca.git
synced 2026-10-12 16:03:03 +00:00
fix(ui): unmount project selector before dialog handoff
This commit is contained in:
Jinwoo-H
@@ -0,0 +1,152 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import React, { act, useRef, useState } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { Dialog, DialogContent, DialogTitle } from '@/components/ui/dialog'
|
||||
import ProjectCombobox from './ProjectCombobox'
|
||||
|
||||
vi.mock('./use-recent-project-ids', () => ({ useRecentProjectIds: () => [] }))
|
||||
|
||||
function Composer({ populated }: { populated: boolean }): React.JSX.Element {
|
||||
const [adding, setAdding] = useState(false)
|
||||
const nameRef = useRef<HTMLInputElement>(null)
|
||||
return (
|
||||
<Dialog open>
|
||||
<DialogContent aria-describedby={undefined}>
|
||||
<DialogTitle>New workspace</DialogTitle>
|
||||
<ProjectCombobox
|
||||
options={
|
||||
populated
|
||||
? [
|
||||
{
|
||||
kind: 'project',
|
||||
id: 'project:existing',
|
||||
projectId: 'project:existing',
|
||||
displayName: 'Existing project',
|
||||
badgeColor: 'var(--muted)',
|
||||
detail: 'Project'
|
||||
}
|
||||
]
|
||||
: []
|
||||
}
|
||||
value={populated ? 'project:existing' : null}
|
||||
onValueChange={vi.fn()}
|
||||
onAddProject={() => setAdding(true)}
|
||||
/>
|
||||
<input ref={nameRef} aria-label="Workspace name" />
|
||||
<Dialog open={adding} onOpenChange={setAdding}>
|
||||
<DialogContent
|
||||
aria-describedby={undefined}
|
||||
onCloseAutoFocus={(event) => {
|
||||
event.preventDefault()
|
||||
nameRef.current?.focus()
|
||||
}}
|
||||
>
|
||||
<DialogTitle>Add a project</DialogTitle>
|
||||
<input aria-label="Project path" />
|
||||
<button onClick={() => setAdding(false)}>Cancel</button>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
)
|
||||
}
|
||||
|
||||
let container: HTMLDivElement
|
||||
let root: Root
|
||||
|
||||
beforeEach(() => {
|
||||
// Radix Presence retains closing content when the production exit animation runs.
|
||||
const getStyle = window.getComputedStyle.bind(window)
|
||||
vi.spyOn(window, 'getComputedStyle').mockImplementation((element, ...args) => {
|
||||
const style = getStyle(element, ...args)
|
||||
if (element.getAttribute('data-slot') === 'popover-content') {
|
||||
return new Proxy(style, {
|
||||
get: (target, property) =>
|
||||
property === 'animationName'
|
||||
? element.getAttribute('data-state') === 'closed'
|
||||
? 'exit'
|
||||
: 'enter'
|
||||
: Reflect.get(target, property)
|
||||
})
|
||||
}
|
||||
return style
|
||||
})
|
||||
container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
root = createRoot(container)
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await act(async () => root.unmount())
|
||||
container.remove()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
function projectField(): HTMLInputElement {
|
||||
return document.querySelector<HTMLInputElement>('[role="combobox"][aria-label="Project"]')!
|
||||
}
|
||||
|
||||
function addOption(): HTMLElement {
|
||||
return Array.from(document.querySelectorAll<HTMLElement>('[role="option"]')).find((option) =>
|
||||
option.textContent?.includes('Add a new project')
|
||||
)!
|
||||
}
|
||||
|
||||
describe.each([false, true])(
|
||||
'project selector to nested dialog handoff (populated: %s)',
|
||||
(populated) => {
|
||||
it.each(['mouse', 'keyboard'] as const)(
|
||||
'removes the selector before the creation dialog is active via %s',
|
||||
async (input) => {
|
||||
await act(async () => root.render(<Composer populated={populated} />))
|
||||
await act(async () => projectField().click())
|
||||
expect(document.querySelector('[role="listbox"]')).not.toBeNull()
|
||||
|
||||
await act(async () => {
|
||||
if (input === 'mouse') {
|
||||
addOption().dispatchEvent(
|
||||
new MouseEvent('mousedown', { bubbles: true, cancelable: true })
|
||||
)
|
||||
addOption().click()
|
||||
} else if (populated) {
|
||||
projectField().dispatchEvent(
|
||||
new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true, cancelable: true })
|
||||
)
|
||||
}
|
||||
})
|
||||
if (input === 'keyboard') {
|
||||
await act(async () => {
|
||||
projectField().dispatchEvent(
|
||||
new KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true })
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
expect(document.querySelector('[role="listbox"]')).toBeNull()
|
||||
expect(document.querySelector('[data-slot="popover-content"]')).toBeNull()
|
||||
expect(projectField().getAttribute('aria-expanded')).toBe('false')
|
||||
expect(projectField().hasAttribute('aria-activedescendant')).toBe(false)
|
||||
const path = document.querySelector<HTMLInputElement>('[aria-label="Project path"]')!
|
||||
expect(document.activeElement).toBe(path)
|
||||
expect(path.closest('[aria-hidden="true"]')).toBeNull()
|
||||
expect(projectField().closest('[aria-hidden="true"]')).not.toBeNull()
|
||||
|
||||
await act(async () => {
|
||||
Array.from(document.querySelectorAll('button'))
|
||||
.find((b) => b.textContent === 'Cancel')!
|
||||
.click()
|
||||
})
|
||||
await vi.waitFor(() =>
|
||||
expect(document.activeElement).toBe(
|
||||
document.querySelector('[aria-label="Workspace name"]')
|
||||
)
|
||||
)
|
||||
await act(async () => projectField().click())
|
||||
expect(projectField().getAttribute('aria-expanded')).toBe('true')
|
||||
expect(document.querySelector('[role="listbox"]')).not.toBeNull()
|
||||
}
|
||||
)
|
||||
}
|
||||
)
|
||||
@@ -72,7 +72,7 @@ function field(): HTMLInputElement {
|
||||
|
||||
function openList(): void {
|
||||
act(() => {
|
||||
field().dispatchEvent(new FocusEvent('focus', { bubbles: true }))
|
||||
field().focus()
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -229,129 +229,132 @@ export default function ProjectCombobox({
|
||||
</button>
|
||||
</div>
|
||||
</PopoverAnchor>
|
||||
<PopoverContent
|
||||
align="start"
|
||||
sideOffset={4}
|
||||
// Why opaque + no fade: this popover lands directly on the composer
|
||||
// dialog, not the app canvas. The shared surface is translucent and
|
||||
// fades 0→1, so mid-animation the Name field underneath reads straight
|
||||
// through the list — two layers at once, which is the "double flash".
|
||||
// An opaque surface that zooms without fading resolves it. (`bg-popover`
|
||||
// alone loses to the primitive's arbitrary-value background, hence the
|
||||
// matching arbitrary form.) Every other caller keeps the blur and fade.
|
||||
className={cn(
|
||||
'flex w-[var(--radix-popover-trigger-width)] min-w-[17rem] flex-col p-0',
|
||||
COMBOBOX_POPOVER_SURFACE
|
||||
)}
|
||||
// Focus stays in the field — it's the search box — so the popover must
|
||||
// not steal it on open, nor yank it back on close after a pick.
|
||||
onOpenAutoFocus={(event) => event.preventDefault()}
|
||||
onCloseAutoFocus={(event) => event.preventDefault()}
|
||||
// Why: the field lives in the anchor, not inside the content, so Radix
|
||||
// sees a focus/pointer event "outside" the layer and dismisses it the
|
||||
// instant you tab in. Keep the layer open whenever the interaction is
|
||||
// within this control; genuine outside events still close it.
|
||||
onFocusOutside={(event) => {
|
||||
if (isWithinComboboxRoot(event.target, ROOT_ATTRIBUTE)) {
|
||||
event.preventDefault()
|
||||
}
|
||||
}}
|
||||
onInteractOutside={(event) => {
|
||||
if (isWithinComboboxRoot(event.target, ROOT_ATTRIBUTE)) {
|
||||
event.preventDefault()
|
||||
}
|
||||
}}
|
||||
>
|
||||
{/* The listbox wraps a scrolling pane plus the pinned Add row, so both
|
||||
stay `option` children of one listbox. */}
|
||||
<div
|
||||
id={listId}
|
||||
role="listbox"
|
||||
aria-label={translate(
|
||||
'auto.components.new.workspace.ProjectCombobox.listLabel',
|
||||
'Projects'
|
||||
{/* Closing must remove the selector before a nested dialog takes focus. */}
|
||||
{open ? (
|
||||
<PopoverContent
|
||||
align="start"
|
||||
sideOffset={4}
|
||||
// Why opaque + no fade: this popover lands directly on the composer
|
||||
// dialog, not the app canvas. The shared surface is translucent and
|
||||
// fades 0→1, so mid-animation the Name field underneath reads straight
|
||||
// through the list — two layers at once, which is the "double flash".
|
||||
// An opaque surface that zooms without fading resolves it. (`bg-popover`
|
||||
// alone loses to the primitive's arbitrary-value background, hence the
|
||||
// matching arbitrary form.) Every other caller keeps the blur and fade.
|
||||
className={cn(
|
||||
'flex w-[var(--radix-popover-trigger-width)] min-w-[17rem] flex-col p-0',
|
||||
COMBOBOX_POPOVER_SURFACE
|
||||
)}
|
||||
className="flex min-h-0 flex-col"
|
||||
// Focus stays in the field — it's the search box — so the popover must
|
||||
// not steal it on open, nor yank it back on close after a pick.
|
||||
onOpenAutoFocus={(event) => event.preventDefault()}
|
||||
onCloseAutoFocus={(event) => event.preventDefault()}
|
||||
// Why: the field lives in the anchor, not inside the content, so Radix
|
||||
// sees a focus/pointer event "outside" the layer and dismisses it the
|
||||
// instant you tab in. Keep the layer open whenever the interaction is
|
||||
// within this control; genuine outside events still close it.
|
||||
onFocusOutside={(event) => {
|
||||
if (isWithinComboboxRoot(event.target, ROOT_ATTRIBUTE)) {
|
||||
event.preventDefault()
|
||||
}
|
||||
}}
|
||||
onInteractOutside={(event) => {
|
||||
if (isWithinComboboxRoot(event.target, ROOT_ATTRIBUTE)) {
|
||||
event.preventDefault()
|
||||
}
|
||||
}}
|
||||
>
|
||||
{/* Why: `presentation` — this element exists to scroll, and an
|
||||
{/* The listbox wraps a scrolling pane plus the pinned Add row, so both
|
||||
stay `option` children of one listbox. */}
|
||||
<div
|
||||
id={listId}
|
||||
role="listbox"
|
||||
aria-label={translate(
|
||||
'auto.components.new.workspace.ProjectCombobox.listLabel',
|
||||
'Projects'
|
||||
)}
|
||||
className="flex min-h-0 flex-col"
|
||||
>
|
||||
{/* Why: `presentation` — this element exists to scroll, and an
|
||||
unroled div between a listbox and its options breaks the
|
||||
ownership relationship assistive tech relies on. */}
|
||||
<div
|
||||
ref={setListNode}
|
||||
role="presentation"
|
||||
className="max-h-72 min-h-0 flex-1 overflow-y-auto p-1 scrollbar-sleek"
|
||||
>
|
||||
{matches.length === 0 ? (
|
||||
// Why: row-height rather than a tall centred block — a 60px panel
|
||||
// next to 32px rows reads as a different kind of surface and
|
||||
// makes an empty result feel like an error.
|
||||
<p className="flex h-8 items-center justify-center px-2 text-sm text-muted-foreground">
|
||||
{options.length === 0
|
||||
? translate(
|
||||
'auto.components.new.workspace.ProjectCombobox.noProjects',
|
||||
'No projects yet.'
|
||||
)
|
||||
: translate(
|
||||
'auto.components.new.workspace.ProjectCombobox.empty',
|
||||
'No projects match your search.'
|
||||
)}
|
||||
</p>
|
||||
) : null}
|
||||
{sections.map((section) => (
|
||||
// Why: `role="group"` — a bare div between a listbox and its
|
||||
// options breaks the ownership relationship for screen readers,
|
||||
// which is the only thing that makes the headings announceable.
|
||||
<div key={section.key} role="group" aria-label={section.heading ?? undefined}>
|
||||
{section.heading ? (
|
||||
<div
|
||||
aria-hidden="true"
|
||||
className="px-2 pt-2.5 pb-1 text-[11px] font-semibold tracking-[0.05em] text-muted-foreground uppercase"
|
||||
>
|
||||
{section.heading}
|
||||
</div>
|
||||
) : null}
|
||||
{section.items.map((scored) => (
|
||||
<ProjectOptionRow
|
||||
key={scored.option.id}
|
||||
option={scored.option}
|
||||
nameHits={scored.nameHits}
|
||||
detailHits={scored.detailHits}
|
||||
armed={armedKey === scored.option.id}
|
||||
current={scored.option.id === value}
|
||||
ambiguous={ambiguous.has(scored.option.id)}
|
||||
optionId={armedKey === scored.option.id ? `${listId}-armed` : undefined}
|
||||
onArm={() => arm(scored.option.id)}
|
||||
onCommit={() => commit(scored.option.id)}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
{onAddProject ? (
|
||||
<div
|
||||
role="option"
|
||||
id={armedKey === ADD_PROJECT_KEY ? `${listId}-armed` : undefined}
|
||||
aria-selected={armedKey === ADD_PROJECT_KEY}
|
||||
data-armed={armedKey === ADD_PROJECT_KEY || undefined}
|
||||
onMouseDown={(event) => event.preventDefault()}
|
||||
onMouseMove={() => arm(ADD_PROJECT_KEY)}
|
||||
onClick={() => commit(ADD_PROJECT_KEY)}
|
||||
className={cn(
|
||||
'flex h-9 shrink-0 cursor-default items-center gap-2 border-t border-border px-2 text-sm',
|
||||
armedKey === ADD_PROJECT_KEY && 'bg-accent text-accent-foreground'
|
||||
)}
|
||||
ref={setListNode}
|
||||
role="presentation"
|
||||
className="max-h-72 min-h-0 flex-1 overflow-y-auto p-1 scrollbar-sleek"
|
||||
>
|
||||
<FolderPlus className="size-3.5 shrink-0 text-muted-foreground" />
|
||||
<span className="truncate">
|
||||
{translate(
|
||||
'auto.components.new.workspace.ProjectCombobox.addProject',
|
||||
'Add a new project'
|
||||
)}
|
||||
</span>
|
||||
{matches.length === 0 ? (
|
||||
// Why: row-height rather than a tall centred block — a 60px panel
|
||||
// next to 32px rows reads as a different kind of surface and
|
||||
// makes an empty result feel like an error.
|
||||
<p className="flex h-8 items-center justify-center px-2 text-sm text-muted-foreground">
|
||||
{options.length === 0
|
||||
? translate(
|
||||
'auto.components.new.workspace.ProjectCombobox.noProjects',
|
||||
'No projects yet.'
|
||||
)
|
||||
: translate(
|
||||
'auto.components.new.workspace.ProjectCombobox.empty',
|
||||
'No projects match your search.'
|
||||
)}
|
||||
</p>
|
||||
) : null}
|
||||
{sections.map((section) => (
|
||||
// Why: `role="group"` — a bare div between a listbox and its
|
||||
// options breaks the ownership relationship for screen readers,
|
||||
// which is the only thing that makes the headings announceable.
|
||||
<div key={section.key} role="group" aria-label={section.heading ?? undefined}>
|
||||
{section.heading ? (
|
||||
<div
|
||||
aria-hidden="true"
|
||||
className="px-2 pt-2.5 pb-1 text-[11px] font-semibold tracking-[0.05em] text-muted-foreground uppercase"
|
||||
>
|
||||
{section.heading}
|
||||
</div>
|
||||
) : null}
|
||||
{section.items.map((scored) => (
|
||||
<ProjectOptionRow
|
||||
key={scored.option.id}
|
||||
option={scored.option}
|
||||
nameHits={scored.nameHits}
|
||||
detailHits={scored.detailHits}
|
||||
armed={armedKey === scored.option.id}
|
||||
current={scored.option.id === value}
|
||||
ambiguous={ambiguous.has(scored.option.id)}
|
||||
optionId={armedKey === scored.option.id ? `${listId}-armed` : undefined}
|
||||
onArm={() => arm(scored.option.id)}
|
||||
onCommit={() => commit(scored.option.id)}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
</PopoverContent>
|
||||
{onAddProject ? (
|
||||
<div
|
||||
role="option"
|
||||
id={armedKey === ADD_PROJECT_KEY ? `${listId}-armed` : undefined}
|
||||
aria-selected={armedKey === ADD_PROJECT_KEY}
|
||||
data-armed={armedKey === ADD_PROJECT_KEY || undefined}
|
||||
onMouseDown={(event) => event.preventDefault()}
|
||||
onMouseMove={() => arm(ADD_PROJECT_KEY)}
|
||||
onClick={() => commit(ADD_PROJECT_KEY)}
|
||||
className={cn(
|
||||
'flex h-9 shrink-0 cursor-default items-center gap-2 border-t border-border px-2 text-sm',
|
||||
armedKey === ADD_PROJECT_KEY && 'bg-accent text-accent-foreground'
|
||||
)}
|
||||
>
|
||||
<FolderPlus className="size-3.5 shrink-0 text-muted-foreground" />
|
||||
<span className="truncate">
|
||||
{translate(
|
||||
'auto.components.new.workspace.ProjectCombobox.addProject',
|
||||
'Add a new project'
|
||||
)}
|
||||
</span>
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
</PopoverContent>
|
||||
) : null}
|
||||
</Popover>
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user