fix(app): keep the composer toolbar's focus rings clean (#57192)

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"]')

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(() => <DialogSelectModel model={props.model} />, 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(
+                  () => <DialogSelectModel model={props.model} />,
+                  event.detail === 0 ? undefined : props.onClose,
+                )
               }}
             >
               {content()}

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

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 (
     <Tooltip
       placement="top"
@@ -834,6 +838,7 @@ export function ComposerEditorAddMenu(props: {
     >
       <Menu gutter={6} modal={false} placement="top-start">
         <Menu.Trigger
+          {...focus.trigger}
           as={IconButton}
           data-action="composer-attach"
           type="button"
@@ -845,6 +850,7 @@ export function ComposerEditorAddMenu(props: {
         />
         <Menu.Portal>
           <Menu.Content
+            onCloseAutoFocus={focus.onCloseAutoFocus}
             class="[&_[data-slot=menu-v2-item-shortcut]]:w-5 [&_[data-slot=menu-v2-item-shortcut]]:justify-center"
             style={{ "min-width": "180px" }}
           >
@@ -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={
           <Menu gutter={6} modal={false} placement="top-start" onOpenChange={props.onOpenChange}>
             <Menu.Trigger
+              {...focus.trigger}
               as={Button}
               variant="ghost-muted"
               size="normal"
@@ -960,7 +996,7 @@ export function ComposerEditorSelect(props: {
               {content()}
             </Menu.Trigger>
             <Menu.Portal>
-              <Menu.Content>
+              <Menu.Content onCloseAutoFocus={focus.onCloseAutoFocus}>
                 <Menu.RadioGroup value={props.current} onChange={props.onSelect}>
                   <For each={props.options}>
                     {(option) => (