diff --git a/packages/web-shell/client/components/dialogs/ApprovalModeDialog.module.css b/packages/web-shell/client/components/dialogs/ApprovalModeDialog.module.css index efb78a1511..f3a68d353f 100644 --- a/packages/web-shell/client/components/dialogs/ApprovalModeDialog.module.css +++ b/packages/web-shell/client/components/dialogs/ApprovalModeDialog.module.css @@ -7,6 +7,12 @@ padding: 6px 8px 0; } +/* Focused programmatically for keyboard nav / aria-activedescendant; selection + is shown by the roving row highlight, so suppress the container outline. */ +.list:focus { + outline: none; +} + .row { display: flex; align-items: center; @@ -27,6 +33,17 @@ background: var(--secondary); } +/* Keyboard mode: the pointer yields to the keyboard, so a cursor resting on a + row must not paint a second highlight. Only the keyboard-selected row keeps + its background. */ +.keyboardOnly .row:hover { + background: transparent; +} + +.keyboardOnly .row.selected:hover { + background: var(--secondary); +} + .selected .modeName { color: var(--agent-blue-500); } diff --git a/packages/web-shell/client/components/dialogs/ApprovalModeDialog.test.tsx b/packages/web-shell/client/components/dialogs/ApprovalModeDialog.test.tsx new file mode 100644 index 0000000000..cab4e3cf52 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/ApprovalModeDialog.test.tsx @@ -0,0 +1,103 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { I18nProvider } from '../../i18n'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = () => {}; +} + +vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ + DAEMON_APPROVAL_MODES: ['plan', 'default', 'yolo'], +})); + +const { ApprovalModeDialog } = await import('./ApprovalModeDialog'); + +let container: HTMLDivElement | null = null; +let root: Root | null = null; + +function mount(node: React.ReactNode) { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + act(() => { + root!.render({node}); + }); +} + +function rerender(node: React.ReactNode) { + act(() => { + root!.render({node}); + }); +} + +function press(key: string) { + act(() => { + window.dispatchEvent( + new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true }), + ); + }); +} + +const activeDescendant = () => + container! + .querySelector('[role="listbox"]')! + .getAttribute('aria-activedescendant'); + +afterEach(() => { + act(() => root?.unmount()); + container?.remove(); + root = null; + container = null; +}); + +describe('ApprovalModeDialog', () => { + it('opens with the highlight on the current mode and confirms on Enter', () => { + const onSelect = vi.fn(); + mount(); + + expect(activeDescendant()).toBe('mode-opt-1'); + + press('ArrowDown'); + expect(activeDescendant()).toBe('mode-opt-2'); + press('Enter'); + expect(onSelect).toHaveBeenCalledWith('yolo'); + }); + + it('binds aria-selected to the current mode, not the roving highlight', () => { + mount(); + const selected = () => + Array.from(container!.querySelectorAll('[aria-selected="true"]')); + + expect(selected()).toHaveLength(1); + expect(selected()[0].id).toBe('mode-opt-0'); + + press('ArrowDown'); + expect(selected()).toHaveLength(1); + expect(selected()[0].id).toBe('mode-opt-0'); + }); + + it('re-syncs the highlight when the current mode changes while open', () => { + mount(); + expect(activeDescendant()).toBe('mode-opt-0'); + + // Another client sharing the session flips approval mode while the dialog + // is open: the highlight (and Enter's target) must follow. + rerender(); + expect(activeDescendant()).toBe('mode-opt-2'); + }); + + it('stops following once the user has navigated', () => { + mount(); + + press('ArrowDown'); + expect(activeDescendant()).toBe('mode-opt-1'); + + // The user owns the highlight now — a mode change must not steal it. + rerender(); + expect(activeDescendant()).toBe('mode-opt-1'); + }); +}); diff --git a/packages/web-shell/client/components/dialogs/ApprovalModeDialog.tsx b/packages/web-shell/client/components/dialogs/ApprovalModeDialog.tsx index f1b42a9b68..24b13785c3 100644 --- a/packages/web-shell/client/components/dialogs/ApprovalModeDialog.tsx +++ b/packages/web-shell/client/components/dialogs/ApprovalModeDialog.tsx @@ -1,6 +1,8 @@ -import { useEffect, useRef } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { DAEMON_APPROVAL_MODES } from '@qwen-code/webui/daemon-react-sdk'; import { useI18n } from '../../i18n'; +import { useListboxKeyboard } from '../../hooks/useListboxKeyboard'; +import { dp } from './dialogStyles'; import { ModeIcon } from '../ModeIcon'; import styles from './ApprovalModeDialog.module.css'; @@ -27,32 +29,71 @@ export function ApprovalModeDialog({ description: t(`mode.desc.${id}`), })); - const selectedIdx = approvalModes.findIndex((m) => m.id === currentMode); + const currentIdx = approvalModes.findIndex((m) => m.id === currentMode); + const [activeIndex, setActiveIndex] = useState( + currentIdx >= 0 ? currentIdx : 0, + ); + + // Follow the current mode until the user first navigates: it can change + // while the dialog is open (e.g. another client sharing the session flips + // approval mode). Once the user has moved the highlight, don't steal it. + const userNavigatedRef = useRef(false); + useEffect(() => { + if (userNavigatedRef.current || currentIdx < 0) return; + setActiveIndex(currentIdx); + }, [currentIdx]); + + const moveHighlight = (index: number) => { + userNavigatedRef.current = true; + setActiveIndex(index); + }; + + const confirm = (index: number) => { + const mode = approvalModes[index]; + if (mode) onSelect(mode.id); + }; + + const { keyboardMode } = useListboxKeyboard({ + itemCount: approvalModes.length, + activeIndex, + onActiveIndexChange: moveHighlight, + onConfirm: confirm, + }); useEffect(() => { - const el = listRef.current?.children[selectedIdx] as + const el = listRef.current?.children[activeIndex] as | HTMLElement | undefined; el?.scrollIntoView({ block: 'nearest' }); - }, [selectedIdx]); + }, [activeIndex]); return (
0 ? `mode-opt-${activeIndex}` : undefined + } aria-label={t('mode.select')} > - {approvalModes.map((mode) => { - const selected = mode.id === currentMode; + {approvalModes.map((mode, index) => { + const selected = index === activeIndex; + const isCurrent = mode.id === currentMode; return ( - +
); })} diff --git a/packages/web-shell/client/components/dialogs/DeleteSessionDialog.test.tsx b/packages/web-shell/client/components/dialogs/DeleteSessionDialog.test.tsx new file mode 100644 index 0000000000..baa3c86b24 --- /dev/null +++ b/packages/web-shell/client/components/dialogs/DeleteSessionDialog.test.tsx @@ -0,0 +1,255 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { I18nProvider } from '../../i18n'; +import { dp } from './dialogStyles'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = () => {}; +} + +let sessions = [ + { + sessionId: 's0', + displayName: 'S0', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + }, + { + sessionId: 's1', + displayName: 'S1', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + }, + { + sessionId: 'me', + displayName: 'Current Session', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + }, +]; + +const deleteSessionMock = vi.fn(); +const deleteSessionsMock = vi.fn(); +const initialSessions = sessions.slice(); + +vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ + useConnection: () => ({ sessionId: 'me' }), + useSessions: () => ({ + sessions, + loading: false, + error: undefined, + deleteSession: deleteSessionMock, + deleteSessions: deleteSessionsMock, + }), +})); + +const { DeleteSessionDialog } = await import('./DeleteSessionDialog'); + +let container: HTMLDivElement | null = null; +let root: Root | null = null; +let onDeleted: ReturnType; +let onError: ReturnType; +let onClose: ReturnType; + +function renderDialog() { + root!.render( + + + , + ); +} + +function mount() { + onDeleted = vi.fn(); + onError = vi.fn(); + onClose = vi.fn(); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + act(() => { + renderDialog(); + }); +} + +function rerender() { + act(() => { + renderDialog(); + }); +} + +function rows(): HTMLElement[] { + return Array.from(container!.querySelectorAll('[role="option"]')); +} + +function dangerButton(): HTMLButtonElement { + return Array.from(container!.querySelectorAll('button')).find((b) => + b.className.includes(dp('dialog-danger-button')), + ) as HTMLButtonElement; +} + +function press(key: string) { + act(() => { + window.dispatchEvent( + new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true }), + ); + }); +} + +function clickRow(index: number) { + act(() => { + rows()[index].dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); +} + +const nativeInputValueSetter = Object.getOwnPropertyDescriptor( + window.HTMLInputElement.prototype, + 'value', +)!.set!; + +function typeFilter(value: string) { + act(() => { + const el = container!.querySelector('input')!; + nativeInputValueSetter.call(el, value); + el.dispatchEvent(new Event('input', { bubbles: true })); + }); +} + +const isCursor = (el: HTMLElement) => el.className.includes(dp('selected')); +const isChecked = (el: HTMLElement) => el.textContent?.includes('[x]') === true; + +beforeEach(() => { + deleteSessionMock.mockReset(); + deleteSessionsMock.mockReset(); +}); + +afterEach(() => { + act(() => root?.unmount()); + container?.remove(); + root = null; + container = null; + sessions = initialSessions.slice(); +}); + +describe('DeleteSessionDialog selection', () => { + it('keeps the keyboard cursor separate from the checked set; Enter only toggles', () => { + mount(); + + // Opens with no highlight and nothing checked; delete stays disabled. + expect(rows().some(isCursor)).toBe(false); + expect(rows().some(isChecked)).toBe(false); + expect(dangerButton().disabled).toBe(true); + + // The first ArrowDown lands the cursor on row 0 without checking it. + press('ArrowDown'); + expect(isCursor(rows()[0])).toBe(true); + expect(isChecked(rows()[0])).toBe(false); + expect(dangerButton().disabled).toBe(true); + + // Enter toggles the cursor row's checkbox — it must not delete anything. + press('Enter'); + expect(isChecked(rows()[0])).toBe(true); + expect(dangerButton().disabled).toBe(false); + expect(deleteSessionsMock).not.toHaveBeenCalled(); + expect(deleteSessionMock).not.toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); + + // Moving the cursor keeps prior checks intact (multi-select). + press('ArrowDown'); + expect(isCursor(rows()[1])).toBe(true); + expect(isChecked(rows()[0])).toBe(true); + expect(isChecked(rows()[1])).toBe(false); + + press('Enter'); + expect(isChecked(rows()[1])).toBe(true); + + // Enter on an already-checked row unchecks it. + press('Enter'); + expect(isChecked(rows()[1])).toBe(false); + expect(isChecked(rows()[0])).toBe(true); + }); + + it('does not check the current session row', () => { + mount(); + + clickRow(2); + expect(isChecked(rows()[2])).toBe(false); + expect(dangerButton().disabled).toBe(true); + + // Keyboard Enter on the current session row must not check it either. + press('ArrowDown'); + press('ArrowDown'); + press('ArrowDown'); + expect(isCursor(rows()[2])).toBe(true); + press('Enter'); + expect(isChecked(rows()[2])).toBe(false); + expect(dangerButton().disabled).toBe(true); + }); + + it('clears checked rows and disarms delete when the filter changes', () => { + mount(); + + clickRow(0); + clickRow(1); + expect(isChecked(rows()[0])).toBe(true); + expect(isChecked(rows()[1])).toBe(true); + expect(dangerButton().disabled).toBe(false); + + typeFilter('s1'); + expect(rows()).toHaveLength(1); + expect(isChecked(rows()[0])).toBe(false); + expect(rows().some(isCursor)).toBe(false); + expect(dangerButton().disabled).toBe(true); + }); + + it('prunes stale checked ids after an unfiltered session refresh', async () => { + mount(); + + clickRow(0); + clickRow(1); + expect(dangerButton().disabled).toBe(false); + + sessions = [ + { + sessionId: 'me', + displayName: 'Current Session', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + }, + ]; + rerender(); + + expect(rows()).toHaveLength(1); + expect(isChecked(rows()[0])).toBe(false); + expect(dangerButton().disabled).toBe(true); + }); + + it('deletes the checked sessions via the batch API and closes', async () => { + deleteSessionsMock.mockResolvedValue({ + removed: ['s0', 's1'], + notFound: [], + errors: [], + }); + mount(); + + clickRow(0); + clickRow(1); + expect(dangerButton().disabled).toBe(false); + + await act(async () => { + dangerButton().dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + expect(deleteSessionsMock).toHaveBeenCalledWith(['s0', 's1']); + expect(onDeleted).toHaveBeenCalledWith(['s0', 's1']); + expect(onClose).toHaveBeenCalledTimes(1); + expect(onError).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/web-shell/client/components/dialogs/DeleteSessionDialog.tsx b/packages/web-shell/client/components/dialogs/DeleteSessionDialog.tsx index 02e9a7ab20..a1be13be8a 100644 --- a/packages/web-shell/client/components/dialogs/DeleteSessionDialog.tsx +++ b/packages/web-shell/client/components/dialogs/DeleteSessionDialog.tsx @@ -2,7 +2,9 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { dp } from './dialogStyles'; import { useConnection, useSessions } from '@qwen-code/webui/daemon-react-sdk'; import { useI18n } from '../../i18n'; -import { formatRelativeTime } from '../../utils/formatRelativeTime'; +import { useListboxKeyboard } from '../../hooks/useListboxKeyboard'; +import { useFilterInput } from '../../hooks/useFilterInput'; +import { SessionRow } from './SessionRow'; interface DeleteSessionDialogProps { onDeleted: (sessionIds: string[]) => void; @@ -10,6 +12,9 @@ interface DeleteSessionDialogProps { onClose: () => void; } +const LIST_ID = 'delete-session-list'; +const optionId = (index: number) => `${LIST_ID}-opt-${index}`; + export function DeleteSessionDialog({ onDeleted, onError, @@ -26,9 +31,15 @@ export function DeleteSessionDialog({ } = useSessions({ autoLoad: true }); const currentSessionId = connection.sessionId; const [deleting, setDeleting] = useState(false); - const [selectedIdx, setSelectedIdx] = useState(0); + // `selectedIdx` is the keyboard/hover cursor (roving highlight, -1 = none — + // see ResumeDialog for the rationale); `selectedIds` is the multi-select set + // marked for deletion (shown by the [x] checkbox). + const [selectedIdx, setSelectedIdx] = useState(-1); const [selectedIds, setSelectedIds] = useState>(new Set()); - const [searchQuery, setSearchQuery] = useState(''); + const { filterValue: filterQuery, inputProps } = useFilterInput(() => { + setSelectedIdx(-1); + setSelectedIds(new Set()); + }); const [message, setMessage] = useState(null); const listRef = useRef(null); const inputRef = useRef(null); @@ -39,27 +50,42 @@ export function DeleteSessionDialog({ const filtered = useMemo( () => - searchQuery + filterQuery ? sessions.filter((s) => { - const q = searchQuery.toLowerCase(); + const q = filterQuery.toLowerCase(); return ( (s.displayName || '').toLowerCase().includes(q) || s.sessionId.toLowerCase().includes(q) ); }) : sessions, - [sessions, searchQuery], + [sessions, filterQuery], + ); + + const toggleSelection = useCallback( + (sessionId: string) => { + if (sessionId === currentSessionId) return; + setSelectedIds((prev) => { + const next = new Set(prev); + if (next.has(sessionId)) { + next.delete(sessionId); + } else { + next.add(sessionId); + } + return next; + }); + }, + [currentSessionId], ); useEffect(() => { - if (searchQuery && selectedIds.size > 0) { - const filteredSet = new Set(filtered.map((s) => s.sessionId)); - setSelectedIds((prev) => { - const pruned = new Set([...prev].filter((id) => filteredSet.has(id))); - return pruned.size === prev.size ? prev : pruned; - }); - } - }, [searchQuery, filtered, selectedIds.size]); + if (selectedIds.size === 0) return; + const filteredSet = new Set(filtered.map((s) => s.sessionId)); + setSelectedIds((prev) => { + const pruned = new Set([...prev].filter((id) => filteredSet.has(id))); + return pruned.size === prev.size ? prev : pruned; + }); + }, [filtered, selectedIds.size]); useEffect(() => { if (selectedIdx >= filtered.length && filtered.length > 0) { @@ -78,21 +104,17 @@ export function DeleteSessionDialog({ inputRef.current?.focus(); }, []); - const toggleSelection = useCallback( - (sessionId: string) => { - if (sessionId === currentSessionId) return; - setSelectedIds((prev) => { - const next = new Set(prev); - if (next.has(sessionId)) { - next.delete(sessionId); - } else { - next.add(sessionId); - } - return next; - }); + // Enter toggles the cursor row's checkbox; the actual (destructive) delete + // still requires pressing the danger button — mirroring the click behaviour. + const { keyboardMode } = useListboxKeyboard({ + itemCount: filtered.length, + activeIndex: selectedIdx, + onActiveIndexChange: setSelectedIdx, + onConfirm: (index) => { + const session = filtered[index]; + if (session) toggleSelection(session.sessionId); }, - [currentSessionId], - ); + }); const handleDelete = useCallback(() => { if (deleting) return; @@ -191,23 +213,28 @@ export function DeleteSessionDialog({ const canDelete = !deleting && !loading && hasSelection; return ( -
-
- +
+
+ {t('resume.search')}:{' '} { - setSearchQuery(e.target.value); - setSelectedIdx(0); - setSelectedIds(new Set()); - }} + className={dp('picker-search-input')} + aria-label={t('resume.search')} + role="combobox" + aria-autocomplete="list" + aria-expanded="true" + aria-controls={LIST_ID} + aria-activedescendant={ + selectedIdx >= 0 && selectedIdx < filtered.length + ? optionId(selectedIdx) + : undefined + } + {...inputProps} placeholder="" /> - + {message || (deleting ? t('delete.deleting') @@ -215,27 +242,34 @@ export function DeleteSessionDialog({ ? t('common.loading') : hasSelection ? t('delete.selected', { count: selectedIds.size }) - : searchQuery + : filterQuery ? t('delete.matches', { count: filtered.length }) : '')}
-
+
-
+
{loading && ( -
{t('common.loading')}
+
{t('common.loading')}
)} {!loading && sessionsError && ( -
- {sessionsError.message} -
+
{sessionsError.message}
)} {!loading && !sessionsError && filtered.length === 0 && ( -
- {searchQuery - ? t('delete.noMatch', { query: searchQuery }) +
+ {filterQuery + ? t('delete.noMatch', { query: filterQuery }) : t('delete.none')}
)} @@ -243,55 +277,43 @@ export function DeleteSessionDialog({ filtered.map((s, i) => { const isCurrent = s.sessionId === currentSessionId; const isChecked = selectedIds.has(s.sessionId); - const checkbox = isChecked ? '[x] ' : '[ ] '; return ( -
+ {isChecked ? '[x] ' : '[ ] '} + + } + trailing={ + isCurrent ? ( + + {t('resume.current')} + + ) : undefined + } onClick={() => { setSelectedIdx(i); if (!isCurrent) toggleSelection(s.sessionId); }} - > -
- - {checkbox} - - - {s.displayName || s.sessionId.slice(0, 8)} - - {isCurrent && ( - - {t('resume.current')} - - )} -
-
- - {(s.updatedAt || s.createdAt) && - formatRelativeTime(s.updatedAt || s.createdAt || '', t)} - - - {t('common.clients', { count: s.clientCount ?? 0 })} - - {s.hasActivePrompt && ( - - {t('resume.activePrompt')} - - )} -
-
+ onActivate={() => setSelectedIdx(i)} + /> ); })}
-
+
+ , + ); + + press('Escape'); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('ignores Escape that belongs to an IME composition', () => { + const onClose = vi.fn(); + mount( + + + , + ); + + // Chrome/Firefox: Escape cancelling a composition reports isComposing. + press('Escape', { isComposing: true }); + expect(onClose).not.toHaveBeenCalled(); + + // WebKit: compositionend fires first, so only keyCode 229 marks the key. + act(() => { + const imeEscape = new KeyboardEvent('keydown', { + key: 'Escape', + cancelable: true, + }); + Object.defineProperty(imeEscape, 'keyCode', { value: 229 }); + document.dispatchEvent(imeEscape); + }); + expect(onClose).not.toHaveBeenCalled(); + + // A genuine Escape still closes. + press('Escape'); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('keeps focus on the panel when Tab is pressed with nothing focusable inside', () => { + mount( + + + , + ); + + const panel = document.querySelector('[role="dialog"]')!; + // Simulate content whose focusables all went away (e.g. everything became + // disabled/hidden while an action runs). + document.querySelector('[data-dialog-close]')!.remove(); + document.querySelector('[data-testid="inner"]')!.remove(); + + panel.focus(); + act(() => { + document.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Tab', + bubbles: true, + cancelable: true, + }), + ); + }); + // Tab must not escape to the page behind; focus stays parked on the panel. + expect(document.activeElement).toBe(panel); + }); + + it('lets a dialog control consume Escape instead of closing', () => { + const onClose = vi.fn(); + mount( + + { + // e.g. an inline editor cancelling its edit on Escape. + if (event.key === 'Escape') event.preventDefault(); + }} + /> + , + ); + + const input = document.querySelector('input')!; + input.focus(); + act(() => { + input.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Escape', + bubbles: true, + cancelable: true, + }), + ); + }); + expect(onClose).not.toHaveBeenCalled(); + }); + + it('only the topmost of stacked shells handles Escape', () => { + const onCloseBottom = vi.fn(); + const onCloseTop = vi.fn(); + mount( + <> + + + + + + + , + ); + + // One Escape peels off one layer — the top one — not both at once. + press('Escape'); + expect(onCloseTop).toHaveBeenCalledTimes(1); + expect(onCloseBottom).not.toHaveBeenCalled(); + }); + + it('keeps focus inside the top shell if a lower shell unmounts first', () => { + const opener = document.createElement('button'); + document.body.appendChild(opener); + opener.focus(); + + function Harness({ showBottom }: { showBottom: boolean }) { + return ( + <> + {showBottom ? ( + + + + ) : null} + + + + + ); + } + + mount(); + const topButton = document.querySelector( + '[data-testid="top-focus"]', + )!; + expect(document.activeElement).toBe(topButton); + + act(() => { + root!.render( + + + + + , + ); + }); + + // Focus must stay inside the remaining top shell, not jump back behind it. + expect(document.activeElement).toBe(topButton); + opener.remove(); + }); + + it('restores focus to the remaining top shell when the lower shell unmounts after focus moved', () => { + const opener = document.createElement('button'); + document.body.appendChild(opener); + opener.focus(); + + function Harness({ showBottom }: { showBottom: boolean }) { + return ( + <> + {showBottom ? ( + + + + ) : null} + + + + + ); + } + + mount(); + const topButton = document.querySelector( + '[data-testid="top-focus"]', + )!; + document.querySelector('button:not([data-testid])')!.focus(); + + act(() => { + root!.render( + + + + + , + ); + }); + + expect(document.activeElement).toBe(topButton); + opener.remove(); + }); + + it('closes when the backdrop is clicked but not when the panel is clicked', () => { + const onClose = vi.fn(); + mount( + + + , + ); + + const backdrop = document.querySelector( + '[data-keyboard-scope]', + ); + const panel = document.querySelector('[role="dialog"]'); + expect(backdrop).toBeTruthy(); + + act(() => { + panel!.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + panel!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + expect(onClose).not.toHaveBeenCalled(); + + act(() => { + backdrop!.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + backdrop!.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })); + backdrop!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('does not close when a drag starts in the panel and ends on the backdrop', () => { + const onClose = vi.fn(); + mount( + + + , + ); + + const backdrop = document.querySelector( + '[data-keyboard-scope]', + )!; + const panel = document.querySelector('[role="dialog"]')!; + + act(() => { + panel.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + backdrop.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })); + backdrop.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + expect(onClose).not.toHaveBeenCalled(); + }); + + it('does not close when a press starts on the backdrop and ends on the panel', () => { + const onClose = vi.fn(); + mount( + + + , + ); + + const backdrop = document.querySelector( + '[data-keyboard-scope]', + )!; + const panel = document.querySelector('[role="dialog"]')!; + + act(() => { + backdrop.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + panel.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })); + // Browsers synthesize click on the nearest common ancestor for mismatched + // press/release targets; here that's effectively the backdrop. + backdrop.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + expect(onClose).not.toHaveBeenCalled(); + }); + + it('moves focus into the dialog on open', () => { + mount( + + + + , + ); + + const first = document.querySelector('[data-testid="first"]'); + expect(document.activeElement).toBe(first); + }); + + it('restores focus to the opener on close', () => { + const opener = document.createElement('button'); + document.body.appendChild(opener); + opener.focus(); + expect(document.activeElement).toBe(opener); + + mount( + + + , + ); + // Focus moved into the dialog. + expect(document.activeElement).not.toBe(opener); + + act(() => root?.unmount()); + root = null; + expect(document.activeElement).toBe(opener); + opener.remove(); + }); + + it('restores focus to the opener even if a child autofocuses first', () => { + const opener = document.createElement('button'); + document.body.appendChild(opener); + opener.focus(); + expect(document.activeElement).toBe(opener); + + mount( + + + , + ); + + const input = document.querySelector('input'); + expect(document.activeElement).toBe(input); + + act(() => root?.unmount()); + root = null; + expect(document.activeElement).toBe(opener); + opener.remove(); + }); + + it('traps Tab within the dialog, wrapping at both ends', () => { + mount( + + + , + ); + + // Focusables in DOM order: [close button, inner button]. + const close = document.querySelector('[data-dialog-close]')!; + const last = document.querySelector('[data-testid="last"]')!; + + // Tab from the last focusable wraps to the first (the close button). + last.focus(); + act(() => { + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Tab', bubbles: true }), + ); + }); + expect(document.activeElement).toBe(close); + + // Shift+Tab from the first focusable wraps to the last. + close.focus(); + act(() => { + document.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Tab', + shiftKey: true, + bubbles: true, + }), + ); + }); + expect(document.activeElement).toBe(last); + }); + + it('pulls focus into the dialog when Tab is pressed while the panel holds focus', () => { + mount( + + + , + ); + + const panel = document.querySelector('[role="dialog"]')!; + const close = document.querySelector('[data-dialog-close]')!; + const last = document.querySelector('[data-testid="last"]')!; + + // Focus sits on the panel itself (roving-list fallback). Tab pulls it in. + panel.focus(); + act(() => { + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Tab', bubbles: true }), + ); + }); + expect(document.activeElement).toBe(close); + + // Shift+Tab from the panel pulls in from the end instead. + panel.focus(); + act(() => { + document.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Tab', + shiftKey: true, + bubbles: true, + }), + ); + }); + expect(document.activeElement).toBe(last); + }); +}); diff --git a/packages/web-shell/client/components/dialogs/DialogShell.tsx b/packages/web-shell/client/components/dialogs/DialogShell.tsx index 64a0864567..cf49d42775 100644 --- a/packages/web-shell/client/components/dialogs/DialogShell.tsx +++ b/packages/web-shell/client/components/dialogs/DialogShell.tsx @@ -1,4 +1,10 @@ -import { type ReactNode } from 'react'; +import { + createContext, + useEffect, + useRef, + useState, + type ReactNode, +} from 'react'; import { createPortal } from 'react-dom'; import { useI18n } from '../../i18n'; import { useTheme, WebShellThemeId } from '../../themeContext'; @@ -21,6 +27,39 @@ const sizeClass: Record = { xl: styles.sizeXl, }; +const FOCUSABLE_SELECTOR = [ + 'a[href]:not([hidden])', + 'button:not([disabled]):not([hidden])', + 'input:not([disabled]):not([hidden])', + 'select:not([disabled]):not([hidden])', + 'textarea:not([disabled]):not([hidden])', + '[tabindex]:not([tabindex="-1"]):not([hidden])', +].join(','); + +function getFocusable(container: HTMLElement | null): HTMLElement[] { + if (!container) return []; + return Array.from( + container.querySelectorAll(FOCUSABLE_SELECTOR), + ); +} + +// Mounted shells, bottom → top. Every shell listens on `document`, and +// `stopPropagation` cannot silence sibling listeners on the same node — so +// without this, one Escape would close every stacked dialog at once (and the +// bottom one would win any race, since it registered first). Only the topmost +// shell handles keys; stacked dialogs peel off one layer per Escape. +const shellStack: object[] = []; + +export const DialogShellIdContext = createContext(null); + +export function isTopDialogShellId(shellId: object | null): boolean { + // Most production callers live inside DialogShell and get a shell id. Tests or + // any future standalone consumer may not; in that case, preserve the original + // single-dialog behavior and allow the hook to handle keys normally. + if (shellId === null) return true; + return shellStack[shellStack.length - 1] === shellId; +} + export function DialogShell({ title, subtitle, @@ -32,30 +71,182 @@ export function DialogShell({ const theme = useTheme(); const themeClass = theme === WebShellThemeId.Light ? styles.themeLight : styles.themeDark; + const panelRef = useRef(null); + // `onClose` may change identity across renders; keep the latest for the + // once-bound key listener. + const onCloseRef = useRef(onClose); + onCloseRef.current = onClose; + // Capture the opener during the dialog's first render, before any child + // effects can move focus into an autofocused search field. + const [previouslyFocused] = useState(() => + typeof document !== 'undefined' + ? (document.activeElement as HTMLElement | null) + : null, + ); + // A completed backdrop click should close, but any drag that crosses the + // panel boundary in either direction must not. Record whether the press both + // started and ended on the backdrop itself, then let the synthesized click + // close only when both are true. + const backdropPressStartedRef = useRef(false); + const backdropPressEndedRef = useRef(false); + // Identity token for this shell instance in the module-level stack. + const shellIdRef = useRef(null); + if (shellIdRef.current === null) shellIdRef.current = {}; + + // Move focus into the dialog on open, restore it to the opener on close, and + // trap Tab within the panel. Escape closes. + useEffect(() => { + const panel = panelRef.current; + const shellId = shellIdRef.current!; + shellStack.push(shellId); + + // Autofocus: respect a child that already claimed focus (e.g. a search + // input's own effect); otherwise focus the first content focusable (skipping + // the header close button), else the panel itself. Falling back to the panel + // rather than the close button avoids a stray focus ring when a list dialog's + // options are managed via a roving highlight (tabIndex=-1) instead of focus. + if (panel && !panel.contains(document.activeElement)) { + const focusables = getFocusable(panel); + const preferred = focusables.find( + (el) => !el.hasAttribute('data-dialog-close'), + ); + (preferred ?? panel).focus(); + } + + const handleKeyDown = (event: KeyboardEvent) => { + // With stacked dialogs, only the topmost shell may handle Escape/Tab — + // a lower shell closing or trapping focus would act "through" the one + // covering it. + if (shellStack[shellStack.length - 1] !== shellId) return; + // A control inside the dialog may consume the key first (e.g. Escape to + // cancel an inline edit) — honor that instead of dismissing the dialog. + if (event.defaultPrevented) return; + // Escape mid-IME-composition cancels the composition, not the dialog. + // keyCode 229 covers WebKit, which fires compositionend before the + // committing key's keydown (see useListboxKeyboard for the same guard). + if (event.isComposing || event.keyCode === 229) return; + if (event.key === 'Escape') { + event.preventDefault(); + event.stopPropagation(); + onCloseRef.current(); + return; + } + if (event.key === 'Tab') { + const focusables = getFocusable(panelRef.current); + if (focusables.length === 0) { + // Nothing focusable inside — keep focus on the panel itself. + event.preventDefault(); + panelRef.current?.focus(); + return; + } + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + const activeEl = document.activeElement; + const insideList = focusables.includes(activeEl as HTMLElement); + if (!insideList) { + // Focus is on the panel itself (e.g. a roving-highlight list where the + // options are tabIndex=-1) — pull it into the dialog so Tab can't + // escape to the page behind. + event.preventDefault(); + (event.shiftKey ? last : first).focus(); + } else if (event.shiftKey && activeEl === first) { + event.preventDefault(); + last.focus(); + } else if (!event.shiftKey && activeEl === last) { + event.preventDefault(); + first.focus(); + } + } + }; + + // Bubble phase on `document`, deliberately positioned in the middle of the + // propagation chain: controls inside the dialog run first (and can consume + // Escape via preventDefault, honored above), while `window`-level listeners + // — the app's global shortcuts and useListboxKeyboard — run after, so the + // stopPropagation on Escape still shields them. Moving this listener to the + // capture phase would steal Escape from the dialog's own controls; moving + // it to `window` would lose the race with the app-level handlers. + document.addEventListener('keydown', handleKeyDown); + return () => { + document.removeEventListener('keydown', handleKeyDown); + const idx = shellStack.indexOf(shellId); + if (idx >= 0) shellStack.splice(idx, 1); + if (shellStack.length === 0) { + previouslyFocused?.focus?.(); + return; + } + // Another modal is still stacked above the page — keep focus inside the + // remaining top shell instead of restoring it behind the modal layer. + const scopes = Array.from( + document.querySelectorAll('[data-keyboard-scope]'), + ); + const topPanel = + scopes[scopes.length - 1]?.querySelector( + '[role="dialog"]', + ); + const topFocusables = getFocusable(topPanel); + const preferred = topFocusables.find( + (el) => !el.hasAttribute('data-dialog-close'), + ); + (preferred ?? topPanel)?.focus(); + }; + }, [previouslyFocused]); + + const handleBackdropMouseDown = (event: React.MouseEvent) => { + backdropPressStartedRef.current = event.target === event.currentTarget; + backdropPressEndedRef.current = false; + }; + + const handleBackdropMouseUp = (event: React.MouseEvent) => { + backdropPressEndedRef.current = event.target === event.currentTarget; + }; + + const handleBackdropClick = (event: React.MouseEvent) => { + const shouldClose = + backdropPressStartedRef.current && + backdropPressEndedRef.current && + event.target === event.currentTarget; + backdropPressStartedRef.current = false; + backdropPressEndedRef.current = false; + if (shouldClose) { + onClose(); + } + }; const content = ( -
-
-
-
-
{title}
- {subtitle &&
{subtitle}
} -
-
-
{children}
-
+
+ +
+
+
+
{title}
+ {subtitle &&
{subtitle}
} +
+
+
{children}
+
+
); diff --git a/packages/web-shell/client/components/dialogs/ExtensionsDialog.tsx b/packages/web-shell/client/components/dialogs/ExtensionsDialog.tsx index 4389124db7..17472a8735 100644 --- a/packages/web-shell/client/components/dialogs/ExtensionsDialog.tsx +++ b/packages/web-shell/client/components/dialogs/ExtensionsDialog.tsx @@ -176,11 +176,9 @@ export function ExtensionsDialog() { }, [checking, extensions.length, loading, t]); return ( -
-
- - {message || summary} - +
+
+ {message || summary}
-
+
-
+
{!loading && extensions.length === 0 && ( -
+
{t('extensions.manage.empty')}
)} @@ -207,8 +205,8 @@ export function ExtensionsDialog() {