fix(app): keep focus rings unclipped and keyboard-only (#57221)

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")
+})

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;
+}

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: () => (
+    <div class="w-[560px]">
+      <SessionQueuePanel queue={view(3)} />
+    </div>
+  ),
+}
+
+export const Scrolling = {
+  render: () => (
+    <div class="w-[560px]">
+      <SessionQueuePanel queue={view(6)} />
+    </div>
+  ),
+}

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. */}
           <div
             ref={listRef}
+            data-slot="session-queue-list"
             class="flex flex-col gap-px"
             classList={{ "max-h-[148px] overflow-y-auto": count() > 4 }}
             style={{

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()

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<MenuAppearance> }>({
+const MenuAppearanceContext = createContext<{
+  appearance: Accessor<MenuAppearance>
+  pointerOpened: () => boolean
+}>({
   appearance: (): MenuAppearance => "compact",
+  pointerOpened: () => false,
 })
 
 function useMenuContext() {
@@ -242,11 +247,28 @@ function MenuSeparator(props: ComponentProps<typeof DropdownMenu.Separator>) {
 
 function MenuContent(props: ComponentProps<typeof DropdownMenu.Content>) {
   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 (
     <DropdownMenu.Content
       {...r}
+      onCloseAutoFocus={onCloseAutoFocus}
       data-component="menu-v2-content"
       data-appearance={ctx.appearance()}
       classList={{ ...s.classList, [s.class ?? ""]: !!s.class }}
@@ -259,12 +281,26 @@ export interface MenuProps extends ComponentProps<typeof DropdownMenu> {
 }
 
 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 (
-    <MenuAppearanceContext.Provider value={{ appearance }}>
-      <DropdownMenu {...rest}>{local.children}</DropdownMenu>
+    <MenuAppearanceContext.Provider value={{ appearance, pointerOpened: () => pointerOpened }}>
+      <DropdownMenu
+        {...rest}
+        onOpenChange={(open) => {
+          if (open) pointerOpened = pointer
+          local.onOpenChange?.(open)
+        }}
+      >
+        {local.children}
+      </DropdownMenu>
     </MenuAppearanceContext.Provider>
   )
 }
@@ -278,7 +314,7 @@ function MenuContextRoot(props: MenuContextProps) {
   const appearance = () => local.appearance ?? "compact"
 
   return (
-    <MenuAppearanceContext.Provider value={{ appearance }}>
+    <MenuAppearanceContext.Provider value={{ appearance, pointerOpened: () => false }}>
       <ContextMenu {...rest}>{local.children}</ContextMenu>
     </MenuAppearanceContext.Provider>
   )

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 = {
     </Root>
   ),
 }
+
+function OpenFromButton() {
+  const dialog = useDialog()
+
+  return (
+    <Button
+      variant="neutral"
+      onClick={() =>
+        dialog.show(() => (
+          <Dialog>
+            {dialogHeader("Choose model", "Pick Alpha or Beta")}
+            <DialogBody>Alpha and Beta are available.</DialogBody>
+          </Dialog>
+        ))
+      }
+    >
+      Choose model
+    </Button>
+  )
+}
+
+export const OpenedFromButton = {
+  render: () => <OpenFromButton />,
+}