client: a click made during the Settings exit keeps its focus

changelog.d/2270-settings-focus-restore.fixed.md

@@ -0,0 +1 @@
+Closing Settings no longer pulls focus back to its opener after you clicked somewhere else while the dialog was fading out: the focus hand-back now runs only while the dialog still holds it (#2270).

core/client.js

@@ -8261,7 +8261,10 @@ function useDialogFocus(ref) {
     const before = document.activeElement;
     ref.current?.focus?.({ preventScroll: true });
     return () => {
-      if (before && typeof before.focus === "function" && before.isConnected) before.focus({ preventScroll: true });
+      const active = document.activeElement;
+      const stillInDialog = active === ref.current || ref.current?.contains?.(active) === true;
+      const noFocusHeld = !active || active === document.body || active.isConnected === false;
+      if (before && typeof before.focus === "function" && before.isConnected && (stillInDialog || noFocusHeld || active === before)) before.focus({ preventScroll: true });
     };
   }, []);
 }

core/src/client/overlay-hooks.js

@@ -22,11 +22,19 @@ 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).
+// It lingers for DIALOG_EXIT_MS (160 ms) with `pointer-events: none` on the leaving layer,
+// so a click there lands on the page behind it: restore only while the dialog still holds
+// focus (or nothing does), never over a place the user picked during the exit (#2270).
 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 }) }
+    return () => {
+      const active = document.activeElement
+      const stillInDialog = active === ref.current || ref.current?.contains?.(active) === true
+      const noFocusHeld = !active || active === document.body || active.isConnected === false
+      if (before && typeof before.focus === 'function' && before.isConnected && (stillInDialog || noFocusHeld || active === before)) before.focus({ preventScroll: true })
+    }
   }, [])
 }
 

core/test/dialog-focus.test.js

@@ -42,9 +42,14 @@ const react = {
   useSyncExternalStore: (_subscribe, get) => get(),
 }
 
+// A browser moves `document.activeElement` when a node takes focus. A fake that only
+// records the call cannot tell "the dialog still has focus" from "the user moved it on".
+let activeDocument = null
+
 const fakeNode = (kind) => ({
   kind, focusCalls: [], style: {}, dataset: {}, isConnected: true,
-  focus(options) { this.focusCalls.push(options ?? {}) },
+  focus(options) { this.focusCalls.push(options ?? {}); if (activeDocument) activeDocument.activeElement = this },
+  contains: node => node === this,
   querySelector: () => null,
   querySelectorAll: () => [],
   addEventListener() {}, removeEventListener() {},
@@ -88,6 +93,7 @@ function mount(overrides = {}) {
     },
   }
   context.module.exports = context.exports
+  activeDocument = context.document
   runInNewContext(outputFiles[0].text, context)
   const { SettingsDialog } = context.module.exports
   const tree = SettingsDialog({ page: 'general', id: null, roster, actions, useUpdate: () => ({}), ...overrides })
@@ -99,7 +105,7 @@ function mount(overrides = {}) {
     if (typeof cleanup === 'function') cleanups.push(cleanup)
   }
   return {
-    tree, dialogNode, opener,
+    tree, dialogNode, opener, document: context.document,
     unmount() { for (const cleanup of cleanups.reverse()) cleanup() },
   }
 }
@@ -123,6 +129,20 @@ test('opening moves focus into the dialog and closing gives it back to the still
   assert.equal(opener.focusCalls[0]?.preventScroll, true)
 })
 
+// #2270: the Settings dialog is drawn for DIALOG_EXIT_MS (160 ms) after it closes, with
+// `pointer-events: none` on that leaving layer, so a click there lands on the page behind
+// it. The unmount must not drag focus back to the opener once the user has moved it.
+test('a click that moves focus during the exit animation is not pulled back to the opener', () => {
+  const { document, opener, unmount } = mount()
+  const composer = fakeNode('composer')
+  composer.focus({ preventScroll: true })
+  assert.equal(document.activeElement, composer, 'the user\'s click took focus')
+  unmount()
+  assert.equal(document.activeElement, composer, 'focus stays where the user put it')
+  assert.equal(composer.focusCalls.length, 1, 'the late cleanup does not refocus the page')
+  assert.equal(opener.focusCalls.length, 0, 'the opener is not focused after the user moved on')
+})
+
 test('the dialog keeps its role, aria-modal, aria-label and data-wide contract', () => {
   const narrow = mount().dialogNode.props
   assert.equal(narrow.role, 'dialog')

docs/subsystems/core-client.md

@@ -16,7 +16,7 @@ The browser side of the patched `ds-bot` core (`core/`): sources in `core/src/cl
 - The chat's `ask_user` card (`questions.js`) answers where it is drawn: one option answers a single-select card at once, a multi-select card toggles its rows and **Submit** sends the set, and the × dismisses it; the same card waits in the dock above the composer when its call is not on screen. In a window that holds the team read-only the card keeps the question, its rows and the host's sentence (`roster.readOnly`, read through `lockedWindow` from `schedules.js`) and draws none of those controls — no option button, **Submit**, **Skip**, × or own-answer field, and no form to submit. Each of them reached a write the host refuses (`answer-question`, `dismiss-question`), and the refusal was invisible: a pressed answer marked the card answered and the next poll said nothing, while the × hid the card until that poll brought it back. Tests: `core/test/questions-readonly.test.js`.
 - Read-only banner: when another dsh holds the team (`roster.readOnly` a non-empty string), the sidebar draws the host's own sentence and, beside it, one **Reload** control in both shapes (wide and rail; the control stays outside the `role="status"` line so a live region remains text). It calls `actions.reloadTeam`, which re-reads the whole team (`state` without `since`, see `stateRequest` / `stateAnswerIsStale` in `sources.js`) so the banner and every refused control clear once the other holder let go, without a page reload that would drop what is in the composer. Tests: `core/test/sidebar-readonly-reload.test.js`, `core/test/client-reload-state.test.js`.
 - 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).
+- 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). It is drawn for `DIALOG_EXIT_MS` (160 ms) with `pointer-events: none` on the leaving layer, so the unmount restores focus only while the dialog still holds it (or nothing does) — a click on the page during that window keeps its focus (#2270).
 - Settings → Feedback's error list (`feedback.js`) still titles the block with the whole log (`Recent errors · 60`) but lists only the newest eight, so when the report holds more rows one `Show more` control (`aria-expanded`) sits under them and draws every loaded error, folding back to `Show less`; a report of eight or fewer draws no control (#2266).
 
 ## Does not own

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

@@ -2287,17 +2287,25 @@ diff --git a/src/client/new-chat.js b/src/client/new-chat.js
 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) {
+@@ -20,6 +20,24 @@ 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).
++// It lingers for DIALOG_EXIT_MS (160 ms) with `pointer-events: none` on the leaving layer,
++// so a click there lands on the page behind it: restore only while the dialog still holds
++// focus (or nothing does), never over a place the user picked during the exit (#2270).
 +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 }) }
++    return () => {
++      const active = document.activeElement
++      const stillInDialog = active === ref.current || ref.current?.contains?.(active) === true
++      const noFocusHeld = !active || active === document.body || active.isConnected === false
++      if (before && typeof before.focus === 'function' && before.isConnected && (stillInDialog || noFocusHeld || active === before)) before.focus({ preventScroll: true })
++    }
 +  }, [])
 +}
 +

patches/ds-bot/0008-client-bundle.patch

@@ -5029,7 +5029,7 @@ diff --git a/client.js b/client.js
  .bt-mem-skel{display:flex;flex-direction:column;gap:24px}
  .bt-mem-skel-block{display:flex;flex-direction:column;gap:10px}
  .bt-mem-skel-block span{display:block;height:12px;border-radius:6px;background:linear-gradient(90deg,var(--bt-hover) 25%,var(--bt-active) 50%,var(--bt-hover) 75%);background-size:200% 100%;animation:bt-mem-skel 1.4s ease-in-out infinite}
-@@ -2574,2935 +2574,7487 @@ body[data-ds-dark-theme] .bt-theme-chip i{background:var(--da)}
+@@ -2574,2935 +2574,7490 @@ body[data-ds-dark-theme] .bt-theme-chip i{background:var(--da)}
  @media (max-width:640px){.bt-mem{width:100vw;height:100%;border-radius:0;border:0}.bt-mem-body{padding:18px 18px 20px}.bt-mem-foot{padding:8px 12px 12px}}
  @media (prefers-reduced-motion:reduce){.bt-mem,.bt-mem-scrim,.bt-mem-summary>*,.bt-mem-empty,.bt-mem-reply,.bt-mem-skel-block span,.bt-mem-entries,.bt-mem-send>*,.bt-mem-changes li svg{animation:none}}
 
@@ -9203,7 +9203,10 @@ diff --git a/client.js b/client.js
 +    const before = document.activeElement;
 +    ref.current?.focus?.({ preventScroll: true });
 +    return () => {
-+      if (before && typeof before.focus === "function" && before.isConnected) before.focus({ preventScroll: true });
++      const active = document.activeElement;
++      const stillInDialog = active === ref.current || ref.current?.contains?.(active) === true;
++      const noFocusHeld = !active || active === document.body || active.isConnected === false;
++      if (before && typeof before.focus === "function" && before.isConnected && (stillInDialog || noFocusHeld || active === before)) before.focus({ preventScroll: true });
 +    };
 +  }, []);
  }

patches/ds-bot/files/test/dialog-focus.test.js

@@ -42,9 +42,14 @@ const react = {
   useSyncExternalStore: (_subscribe, get) => get(),
 }
 
+// A browser moves `document.activeElement` when a node takes focus. A fake that only
+// records the call cannot tell "the dialog still has focus" from "the user moved it on".
+let activeDocument = null
+
 const fakeNode = (kind) => ({
   kind, focusCalls: [], style: {}, dataset: {}, isConnected: true,
-  focus(options) { this.focusCalls.push(options ?? {}) },
+  focus(options) { this.focusCalls.push(options ?? {}); if (activeDocument) activeDocument.activeElement = this },
+  contains: node => node === this,
   querySelector: () => null,
   querySelectorAll: () => [],
   addEventListener() {}, removeEventListener() {},
@@ -88,6 +93,7 @@ function mount(overrides = {}) {
     },
   }
   context.module.exports = context.exports
+  activeDocument = context.document
   runInNewContext(outputFiles[0].text, context)
   const { SettingsDialog } = context.module.exports
   const tree = SettingsDialog({ page: 'general', id: null, roster, actions, useUpdate: () => ({}), ...overrides })
@@ -99,7 +105,7 @@ function mount(overrides = {}) {
     if (typeof cleanup === 'function') cleanups.push(cleanup)
   }
   return {
-    tree, dialogNode, opener,
+    tree, dialogNode, opener, document: context.document,
     unmount() { for (const cleanup of cleanups.reverse()) cleanup() },
   }
 }
@@ -123,6 +129,20 @@ test('opening moves focus into the dialog and closing gives it back to the still
   assert.equal(opener.focusCalls[0]?.preventScroll, true)
 })
 
+// #2270: the Settings dialog is drawn for DIALOG_EXIT_MS (160 ms) after it closes, with
+// `pointer-events: none` on that leaving layer, so a click there lands on the page behind
+// it. The unmount must not drag focus back to the opener once the user has moved it.
+test('a click that moves focus during the exit animation is not pulled back to the opener', () => {
+  const { document, opener, unmount } = mount()
+  const composer = fakeNode('composer')
+  composer.focus({ preventScroll: true })
+  assert.equal(document.activeElement, composer, 'the user\'s click took focus')
+  unmount()
+  assert.equal(document.activeElement, composer, 'focus stays where the user put it')
+  assert.equal(composer.focusCalls.length, 1, 'the late cleanup does not refocus the page')
+  assert.equal(opener.focusCalls.length, 0, 'the opener is not focused after the user moved on')
+})
+
 test('the dialog keeps its role, aria-modal, aria-label and data-wide contract', () => {
   const narrow = mount().dialogNode.props
   assert.equal(narrow.role, 'dialog')