diff --git a/desktop/src/features/channels/ui/CoverDrawer.tsx b/desktop/src/features/channels/ui/CoverDrawer.tsx index 02c242d691..4f775f89bc 100644 --- a/desktop/src/features/channels/ui/CoverDrawer.tsx +++ b/desktop/src/features/channels/ui/CoverDrawer.tsx @@ -184,6 +184,14 @@ export function CoverDrawer({ const travelPx = prefersReducedMotion ? 0 : COVER_DRAWER_TRAVEL_PX; const drawerRef = React.useRef(null); const previousFocusRef = React.useRef(null); + /** + * Whether the opener has been captured for this drawer instance. + * + * Distinct from `previousFocusRef.current === null`, which is a legitimate + * capture (nothing was focused) and must not be retried. See the capture + * effect for why one attempt is all this gets. + */ + const hasCapturedPreviousFocusRef = React.useRef(false); React.useEffect(() => { if (!ownsEscape) return; @@ -214,10 +222,27 @@ export function CoverDrawer({ }, [escapeYieldsToContent, onClose, ownsEscape]); React.useLayoutEffect(() => { - previousFocusRef.current = - document.activeElement instanceof HTMLElement - ? document.activeElement - : null; + // Capture the opener exactly once per drawer instance. + // + // `React.StrictMode` replays effects in development as setup → cleanup → + // setup, and by that second setup this drawer has already focused itself. An + // unconditional read of `document.activeElement` would therefore record the + // drawer as its own opener, and a real close would focus a node React has + // since detached — leaving focus on ``, keyboard-stranded. Refs survive + // the replay, so a one-shot flag is enough. The flag is deliberately not + // reset in cleanup: the only cleanup it would see before a real close is the + // simulated one, which is exactly what it exists to ignore. + // + // Re-claiming the focus slot on the replayed setup is correct and stays as + // is — that new generation is what makes the first cleanup's deferred + // restore stand down. + if (!hasCapturedPreviousFocusRef.current) { + hasCapturedPreviousFocusRef.current = true; + previousFocusRef.current = + document.activeElement instanceof HTMLElement + ? document.activeElement + : null; + } const focusClaim = claimCoverDrawerFocus(); drawerRef.current?.focus({ preventScroll: true }); diff --git a/desktop/src/features/channels/ui/CoverDrawerFocusHandoff.test.mjs b/desktop/src/features/channels/ui/CoverDrawerFocusHandoff.test.mjs index 74b94f8a9d..ce07ddc7e0 100644 --- a/desktop/src/features/channels/ui/CoverDrawerFocusHandoff.test.mjs +++ b/desktop/src/features/channels/ui/CoverDrawerFocusHandoff.test.mjs @@ -143,6 +143,26 @@ test("only the last-opened drawer keeps focus across a chain of replacements", a third.unmount(); }); +test("a drawer restores focus to the opener even when StrictMode replays its capture", async () => { + // The app root is wrapped in `React.StrictMode`, which in development runs + // every effect setup → cleanup → setup. By that second setup the drawer has + // already focused itself, so a capture that reads `document.activeElement` + // unconditionally records the drawer as its own opener; closing then focuses a + // node React has detached and focus falls to ``. Capture has to survive + // the replay, which is what the other tests here cannot see because they + // render without StrictMode. + const { act, drawer, opener, render } = await loadHarness(); + const view = render(drawer("strict-drawer"), { reactStrictMode: true }); + assert.equal(activeTestId(), "strict-drawer"); + + await act(async () => { + view.unmount(); + }); + await flushDeferredFocusRestore(act); + + assert.equal(dom.window.document.activeElement, opener); +}); + test("a released slot leaves focus where the caller put it", async () => { // The thread view-mode switch: nothing replaces the drawer, but the switch has // already decided where focus belongs, so the drawer's own restore must not