jerry/buzz

mirror of https://github.com/block/buzz.git synced 2026-11-09 10:51:42 +00:00

Files

ss-dev-00 1afe30f650 fix(desktop): coordinate cover drawer focus with a shared slot claim

**Category:** fix
**User Impact:** Opening a thread over agent activity (or activity over a
thread) now leaves keyboard focus inside the drawer that just opened. Before,
the outgoing drawer's deferred focus restore fired after the new drawer had
already taken focus and pushed focus into the covered channel, which is
`inert` — so focus landed on `<body>` with no visible focus ring and no
keyboard path back into the visible drawer.

**Problem:** Two review findings, one root cause. `CoverDrawer` decided at
teardown whether to restore focus by calling a caller-supplied predicate that
read thread view-mode state, so a presentation-only primitive was
interpreting a specific caller's presentation, and the predicate could not
see the replacement race at all. Separately, last-opened-wins was only
demonstrated through the resolver's fixed thread-beats-agent priority; the
e2e "exclusivity" test closed one drawer before opening the other and its
final step opened nothing, so no test exercised a real open-over-open
transition or asserted the replaced surface's URL param was cleared.

**Solution:** Replace the predicate with a module-level focus slot: a drawer
claims the slot when it captures focus and restores only if its claim is
still current, so any successor's claim — from any source, with no mount
ordering assumptions — invalidates the loser's pending restore. The thread
view-mode switch releases the slot from the thread side, keeping that
decision out of the primitive while leaving plain thread close identical.
Document the resolver's priority as a stale-simultaneous-param safety net
and test the open handlers directly for both orderings, which is where the
real rule lives.

<details>
<summary>File changes</summary>

**desktop/src/features/channels/lib/coverDrawerFocusSlot.ts**
New single-slot coordinator: `claimCoverDrawerFocus`,
`hasCoverDrawerFocusClaim`, `releaseCoverDrawerFocus`. A monotonic
generation answers "was I superseded?" without any drawer naming its
successor.

**desktop/src/features/channels/ui/CoverDrawer.tsx**
Drops the `shouldRestoreFocusOnClose` prop. Claims the slot on focus
capture and gates the deferred restore on still holding it.

**desktop/src/features/channels/ui/FocusThreadDrawer.tsx**
No longer reads thread view-mode state to decide focus restore.

**desktop/src/features/channels/ui/useThreadViewModeSwitch.ts**
Releases the focus slot when the switch places focus itself.

**desktop/src/features/channels/lib/channelAuxiliarySurface.ts**
Documents the priority order as a safety net for stale/simultaneous params,
not the product rule.

**desktop/src/features/channels/ui/CoverDrawerFocusHandoff.test.mjs**
Drives `CoverDrawer` in jsdom: plain close restores, a replaced drawer
leaves focus with its successor, a chain of replacements keeps only the
last, and a released slot leaves focus where the caller put it.

**desktop/src/features/channels/lib/coverDrawerFocusSlot.test.mjs**
Claim/supersede/release semantics, including that claims are never reused.

**desktop/src/features/channels/ui/useChannelAgentSessionExclusivity.test.mjs**
Tests the open handlers directly in both orderings, plus the breadcrumb
that keeps the replaced thread recoverable.

**desktop/tests/e2e/agent-activity-cover.spec.ts**
Rewrites the exclusivity test as genuine open-over-open transitions with no
closed intermediate: thread over activity via a `messageId` deep link,
activity over thread via the thread composer's activity bar (the one
ingress reachable while the channel is inert). Asserts the replaced param
is cleared and exactly one overlay exists.

</details>

Note on the e2e focus assertions: they are positive checks, not the
regression guard. Because the covered channel is `inert`, a wrongly-restored
focus is silently refused and lands on `<body>` rather than visibly stealing
focus — mutating the guard to a no-op still passes e2e. The discriminating
assertions therefore live in `CoverDrawerFocusHandoff.test.mjs`, where
disabling the guard fails 3 of 4 tests while the plain-close test keeps
passing.

Signed-off-by: ss-dev-00 <a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971@buzz.block.builderlab.xyz>
Co-authored-by: Bradley Axen <baxen@squareup.com>
Signed-off-by: Bradley Axen <baxen@squareup.com>

2026-08-24 14:18:49 -07:00