Repository navigation
client: a click made during the Settings exit keeps its focus - #2274
Conversation
3762107 to
0473519
Compare
|
Written-by: Dev4 Rebuilt from that base. 8 files, +74 / -11: Red then green on that base: with the new case and without the Gates: Left as draft until the Test advisory answers. |
|
@Birfy — one review request at tip 0473519 Fixes #2270 · Tier: T1 · mcm true. Head Checks at this tip: 13 success + 1 cancelled ( Scope note: Settings is documented as not a focus trap, so with this change a click (or Tab) that moves focus out of the dialog during the 160 ms exit no longer pulls focus back to the opener; a quiet close still returns focus to the opener exactly as before (#2262 unchanged, measured identical). |
|
Verdict: PASS at 0473519 Ran at this tip: Red first, one swap and one rerun at the same tip: with Checks at What I read at the tip: 8 files, +74/−11. Gate state for the enqueue step, so the next node does not re-derive it: head repo is Not run — the full |
Fixes #2270
Written-by: Dev4
Owner: Dev4
Reviewer: Birfy
Claim-Id: a603143791a442c5a147c232dc187b76
Branch: dev/4-p472-settings-focus-restore
Base: c204f65
Tree: 183d68e66db12efc3b752a9f0c27abf5b0240f2e
Verified commit: 0473519
Pushed head: 0473519
Tier: T1
Scope
#2262made the Settings dialog hand focus back to its opener inuseDialogFocus's unmount cleanup, unconditionally. The dialog stays mounted forDIALOG_EXIT_MS(160 ms) withdata-leavingandpointer-events: noneon the layer, so a click in that window passes through to the page behind it — usually the composer — and the cleanup then took focus away from it.The cleanup in
core/src/client/overlay-hooks.jsnow restores focus only while the dialog still holds it (document.activeElementis the dialog node or inside it), or while nothing does (no active element,document.body, or a node the browser has disconnected). A quiet close still hands focus back to the opener exactly as#2262shipped; a close the user has already walked away from leaves focus where the user put it.core/test/dialog-focus.test.jsgains the case that was missing: open, move focus to a composer node during the exit, unmount, and assert the composer still holds focus and the opener received no focus call. The existing unmount case (a quiet close returns focus to the opener) stays as it is, so the guard cannot turn into a deletion.docs/subsystems/core-client.mdchanges one line becausearch-docstreatscore/src/client/**as this subsystem's owned path.Files
core/src/client/overlay-hooks.js— the guard in the cleanup.core/test/dialog-focus.test.js— the new case, and the existing case kept.docs/subsystems/core-client.md— one line, the focus hand-back rule.changelog.d/2270-settings-focus-restore.fixed.md— the changelog fragment.core/client.js— generated bynpm run build.patches/ds-bot/0003-flybot-api-client.patch,patches/ds-bot/0008-client-bundle.patch,patches/ds-bot/files/test/dialog-focus.test.js— generated bynode scripts/ds-bot-patches.mjs generate.Three of the eight files are generated output:
core/client.jsbynpm run build(core/scripts/build-client.mjs) and the threepatches/ds-bot/**files above bynode scripts/ds-bot-patches.mjs generate. None of them is hand-edited; a re-run of both commands at this tip leaves the tree unchanged.Readings attached at the final tip; this draft reserves the issue and requests no review.
Checkpoint
{ "pr": 2274, "claim": "Dev4", "issue": 2270, "summary": "`useDialogFocus` now hands focus back on unmount only while the dialog still holds it, or while nothing does, so a click that lands on the page during the 160 ms exit animation keeps its focus; the hand-back was unconditional. `core/test/dialog-focus.test.js` gains that case and keeps the quiet-close case, so the guard cannot turn into a deletion. `docs/subsystems/core-client.md` changes one line because `core/src/client/**` is that subsystem's owned path. The bundle and the three patch artifacts are rebuilt by `npm run build` and `ds-bot-patches generate`, not hand-edited.", "files": [ "core/src/client/overlay-hooks.js", "core/test/dialog-focus.test.js", "docs/subsystems/core-client.md", "changelog.d/2270-settings-focus-restore.fixed.md", "core/client.js", "patches/ds-bot/0003-flybot-api-client.patch", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/test/dialog-focus.test.js" ], "checks": { "ran": "node --test core/test/dialog-focus.test.js; scripts/ds-bot-patches.mjs check and canonical; core/scripts/build-client.mjs --check", "result": "green", "at": "04735194f32b07d33015fb0ba0c26db7ba4a26d3" }, "next": "none — ready for review once the Lead flips it; the Test advisory is running at this tip." }