diff --git a/frontend-modern/src/components/AI/Chat/AssistantCommandHelpDialog.tsx b/frontend-modern/src/components/AI/Chat/AssistantCommandHelpDialog.tsx index 0cc1558b9d..616ab3530f 100644 --- a/frontend-modern/src/components/AI/Chat/AssistantCommandHelpDialog.tsx +++ b/frontend-modern/src/components/AI/Chat/AssistantCommandHelpDialog.tsx @@ -1,4 +1,4 @@ -import { For, Show, createEffect, createMemo, createSignal, onCleanup, onMount } from 'solid-js'; +import { For, Show, createEffect, createMemo, createSignal, onCleanup } from 'solid-js'; import XIcon from 'lucide-solid/icons/x'; import { type AssistantSlashCommandAvailability, @@ -16,6 +16,7 @@ import { AI_CHAT_COMMAND_HELP_TITLE, } from '@/utils/aiChatPresentation'; import { SearchField } from '@/components/shared/SearchField'; +import { Dialog } from '@/components/shared/Dialog'; interface AssistantCommandHelpDialogProps { availability?: AssistantSlashCommandAvailability; @@ -26,7 +27,6 @@ interface AssistantCommandHelpDialogProps { export function AssistantCommandHelpDialog(props: AssistantCommandHelpDialogProps) { const [commandSearchQuery, setCommandSearchQuery] = createSignal(''); const [selectedCommandIndex, setSelectedCommandIndex] = createSignal(0); - let searchInputRef: HTMLInputElement | undefined; const commands = createMemo(() => filterAssistantSlashCommands(commandSearchQuery(), undefined, { @@ -58,10 +58,6 @@ export function AssistantCommandHelpDialog(props: AssistantCommandHelpDialogProp const controlOnly = event.ctrlKey && !event.metaKey && !event.shiftKey && !event.altKey; switch (event.key) { - case 'Escape': - consumeDialogCloseKey(event); - props.onClose(); - break; case 'ArrowDown': consumeDialogCloseKey(event); moveSelectedCommand(1); @@ -111,22 +107,15 @@ export function AssistantCommandHelpDialog(props: AssistantCommandHelpDialogProp } }); - onMount(() => { - queueMicrotask(() => searchInputRef?.focus()); - }); - return ( -
- -
+ + ); } diff --git a/frontend-modern/src/components/AI/Chat/__tests__/AssistantCommandHelpDialog.test.tsx b/frontend-modern/src/components/AI/Chat/__tests__/AssistantCommandHelpDialog.test.tsx index 28419c5f7f..1fbcfed4b3 100644 --- a/frontend-modern/src/components/AI/Chat/__tests__/AssistantCommandHelpDialog.test.tsx +++ b/frontend-modern/src/components/AI/Chat/__tests__/AssistantCommandHelpDialog.test.tsx @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { cleanup, fireEvent, render, screen, waitFor } from '@solidjs/testing-library'; +import { Show, createSignal } from 'solid-js'; import { AssistantCommandHelpDialog } from '../AssistantCommandHelpDialog'; afterEach(cleanup); @@ -145,4 +146,38 @@ describe('AssistantCommandHelpDialog', () => { expect(laterDocumentHandler).not.toHaveBeenCalled(); expect(onClose).toHaveBeenCalledOnce(); }); + + it('contains focus, isolates the background, and restores the invoking control', async () => { + const [isOpen, setIsOpen] = createSignal(false); + render(() => ( + <> + + + setIsOpen(false)} onRunCommand={vi.fn()} /> + + + )); + + const trigger = screen.getByRole('button', { name: 'Open command help' }); + trigger.focus(); + fireEvent.click(trigger); + + const search = await screen.findByLabelText('Search Assistant commands'); + await waitFor(() => expect(search).toHaveFocus()); + const dialog = screen.getByRole('dialog', { name: 'Assistant commands' }); + expect(dialog.closest('[data-dialog-layer]')).not.toHaveAttribute('inert'); + expect(trigger.closest('div')).toHaveAttribute('inert'); + + const options = screen.getAllByRole('option'); + const lastOption = options[options.length - 1]; + lastOption.focus(); + fireEvent.keyDown(document, { key: 'Tab' }); + expect(screen.getByRole('button', { name: 'Close Assistant commands' })).toHaveFocus(); + + fireEvent.click(screen.getByRole('button', { name: 'Close Assistant commands' })); + expect(screen.queryByRole('dialog', { name: 'Assistant commands' })).not.toBeInTheDocument(); + expect(trigger).toHaveFocus(); + }); }); diff --git a/frontend-modern/src/components/shared/SearchField.tsx b/frontend-modern/src/components/shared/SearchField.tsx index 2fd67eb882..7075db4237 100644 --- a/frontend-modern/src/components/shared/SearchField.tsx +++ b/frontend-modern/src/components/shared/SearchField.tsx @@ -15,6 +15,7 @@ export const SearchField: Component = (props) => { placeholder={props.placeholder ?? 'Search...'} value={props.value} disabled={props.disabled} + autofocus={props.autofocus} onInput={(e) => props.onChange(e.currentTarget.value)} onKeyDown={search.handleKeyDown} onFocus={search.handleFocus} diff --git a/frontend-modern/src/components/shared/__tests__/dialogModel.branchcov0712.test.ts b/frontend-modern/src/components/shared/__tests__/dialogModel.branchcov0712.test.ts index 0ce55ca255..29ded7b640 100644 --- a/frontend-modern/src/components/shared/__tests__/dialogModel.branchcov0712.test.ts +++ b/frontend-modern/src/components/shared/__tests__/dialogModel.branchcov0712.test.ts @@ -6,11 +6,8 @@ import { getDialogViewportClass, } from '@/components/shared/dialogModel'; -// Drive every branch of getDialogViewportClass, getDialogAlignmentClass, and -// getDialogPanelClass. Each function is a single ternary keyed on -// `layout === 'drawer-right'`; getDialogPanelClass adds a `panelClass ?? ...` -// nullish-coalescing with its own inner drawer-vs-modal ternary, plus a final -// `.trim()`. Every assertion is exact-string equality. +// Drive every layout branch of the dialog presentation helpers. Every +// assertion is exact-string equality so responsive placement cannot drift. describe('getDialogViewportClass', () => { it("returns the 'p-0' padding arm for the drawer-right layout", () => { @@ -25,6 +22,12 @@ describe('getDialogViewportClass', () => { ); }); + it('uses compact padding for the mobile-sheet layout', () => { + expect(getDialogViewportClass('mobile-sheet')).toBe( + 'relative h-full overflow-y-auto pointer-events-none p-3', + ); + }); + it("falls into the modal (else) arm for any value that isn't strictly 'drawer-right'", () => { // Defensive: strict equality means a foreign string lands in the else arm, // producing the same classes as 'modal'. @@ -48,6 +51,12 @@ describe('getDialogAlignmentClass', () => { ); }); + it('bottom-aligns the mobile sheet and centers it at the small breakpoint', () => { + expect(getDialogAlignmentClass('mobile-sheet')).toBe( + 'flex min-h-full items-end justify-center sm:items-center', + ); + }); + it("falls into the modal (else) arm for any value that isn't strictly 'drawer-right'", () => { const foreign = 'bottom-sheet' as unknown as DialogLayout; expect(getDialogAlignmentClass(foreign)).toBe( @@ -64,6 +73,7 @@ describe('getDialogPanelClass', () => { const DRAWER_BRANCH = 'h-dvh max-w-[720px] rounded-none border-y-0 border-r-0 animate-slide-up sm:h-full sm:max-h-dvh sm:rounded-l-xl sm:border-y sm:border-r-0'; const MODAL_BRANCH = 'max-h-[calc(100dvh-2rem)] rounded-md animate-slide-up'; + const MOBILE_SHEET_BRANCH = 'max-h-[calc(100dvh-1.5rem)] rounded-md animate-slide-up'; describe('layout ternary — drawer-right arm', () => { it('emits the drawer branch and trims the trailing empty fallback when panelClass is undefined', () => { @@ -105,6 +115,18 @@ describe('getDialogPanelClass', () => { }); }); + describe('mobile-sheet layout', () => { + it('uses the compact viewport height and modal width fallback', () => { + expect(getDialogPanelClass('mobile-sheet')).toBe(`${BASE} ${MOBILE_SHEET_BRANCH} max-w-lg`); + }); + + it('accepts a custom panel width', () => { + expect(getDialogPanelClass('mobile-sheet', 'max-w-[30rem]')).toBe( + `${BASE} ${MOBILE_SHEET_BRANCH} max-w-[30rem]`, + ); + }); + }); + describe('layout ternary — defensive (foreign value falls into modal arm)', () => { it('routes a non-DialogLayout string through the modal branch', () => { // Strict equality with 'drawer-right' fails for any other string, so the diff --git a/frontend-modern/src/components/shared/dialogModel.ts b/frontend-modern/src/components/shared/dialogModel.ts index 3735890b9c..78a63ae364 100644 --- a/frontend-modern/src/components/shared/dialogModel.ts +++ b/frontend-modern/src/components/shared/dialogModel.ts @@ -1,4 +1,4 @@ -export type DialogLayout = 'modal' | 'drawer-right'; +export type DialogLayout = 'modal' | 'mobile-sheet' | 'drawer-right'; const FOCUSABLE_SELECTOR = 'a[href],area[href],button:not([disabled]),input:not([disabled]):not([type="hidden"]),select:not([disabled]),textarea:not([disabled]),[tabindex]:not([tabindex="-1"])'; @@ -15,23 +15,29 @@ export function getDialogFocusableElements(container: HTMLElement): HTMLElement[ } export function getDialogViewportClass(layout: DialogLayout): string { - return `relative h-full overflow-y-auto pointer-events-none ${ - layout === 'drawer-right' ? 'p-0' : 'p-4 sm:p-6' - }`; + const paddingClass = + layout === 'drawer-right' ? 'p-0' : layout === 'mobile-sheet' ? 'p-3' : 'p-4 sm:p-6'; + return `relative h-full overflow-y-auto pointer-events-none ${paddingClass}`; } export function getDialogAlignmentClass(layout: DialogLayout): string { - return `flex min-h-full ${ + const alignmentClass = layout === 'drawer-right' ? 'items-stretch justify-end' - : 'items-start justify-center sm:items-center' - }`; + : layout === 'mobile-sheet' + ? 'items-end justify-center sm:items-center' + : 'items-start justify-center sm:items-center'; + return `flex min-h-full ${alignmentClass}`; } export function getDialogPanelClass(layout: DialogLayout, panelClass?: string): string { - return `relative flex min-h-0 w-full flex-col overflow-hidden bg-surface border border-border outline-none pointer-events-auto ${ + const layoutClass = layout === 'drawer-right' ? 'h-dvh max-w-[720px] rounded-none border-y-0 border-r-0 animate-slide-up sm:h-full sm:max-h-dvh sm:rounded-l-xl sm:border-y sm:border-r-0' - : 'max-h-[calc(100dvh-2rem)] rounded-md animate-slide-up' + : layout === 'mobile-sheet' + ? 'max-h-[calc(100dvh-1.5rem)] rounded-md animate-slide-up' + : 'max-h-[calc(100dvh-2rem)] rounded-md animate-slide-up'; + return `relative flex min-h-0 w-full flex-col overflow-hidden bg-surface border border-border outline-none pointer-events-auto ${ + layoutClass } ${panelClass ?? (layout === 'drawer-right' ? '' : 'max-w-lg')}`.trim(); } diff --git a/frontend-modern/src/components/shared/searchFieldModel.ts b/frontend-modern/src/components/shared/searchFieldModel.ts index e9412a344c..ff59ef7f40 100644 --- a/frontend-modern/src/components/shared/searchFieldModel.ts +++ b/frontend-modern/src/components/shared/searchFieldModel.ts @@ -24,6 +24,7 @@ export interface SearchFieldProps { class?: string; inputClass?: string; disabled?: boolean; + autofocus?: boolean; onKeyDown?: (event: SearchFieldKeyboardEvent) => void; onFocus?: (event: SearchFieldFocusEvent) => void; onBlur?: (event: SearchFieldFocusEvent) => void;