From 69caff9cab5d0d4bbda73e77b370093eadc82179 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 19 Oct 2026 21:33:54 -0700 Subject: [PATCH] fix(app): keep focus rings unclipped and keyboard-only (#57221) --- .../app/component-tests/focus-ring.spec.ts | 68 +++++++++++++++++++ .../app/src/session/composer/queue-panel.css | 10 +++ .../session/composer/queue-panel.stories.tsx | 52 ++++++++++++++ .../app/src/session/composer/queue-panel.tsx | 2 + packages/ui/src/context/dialog.tsx | 10 +-- packages/ui/src/navigation/menu/menu.tsx | 48 +++++++++++-- .../ui/src/overlays/dialog/dialog.stories.tsx | 25 +++++++ 7 files changed, 205 insertions(+), 10 deletions(-) create mode 100644 packages/app/component-tests/focus-ring.spec.ts create mode 100644 packages/app/src/session/composer/queue-panel.css create mode 100644 packages/app/src/session/composer/queue-panel.stories.tsx diff --git a/packages/app/component-tests/focus-ring.spec.ts b/packages/app/component-tests/focus-ring.spec.ts new file mode 100644 index 00000000000..3030dfad94e --- /dev/null +++ b/packages/app/component-tests/focus-ring.spec.ts @@ -0,0 +1,68 @@ +import { expect, story } from "../../storybook/playwright/story" + +story("returns focus to a menu trigger without a ring after a pointer open", async ({ mount, page }) => { + await mount("ui-menu--basic") + const trigger = page.getByRole("button", { name: "Open menu" }) + + await trigger.click() + await expect(page.getByRole("menuitem", { name: "Save" })).toBeVisible() + await page.keyboard.press("Escape") + await expect(trigger).toBeFocused() + await expect(trigger).toHaveCSS("outline-style", "none") +}) + +story("keeps the ring on a menu trigger after a keyboard open", async ({ mount, page }) => { + await mount("ui-menu--basic") + const trigger = page.getByRole("button", { name: "Open menu" }) + + await page.keyboard.press("Shift") + await trigger.focus() + await page.keyboard.press("Enter") + await expect(page.getByRole("menuitem", { name: "Save" })).toBeVisible() + await page.keyboard.press("Escape") + await expect(trigger).toBeFocused() + await expect(trigger).toHaveCSS("outline-style", "solid") +}) + +story("returns focus to a dialog's trigger without a ring after a pointer open", async ({ mount, page }) => { + await mount("ui-dialog--opened-from-button") + const trigger = page.getByRole("button", { name: "Choose model" }) + + await trigger.click() + await expect(page.getByText("Alpha and Beta are available.")).toBeVisible() + await expect + .poll(() => page.evaluate(() => !!document.activeElement?.closest('[data-component="dialog-v2"]'))) + .toBe(true) + await page.keyboard.press("Escape") + await expect(trigger).toBeFocused() + await expect(trigger).toHaveCSS("outline-style", "none") +}) + +story("keeps the ring on a dialog's trigger after a keyboard open", async ({ mount, page }) => { + await mount("ui-dialog--opened-from-button") + const trigger = page.getByRole("button", { name: "Choose model" }) + + await page.keyboard.press("Shift") + await trigger.focus() + await page.keyboard.press("Enter") + await expect(page.getByText("Alpha and Beta are available.")).toBeVisible() + await expect + .poll(() => page.evaluate(() => !!document.activeElement?.closest('[data-component="dialog-v2"]'))) + .toBe(true) + await page.keyboard.press("Escape") + await expect(trigger).toBeFocused() + await expect(trigger).toHaveCSS("outline-style", "solid") +}) + +story("keeps queue rings inside controls under the list fade", async ({ mount, page }) => { + await mount("session-queue-panel--scrolling") + const list = page.locator('[data-slot="session-queue-list"]') + const remove = page.locator('[data-action="session-queue-remove"]').first() + + await expect(list).toHaveCSS("scroll-padding-top", "8px") + await expect(list).toHaveCSS("scroll-padding-bottom", "12px") + await page.keyboard.press("Shift") + await remove.focus() + await expect(remove).toHaveCSS("outline-style", "solid") + await expect(remove).toHaveCSS("outline-offset", "-2px") +}) diff --git a/packages/app/src/session/composer/queue-panel.css b/packages/app/src/session/composer/queue-panel.css new file mode 100644 index 00000000000..7c29c32b52f --- /dev/null +++ b/packages/app/src/session/composer/queue-panel.css @@ -0,0 +1,10 @@ +/* The list scrolls under a fade that would cut a ring drawn outside a control, so rings sit inside each control and + focus scrolls a row clear of the 8px and 12px fades. */ +[data-slot="session-queue-list"] { + scroll-padding-block: 8px 12px; +} + +[data-component="session-queue-panel"] + :is([data-component="button-v2"], [data-component="icon-button-v2"]):is(:focus-visible, [data-state="focus"]) { + outline-offset: -2px; +} diff --git a/packages/app/src/session/composer/queue-panel.stories.tsx b/packages/app/src/session/composer/queue-panel.stories.tsx new file mode 100644 index 00000000000..1649cd8f45b --- /dev/null +++ b/packages/app/src/session/composer/queue-panel.stories.tsx @@ -0,0 +1,52 @@ +import { SessionQueuePanel } from "./queue-panel" +import type { SessionQueueView } from "./queue" + +const prompts = [ + "Compare the Alpha and Beta fixtures", + "Rename the Alpha export", + "Add a Beta regression test", + "Summarize the Alpha changes", + "Document the Beta settings", + "Remove the unused Alpha helper", +] + +const view = (count: number): SessionQueueView => ({ + rows: () => prompts.slice(0, count).map((text, index) => ({ id: `queued_${index}`, text, attachments: 0 })), + editing: () => undefined, + working: () => true, + busy: () => false, + steer: async () => undefined, + remove: async () => undefined, + undo: () => undefined, + edit: () => false, + reorder: async () => undefined, +}) + +export default { + title: "OpenCode/Session/Queue panel", + id: "session-queue-panel", + parameters: { + layout: "centered", + docs: { + description: { + component: "Prompts queued behind the current turn. Past four rows the list scrolls under a fade.", + }, + }, + }, +} + +export const Short = { + render: () => ( +
+ +
+ ), +} + +export const Scrolling = { + render: () => ( +
+ +
+ ), +} diff --git a/packages/app/src/session/composer/queue-panel.tsx b/packages/app/src/session/composer/queue-panel.tsx index 650fdde5dff..6c27f80d369 100644 --- a/packages/app/src/session/composer/queue-panel.tsx +++ b/packages/app/src/session/composer/queue-panel.tsx @@ -11,6 +11,7 @@ import { IconButton } from "@opencode/ui/icon-button" import { Tooltip } from "@opencode/ui/tooltip" import { useLanguage } from "@/runtime/i18n/language" import type { SessionQueueView } from "./queue" +import "./queue-panel.css" // Pullout above the composer listing the prompts queued behind the current // turn. The panel slides under the composer card (negative margin, opaque @@ -66,6 +67,7 @@ export function SessionQueuePanel(props: { queue: SessionQueueView }) { {/* Four 32px rows, four 1px gaps, and half a row hint at more queued prompts. */}
4 }} style={{ diff --git a/packages/ui/src/context/dialog.tsx b/packages/ui/src/context/dialog.tsx index 612ee89e973..7a99fd71cb8 100644 --- a/packages/ui/src/context/dialog.tsx +++ b/packages/ui/src/context/dialog.tsx @@ -18,7 +18,7 @@ import { makeEventListener } from "@solid-primitives/event-listener" type DialogElement = () => JSX.Element -type FocusReturn = { element: HTMLElement; range?: Range } +type FocusReturn = { element: HTMLElement; visible: boolean; range?: Range } type Active = { id: string @@ -265,12 +265,14 @@ function captureFocus(): FocusReturn | undefined { if (!(element instanceof HTMLElement) || element === document.body) return const selection = document.getSelection() + // Closing with Escape counts as keyboard use, so Chromium would ring a control the pointer had focused. + const visible = element.matches(":focus-visible") if (!element.isContentEditable || !selection?.rangeCount || !element.contains(selection.anchorNode)) { - return { element } + return { element, visible } } - return { element, range: selection.getRangeAt(0).cloneRange() } + return { element, visible, range: selection.getRangeAt(0).cloneRange() } } /** Kobalte only returns focus to a dialog trigger, and these dialogs open without one. */ @@ -281,7 +283,7 @@ function returnFocus(focus: FocusReturn | undefined, layer: HTMLElement | undefi // Something outside the closed dialog already has focus, for example because the dialog's onClose moved it. if (active && active !== document.body && !layer?.contains(active)) return - focus.element.focus({ preventScroll: true }) + focus.element.focus({ preventScroll: true, focusVisible: focus.visible }) if (!focus.range || !focus.element.contains(focus.range.commonAncestorContainer)) return const selection = document.getSelection() diff --git a/packages/ui/src/navigation/menu/menu.tsx b/packages/ui/src/navigation/menu/menu.tsx index c3347d8a2ef..4aca8f6ed47 100644 --- a/packages/ui/src/navigation/menu/menu.tsx +++ b/packages/ui/src/navigation/menu/menu.tsx @@ -1,5 +1,6 @@ import { DropdownMenu } from "@kobalte/core/dropdown-menu" import { ContextMenu } from "@kobalte/core/context-menu" +import { makeEventListener } from "@solid-primitives/event-listener" import { createContext, Show, @@ -41,8 +42,12 @@ const CheckMark: Component = () => ( export type MenuAppearance = "standard" | "compact" -const MenuAppearanceContext = createContext<{ appearance: Accessor }>({ +const MenuAppearanceContext = createContext<{ + appearance: Accessor + pointerOpened: () => boolean +}>({ appearance: (): MenuAppearance => "compact", + pointerOpened: () => false, }) function useMenuContext() { @@ -242,11 +247,28 @@ function MenuSeparator(props: ComponentProps) { function MenuContent(props: ComponentProps) { const ctx = useMenuContext() - const [s, r] = splitProps(props, ["class", "classList"]) + const [s, r] = splitProps(props, ["class", "classList", "onCloseAutoFocus"]) + + // 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. + const onCloseAutoFocus = (event: Event) => { + s.onCloseAutoFocus?.(event) + + if (event.defaultPrevented || !ctx.pointerOpened()) 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 + const labelledby = event.target instanceof Element ? event.target.getAttribute("aria-labelledby") : null + const trigger = labelledby ? document.getElementById(labelledby) : null + trigger?.focus({ preventScroll: true, focusVisible: false }) + } return ( { } function MenuRoot(props: MenuProps) { - const [local, rest] = splitProps(props, ["appearance", "children"]) + const [local, rest] = splitProps(props, ["appearance", "children", "onOpenChange"]) const appearance = () => local.appearance ?? "compact" + let pointer = false + let pointerOpened = false + + // Capture runs before the trigger's own handlers, so the last input is what opened the menu. + makeEventListener(window, "pointerdown", () => (pointer = true), { capture: true }) + makeEventListener(window, "keydown", () => (pointer = false), { capture: true }) return ( - - {local.children} + pointerOpened }}> + { + if (open) pointerOpened = pointer + local.onOpenChange?.(open) + }} + > + {local.children} + ) } @@ -278,7 +314,7 @@ function MenuContextRoot(props: MenuContextProps) { const appearance = () => local.appearance ?? "compact" return ( - + false }}> {local.children} ) diff --git a/packages/ui/src/overlays/dialog/dialog.stories.tsx b/packages/ui/src/overlays/dialog/dialog.stories.tsx index 207726f4659..fb1e2967c59 100644 --- a/packages/ui/src/overlays/dialog/dialog.stories.tsx +++ b/packages/ui/src/overlays/dialog/dialog.stories.tsx @@ -1,6 +1,7 @@ import { Overlay, Portal, Root, Trigger } from "@kobalte/core/dialog" import { Dialog, DialogBody, DialogFooter, DialogHeader, DialogTitle, DialogTitleGroup } from "./dialog" import { Button } from "@opencode/ui/button" +import { useDialog } from "@opencode/ui/context/dialog" const docs = `### Overview Dialog content wrapper built on Kobalte's dialog primitive with v2 styling. @@ -204,3 +205,27 @@ export const Fit = { ), } + +function OpenFromButton() { + const dialog = useDialog() + + return ( + + ) +} + +export const OpenedFromButton = { + render: () => , +}