diff --git a/packages/app/component-tests/composer.spec.ts b/packages/app/component-tests/composer.spec.ts index 4b6202692db..f3f42dfcfac 100644 --- a/packages/app/component-tests/composer.spec.ts +++ b/packages/app/component-tests/composer.spec.ts @@ -136,6 +136,32 @@ story("does not mask or pad controls when they fit", async ({ mount }) => { await expect(controls).toHaveCSS("padding-inline-end", "0px") }) +story("keeps toolbar focus rings whole and keyboard-only", async ({ mount, page }) => { + const component = await mount("opencode-composer-flow--toolbar-overflow", { args: { alternate: "none" } }) + const controls = component.locator('[data-slot="composer-controls"]') + const agent = controls.getByRole("button", { name: "Choose agent" }) + const variant = controls.getByRole("button", { name: "Choose model variant" }) + await page.evaluate(() => document.fonts.ready.then(() => undefined)) + + // Arriving from the keyboard on a control parked half under the edge scrolls all of it, ring included, into view. + await controls.evaluate((element) => { + const control = element.querySelector('[aria-label="Choose agent"]')!.getBoundingClientRect() + element.scrollLeft = control.width / 2 + }) + await page.keyboard.press("Shift") + await agent.focus() + await expect(agent).toHaveCSS("outline-style", "solid") + await expect(agent).toHaveCSS("outline-offset", "-2px") + await expect.poll(() => controls.evaluate((element) => element.scrollLeft)).toBe(0) + + // A menu the pointer opened returns focus to its trigger without a ring, even when Escape closes it. + await variant.click() + await expect(page.getByRole("menuitemradio", { name: "high", exact: true })).toBeVisible() + await page.keyboard.press("Escape") + await expect(variant).toBeFocused() + await expect(variant).toHaveCSS("outline-style", "none") +}) + story("grows suggestions while preserving visible timeline context", async ({ mount }) => { const component = await mount("opencode-composer-flow--constrained-command-suggestions") const boundary = component.locator('[data-slot="composer-suggestion-boundary-story"]') diff --git a/packages/app/src/composer/composer.tsx b/packages/app/src/composer/composer.tsx index 201ff1b550d..ec6b3d94dea 100644 --- a/packages/app/src/composer/composer.tsx +++ b/packages/app/src/composer/composer.tsx @@ -156,9 +156,11 @@ function ComposerModelControl(props: { onClick={(event: MouseEvent) => { if (!props.paid) return props.onUnpaidClick() // A pointer open keeps focus in the composer, so the dialog returns it to the caret. A keyboard open - // returns focus to this trigger instead of moving it to the editor. - const keyboard = document.activeElement === event.currentTarget - void dialog.show(() => , keyboard ? undefined : props.onClose) + // (a click with no pointer presses) returns focus to this trigger instead of moving it to the editor. + void dialog.show( + () => , + event.detail === 0 ? undefined : props.onClose, + ) }} > {content()} diff --git a/packages/app/src/composer/editor/editor.css b/packages/app/src/composer/editor/editor.css index 444dcf3b808..9740e90d839 100644 --- a/packages/app/src/composer/editor/editor.css +++ b/packages/app/src/composer/editor/editor.css @@ -11,6 +11,11 @@ --mask-end: 16px; } +/* Focus scrolls a control clear of the 16px fade instead of under it. */ +[data-slot="composer-controls"] { + scroll-padding-inline: 16px; +} + [data-slot="composer-controls"]:dir(rtl) { --mask-direction: to left; } @@ -25,9 +30,10 @@ ); } -/* The controls viewport scrolls and clips, so toolbar rings hug each control and fit inside its 2px inline padding. */ -[data-component="composer"] :is([data-component="button-v2"], [data-component="icon-button-v2"]):is(:focus-visible, [data-state="focus"]) { - outline-offset: 0; +/* The controls viewport scrolls and clips, so toolbar rings sit inside each control where no edge can cut them. */ +[data-component="composer"] + :is([data-component="button-v2"], [data-component="icon-button-v2"]):is(:focus-visible, [data-state="focus"]) { + outline-offset: -2px; } @media (hover: hover) { diff --git a/packages/app/src/composer/editor/editor.tsx b/packages/app/src/composer/editor/editor.tsx index ad6e30e1776..2c9742016c1 100644 --- a/packages/app/src/composer/editor/editor.tsx +++ b/packages/app/src/composer/editor/editor.tsx @@ -341,8 +341,10 @@ export function ComposerEditor(props: ComposerEditorProps) { data-slot="composer-controls" data-overflow-start={overflow.start} data-overflow-end={overflow.end} - class="ms-0.5 me-2.5 h-full min-w-0 flex-1 overflow-x-auto overscroll-x-contain px-0.5 no-scrollbar" + class="ms-1 me-3 h-full min-w-0 flex-1 overflow-x-auto overscroll-x-contain no-scrollbar" onScroll={updateOverflow} + // Chromium leaves a partly visible control where it is when it takes focus, cut off by the edge. + onFocusIn={() => document.activeElement?.scrollIntoView({ block: "nearest", inline: "nearest" })} aria-hidden={state.mode === "shell"} inert={state.mode === "shell" ? true : undefined} style={buttons()} @@ -822,6 +824,8 @@ export function ComposerEditorAddMenu(props: { onContext: () => void onShell: () => void }) { + const focus = pointerFocusReturn() + return ( @@ -868,6 +874,34 @@ export function ComposerEditorAddMenu(props: { ) } +/** + * A menu returns focus to its trigger when it closes. Closing with Escape counts as keyboard use, so Chromium would + * ring a trigger the pointer opened; restore focus without the ring and keep it for keyboard opens. + */ +function pointerFocusReturn() { + let pointer: HTMLElement | undefined + + return { + trigger: { + onPointerDown: (event: PointerEvent & { currentTarget: HTMLElement }) => { + pointer = event.currentTarget + }, + onKeyDown: (event: KeyboardEvent) => { + if (event.key !== "Escape") pointer = undefined + }, + }, + onCloseAutoFocus: (event: Event) => { + if (!pointer) return + event.preventDefault() + const active = document.activeElement + + // An outside press already moved focus where it belongs. + if (active && active !== document.body && !active.closest('[role="menu"]')) return + pointer.focus({ preventScroll: true, focusVisible: false }) + }, + } +} + function ComposerEditorConfiguredSelect(props: { mobileDrawer?: boolean title: string @@ -910,6 +944,7 @@ export function ComposerEditorSelect(props: { }) { const mobile = createMediaQuery("(max-width: 767px)") const [store, setStore] = createStore({ open: false, loaded: false }) + const focus = pointerFocusReturn() let trigger: HTMLButtonElement | undefined const setOpen = (open: boolean) => { @@ -951,6 +986,7 @@ export function ComposerEditorSelect(props: { fallback={ - + {(option) => (