Make Assistant command help fully modal

This commit is contained in:
pulse-triage[bot] 2026-08-31 04:36:50 +01:00
parent 445ebacf24
commit 89fccc6c4f
6 changed files with 91 additions and 39 deletions

View file

@ -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 (
<div
class="absolute inset-0 z-50 flex items-end bg-slate-950/20 p-3 sm:items-center sm:justify-center"
onClick={props.onClose}
<Dialog
isOpen={true}
onClose={props.onClose}
layout="mobile-sheet"
panelClass="max-w-[30rem] shadow-xl"
ariaLabel={AI_CHAT_COMMAND_HELP_TITLE}
>
<section
role="dialog"
aria-modal="true"
aria-label={AI_CHAT_COMMAND_HELP_TITLE}
class="max-h-[min(34rem,calc(100%-1.5rem))] w-full max-w-[30rem] overflow-hidden rounded-md border border-border bg-surface shadow-xl"
onClick={(event) => event.stopPropagation()}
>
<>
<div class="flex min-h-12 items-center justify-between gap-3 border-b border-border px-4 py-3">
<h3 class="text-sm font-semibold text-base-content">{AI_CHAT_COMMAND_HELP_TITLE}</h3>
<button
@ -141,9 +130,7 @@ export function AssistantCommandHelpDialog(props: AssistantCommandHelpDialogProp
</div>
<div class="border-b border-border px-3 py-2">
<SearchField
inputRef={(element) => {
searchInputRef = element;
}}
autofocus
value={commandSearchQuery()}
onChange={setCommandSearchQuery}
title={AI_CHAT_COMMAND_HELP_SEARCH_LABEL}
@ -229,7 +216,7 @@ export function AssistantCommandHelpDialog(props: AssistantCommandHelpDialogProp
</For>
</Show>
</div>
</section>
</div>
</>
</Dialog>
);
}

View file

@ -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(() => (
<>
<button type="button" onClick={() => setIsOpen(true)}>
Open command help
</button>
<Show when={isOpen()}>
<AssistantCommandHelpDialog onClose={() => setIsOpen(false)} onRunCommand={vi.fn()} />
</Show>
</>
));
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();
});
});

View file

@ -15,6 +15,7 @@ export const SearchField: Component<SearchFieldProps> = (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}

View file

@ -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

View file

@ -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();
}

View file

@ -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;