client: Settings takes focus when it opens and returns it to the opener (#2251)

changelog.d/2251-settings-dialog-focus.fixed.md

@@ -0,0 +1 @@
+Settings now takes keyboard focus when it opens: the dialog is the focused element, so Tab starts inside it and Escape reaches it even when the message box had focus. Closing returns focus to the still-connected control that opened it (#2251).

core/client.js

@@ -7624,6 +7624,15 @@ function useEscape(onClose) {
     };
   }, []);
 }
+function useDialogFocus(ref) {
+  (0, import_react11.useEffect)(() => {
+    const before = document.activeElement;
+    ref.current?.focus?.({ preventScroll: true });
+    return () => {
+      if (before && typeof before.focus === "function" && before.isConnected) before.focus({ preventScroll: true });
+    };
+  }, []);
+}
 function usePaneLeft() {
   const [paneLeft, setPaneLeft] = (0, import_react11.useState)(280);
   (0, import_react11.useLayoutEffect)(() => {
@@ -19141,6 +19150,8 @@ function SettingsDialog({ page, id, roster, actions, useUpdate, leaving = false
   const [error, setError] = (0, import_react42.useState)("");
   const close = (0, import_react42.useCallback)(() => actions.closeSettings(), []);
   useEscape(close);
+  const dialog = (0, import_react42.useRef)(null);
+  useDialogFocus(dialog);
   const run = (promise) => Promise.resolve(promise).then(() => {
     setError("");
     return true;
@@ -19173,7 +19184,7 @@ function SettingsDialog({ page, id, roster, actions, useUpdate, leaving = false
     (0, import_react42.createElement)("div", { className: "bt-scrim", onMouseDown: close }),
     (0, import_react42.createElement)(
       "div",
-      { className: "bt-settings", role: "dialog", "aria-modal": true, "aria-label": t("FlyBot settings"), "data-wide": shown === "usage" ? "" : void 0 },
+      { className: "bt-settings", role: "dialog", "aria-modal": true, "aria-label": t("FlyBot settings"), "data-wide": shown === "usage" ? "" : void 0, tabIndex: -1, ref: dialog },
       (0, import_react42.createElement)(
         "nav",
         { ref: nav.box, className: "bt-settings-nav", "aria-label": t("Settings pages") },

core/src/client/overlay-hooks.js

@@ -20,6 +20,16 @@ export function useEscape(onClose) {
   }, [])
 }
 
+// A dialog takes focus when it opens, so Tab starts inside it and Escape reaches it,
+// and gives that focus back to the still-connected opener when it closes (#2251, #274).
+export function useDialogFocus(ref) {
+  useEffect(() => {
+    const before = document.activeElement
+    ref.current?.focus?.({ preventScroll: true })
+    return () => { if (before && typeof before.focus === 'function' && before.isConnected) before.focus({ preventScroll: true }) }
+  }, [])
+}
+
 // New chat and Bot exchanges show in place of the conversation, right of the sidebar.
 export function usePaneLeft() {
   const [paneLeft, setPaneLeft] = useState(280)

core/src/client/settings.js

@@ -8,7 +8,7 @@ import { CloseIcon, ChevronLeftIcon, ChevronRightIcon, ClockIcon, ConnectorIcon,
 import { AdminStar, BotAvatar, RoomAvatar } from './mark.js'
 import { openHarnessSettings } from './sidebar.js'
 import { SettingsPageSlot, useSettingsPages, call } from './flybot-slots.js'
-import { useEscape } from './overlay-hooks.js'
+import { useDialogFocus, useEscape } from './overlay-hooks.js'
 import { BotSettingsForm, SwitchRow } from './bot-settings.js'
 import { Segmented, useGlide } from './motion.js'
 import { GroupSettingsForm, NewGroupForm, seatedCount } from './group-settings.js'
@@ -174,6 +174,10 @@ export function SettingsDialog({ page, id, roster, actions, useUpdate, leaving =
   const [error, setError] = useState('')
   const close = useCallback(() => actions.closeSettings(), [])
   useEscape(close)
+  // The dialog takes focus when it opens, so Tab starts inside it and Escape reaches it
+  // even when the composer had focus; closing gives that focus back (#2251).
+  const dialog = useRef(null)
+  useDialogFocus(dialog)
   const run = promise => Promise.resolve(promise)
     .then(() => { setError(''); return true })
     .catch(failure => { setError(failure?.message ?? String(failure)); return false })
@@ -209,7 +213,7 @@ export function SettingsDialog({ page, id, roster, actions, useUpdate, leaving =
   const errorLine = error ? h('div', { className: 'bt-error', role: 'alert', style: { padding: '0 20px 8px' } }, error) : null
   return h('div', { className: 'bt-settings-layer', 'data-leaving': leaving || undefined },
     h('div', { className: 'bt-scrim', onMouseDown: close }),
-    h('div', { className: 'bt-settings', role: 'dialog', 'aria-modal': true, 'aria-label': t('FlyBot settings'), 'data-wide': shown === 'usage' ? '' : undefined },
+    h('div', { className: 'bt-settings', role: 'dialog', 'aria-modal': true, 'aria-label': t('FlyBot settings'), 'data-wide': shown === 'usage' ? '' : undefined, tabIndex: -1, ref: dialog },
       h('nav', { ref: nav.box, className: 'bt-settings-nav', 'aria-label': t('Settings pages') },
         h('span', { ref: nav.thumb, className: 'bt-thumb', 'aria-hidden': true }),
         pages.map(([key, text]) => h('button', {

core/test/dialog-focus.test.js

@@ -0,0 +1,133 @@
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { fileURLToPath } from 'node:url'
+import { runInNewContext } from 'node:vm'
+import { build } from 'esbuild'
+
+// #2251: an `aria-modal` Settings dialog never took focus, so Tab walked the page
+// behind the scrim and Escape could be caught by the composer. The dialog takes focus
+// when it opens and gives it back to the opener when it closes (#274's shape).
+// This test runs the real hooks: a small runtime below commits refs and runs effects.
+const { outputFiles } = await build({
+  stdin: {
+    contents: "export { SettingsDialog } from './src/client/settings.js'",
+    resolveDir: fileURLToPath(new URL('..', import.meta.url)),
+  },
+  bundle: true, format: 'cjs', platform: 'browser', write: false, logLevel: 'silent',
+  external: ['react', 'react-dom', '@deepseek-ai/dsh-client-ui-primitives'],
+})
+
+const h = (type, props, ...children) => ({ type, props: { ...props, children } })
+const all = (node, out = []) => {
+  if (node && typeof node === 'object') {
+    out.push(node)
+    for (const child of [node.props?.children].flat(Infinity)) all(child, out)
+  }
+  return out
+}
+
+// One effect per hook call, run by `mount` after refs are committed, the way React
+// runs layout effects and effects on mount. `useEffect() {}` stubs would hide the bug.
+let effects = []
+const react = {
+  createElement: h,
+  Component: class {},
+  Fragment: Symbol('F'),
+  useEffect: (fn) => { effects.push(fn) },
+  useLayoutEffect: (fn) => { effects.push(fn) },
+  useState: initial => [typeof initial === 'function' ? initial() : initial, () => {}],
+  useMemo: make => make(),
+  useRef: current => ({ current }),
+  useCallback: fn => fn,
+  useSyncExternalStore: (_subscribe, get) => get(),
+}
+
+const fakeNode = (kind) => ({
+  kind, focusCalls: [], style: {}, dataset: {}, isConnected: true,
+  focus(options) { this.focusCalls.push(options ?? {}) },
+  querySelector: () => null,
+  querySelectorAll: () => [],
+  addEventListener() {}, removeEventListener() {},
+  getBoundingClientRect: () => ({ left: 0, top: 0, right: 0, bottom: 0, width: 0, height: 0 }),
+  offsetLeft: 0, offsetTop: 0, offsetWidth: 0, offsetHeight: 0,
+})
+
+// Point every `ref` in the returned tree at a node, as the renderer would on commit.
+const commit = (node, seen = new Set()) => {
+  if (!node || typeof node !== 'object' || seen.has(node)) return
+  seen.add(node)
+  const ref = node.props?.ref
+  if (ref && typeof ref === 'object' && ref.current === null) ref.current = fakeNode('element')
+  for (const child of [node.props?.children].flat(Infinity)) commit(child, seen)
+}
+
+const roster = { bots: [], rooms: [], byId: {}, roomsById: {}, prefs: {}, mainBotIds: [], connectors: [] }
+const actions = { openSettings() {}, closeSettings() {} }
+
+function mount(overrides = {}) {
+  effects = []
+  const opener = fakeNode('opener')
+  const context = {
+    module: { exports: {} }, exports: {},
+    document: {
+      activeElement: opener, hidden: false, addEventListener() {}, removeEventListener() {},
+      querySelector: () => null,
+    },
+    navigator: { language: 'en' },
+    localStorage: { getItem: () => null, setItem() {} },
+    window: {
+      addEventListener() {}, removeEventListener() {},
+      matchMedia: () => ({ matches: false, addEventListener() {}, removeEventListener() {} }),
+    },
+    console, setTimeout, clearTimeout, requestAnimationFrame: () => 0, cancelAnimationFrame() {},
+    require: (name) => {
+      if (name === 'react') return react
+      if (name === 'react-dom') return { createPortal: node => node }
+      if (name === '@deepseek-ai/dsh-client-ui-primitives') return {}
+      throw new Error(`Unexpected platform module ${name}`)
+    },
+  }
+  context.module.exports = context.exports
+  runInNewContext(outputFiles[0].text, context)
+  const { SettingsDialog } = context.module.exports
+  const tree = SettingsDialog({ page: 'general', id: null, roster, actions, useUpdate: () => ({}), ...overrides })
+  commit(tree)
+  const dialogNode = all(tree).find(node => node.props?.className === 'bt-settings')
+  const cleanups = []
+  for (const effect of effects) {
+    const cleanup = effect()
+    if (typeof cleanup === 'function') cleanups.push(cleanup)
+  }
+  return {
+    tree, dialogNode, opener,
+    unmount() { for (const cleanup of cleanups.reverse()) cleanup() },
+  }
+}
+
+test('the Settings dialog is a focus target: tabIndex -1 on the dialog container', () => {
+  const { dialogNode } = mount()
+  assert.ok(dialogNode, 'the dialog container is drawn')
+  assert.equal(dialogNode.props.tabIndex, -1)
+})
+
+test('opening moves focus into the dialog and closing gives it back to the still-connected opener', () => {
+  const { dialogNode, opener, unmount } = mount()
+  const dialogRef = dialogNode.props.ref
+  assert.ok(dialogRef, 'the dialog container carries a ref so it can take focus')
+  const dialog = dialogRef.current
+  assert.equal(dialog.focusCalls.length, 1, 'the dialog takes focus once on mount')
+  assert.equal(dialog.focusCalls[0]?.preventScroll, true)
+  assert.equal(opener.focusCalls.length, 0, 'the opener is not touched while the dialog is open')
+  unmount()
+  assert.equal(opener.focusCalls.length, 1, 'the opener gets focus back when the dialog unmounts')
+  assert.equal(opener.focusCalls[0]?.preventScroll, true)
+})
+
+test('the dialog keeps its role, aria-modal, aria-label and data-wide contract', () => {
+  const narrow = mount().dialogNode.props
+  assert.equal(narrow.role, 'dialog')
+  assert.equal(narrow['aria-modal'], true)
+  assert.equal(narrow['aria-label'], 'FlyBot settings')
+  assert.equal(narrow['data-wide'], undefined)
+  assert.equal(mount({ page: 'usage' }).dialogNode.props['data-wide'], '')
+})

docs/subsystems/core-client.md

@@ -14,6 +14,8 @@ The browser side of the patched `ds-bot` core (`core/`): sources in `core/src/cl
 - Browser client: sidebar, chat, panels, themes, characters, Bot browser panel (`core/client.js` generated from `core/src/client/`). The sidebar **+** new-chat picker (`core/src/client/new-chat.js`) takes extra rows from slot `flybot.new-chat.action` (after New group chat; `label` / `hint` go through the same `t()` map) and lists only `listedChatBots` (gone, hidden, and home-level remotes stay out; owner labels match the sidebar). Covered by `core/test/new-chat.test.js` in patch `0003-flybot-api-client`.
 
 - Chat context rows: a tool the plugin registers at install draws one `tool-addition` row, and `cells.js` keeps every name in `TEAM_TOOLS` out of the chat (`SILENT_REGISTRATION_TOOLS`, #1101) — the Project Assistant's `project_digest` joins them (#1712), so its 09:00 Session shows the digest, not its registration.
+- A Settings dialog takes focus when it opens (`useDialogFocus` in `core/src/client/overlay-hooks.js`; the `.bt-settings` dialog carries `tabIndex -1` and a ref) and gives it back to the still-connected opener when it closes; it is not a focus trap, so Tab can still leave the dialog for the page behind the scrim (#2251).
+
 
 ## Does not own
 

patches/ds-bot/0003-flybot-api-client.patch

@@ -2213,6 +2213,26 @@ diff --git a/src/client/new-chat.js b/src/client/new-chat.js
          h('div', { className: 'bt-nc-card' },
            h('span', { className: 'bt-nc-plus', 'aria-hidden': true }, h(PlusIcon)),
            h('textarea', {
+diff --git a/src/client/overlay-hooks.js b/src/client/overlay-hooks.js
+--- a/src/client/overlay-hooks.js
++++ b/src/client/overlay-hooks.js
+@@ -20,6 +20,16 @@ export function useEscape(onClose) {
+   }, [])
+ }
+
++// A dialog takes focus when it opens, so Tab starts inside it and Escape reaches it,
++// and gives that focus back to the still-connected opener when it closes (#2251, #274).
++export function useDialogFocus(ref) {
++  useEffect(() => {
++    const before = document.activeElement
++    ref.current?.focus?.({ preventScroll: true })
++    return () => { if (before && typeof before.focus === 'function' && before.isConnected) before.focus({ preventScroll: true }) }
++  }, [])
++}
++
+ // New chat and Bot exchanges show in place of the conversation, right of the sidebar.
+ export function usePaneLeft() {
+   const [paneLeft, setPaneLeft] = useState(280)
 diff --git a/src/client/overlays.js b/src/client/overlays.js
 --- a/src/client/overlays.js
 +++ b/src/client/overlays.js