fix(desktop): scope composer autocomplete to focus - #6860
Conversation
Prevent inactive channel composers from rendering stale inline autocomplete state while a thread send toggles their shared disabled state. Preserve editor focus for composer mention controls and cover the thread-send regression end to end. Co-authored-by: Jitter <2b2e6415e748c35180847a37aadae06a11e30dddc76b784e7fd9354c4eb42e7a@buzz.block.builderlab.xyz> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The mid-send editable toggles emitted Tiptap's default synthetic update event with an unchanged doc, replaying onUpdate with stale text/cursor. That resurrected consumer state derived from it — most visibly reopening a mention menu the user had dismissed with Escape as soon as the drafted composer regained focus, and re-firing typing notifications for an untouched draft. Pass emitUpdate=false to both setEditable calls (real content changes dispatch their own transactions and still emit updates) and extend the thread-send E2E regression test to assert the dismissed menu stays closed after refocusing the drafted composer. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Requesting changes for one user-facing keyboard accessibility regression at exact head 1ffbbcb6caa92764e9539ec277d396768f3b7859 (base e8172b5baccd0f4a711f5e19a3bc4313708f1c78).
[P2] Keep the mention surface mounted while focus is inside its composer-owned controls
MessageComposer.tsx:814-815,889-904 now supplies no suggestions as soon as the ProseMirror editor loses focus. The mention surface itself contains keyboard-focusable Options and switch controls (MentionAutocomplete.tsx:192-212), but tabbing from the editor toward them fires the new editor blur callback (useRichTextEditor.ts:597-598). The empty suggestion array then resets options and unmounts the entire surface (MentionAutocomplete.tsx:86-90,138-140), including the control keyboard focus is moving to. Pointer activation is preserved by the new mousedown.preventDefault() paths, but keyboard and switch-control users lose access. That violates the repository’s WCAG 2.1 AA minimum (VISION.md:196-206).
Author action: Gate menu visibility on focus ownership by either the editor or that composer’s autocomplete controls (for example, composer-level focus-within/related-target ownership), while continuing to exclude the side-thread composer. Add a causal regression that opens mention autocomplete, tabs into Options and its switch, proves the surface stays mounted and operable, then moves focus to the thread composer and proves it hides. Keep the existing stale-reopen regression.
The systems path otherwise holds: TipTap focus/blur state and per-editor disable/restore ownership prevent an inactive main editor from reclaiming focus, and all three inline menus use the local gate. The added stale-reopen E2E is causal: the full changed spec passed 18/18; removing the focus condition made its new assertion fail; restoring exact-head bytes returned it green. A separate full desktop unit run passed 5,556/5,556. git diff --check is clean. At final preflight, Desktop Build, all Desktop Smoke shards, both Desktop Integration shards, Desktop Relay, DCO, and release-candidate checks were green; Desktop Core remained pending.
Verification ownership: author for the ownership fix and causal keyboard regression; reviewer/release tooling for native GUI observation and final CI. Native GUI evidence was unavailable on the shared review host and is a confidence gap, not additional author action.
The focus gate that keeps a background composer from rendering stale suggestion overlays lived in MessageComposer's render, so ForumComposer — which renders the same autocomplete components and toggles its own editable state during submission — was left ungated, and any future composer surface could silently reintroduce this class of bug. Move the gate into MentionAutocomplete, ChannelAutocomplete, and EmojiAutocomplete behind a required isEditorFocused prop, wire both composers through it, and cover the gate in the MentionAutocomplete unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The dismissed-mention-menu regression test asserted the main composer's contenteditable=false state inside a 500ms send window, while sibling tests use 1500-2000ms for equivalent mid-send checks — on a loaded CI runner the window could close before the first poll. Bump the fixture's sendMessageDelayMs to 1500 to match; the MutationObserver-based core regression check was already timing-independent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The focus-gate prop pushed MessageComposer.tsx three lines past the 1000-line ceiling. Extract its three suggestion overlays into a sibling component, mirroring the existing ForumComposerAutocompletes split, so the composer sheds the block rather than the gate raising the limit. Behavior is unchanged: the two agent-audience callbacks were each guarded by `audienceScope && editTarget == null`, so that predicate crosses the boundary once as `audienceControlsEnabled` and the child resolves both callbacks from it. Signed-off-by: Matt Toohey <contact@matttoohey.com>
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Requesting changes for the still-unresolved keyboard accessibility regression at exact head e0d3582a43e60208f00bff90bc142d50ecc2c1e5 (base e8172b5baccd0f4a711f5e19a3bc4313708f1c78).
[P2] Give keyboard users a path to the mention Options controls
The new head moves the editor-focus gate into the autocomplete components, but it does not extend focus ownership to their controls. MentionAutocomplete.tsx:146-148 still returns null as soon as richText.isFocused becomes false, before rendering the focusable Options button and switch at MentionAutocomplete.tsx:169-230; MessageComposerAutocompletes.tsx:74-89 passes only that editor-focused boolean.
More fundamentally, the real composer handler consumes Tab while mention autocomplete is open and selects the highlighted suggestion (useMentions.ts:884-939, wired by MessageComposer.tsx:725-757). Keyboard users therefore cannot Tab from the editor into Options at all. The unit named “does not intercept Tab from the editor” dispatches directly to an isolated input without the composer key handler (MentionAutocomplete.test.mjs:295-324), so it passes while omitting the behavior that causes the regression. The E2E delta only extends the stale-dismissed-menu journey and likewise never reaches or operates Options by keyboard. Calling these focusable controls primarily pointer-operated does not satisfy the repository’s WCAG 2.1 AA minimum (VISION.md:196-206).
Author action: implement composer focus ownership and keyboard behavior that includes the editor and its autocomplete controls while excluding sibling composers. Add a real composer E2E that opens mention autocomplete, reaches Options and its switch by keyboard, operates it while the surface remains mounted, then moves focus into the thread composer and proves the main menu hides. Keep the stale-menu regression.
The changed head does fix the stale-update mechanism: both setEditable transitions now use emitUpdate=false (useRichTextEditor.ts:614-638). At the exact clean head, the changed E2E spec passed 18/18 and the Desktop unit suite passed 5,557/5,557. Re-enabling default update emission causally failed the new refocus assertion by resurrecting the menu; restoring exact-head bytes returned the tree clean. git diff --check, macOS Desktop Build, both Integration shards, Relay, release candidate, and DCO are green; Desktop Core and Smoke shards were still pending at final preflight.
Author action: keyboard ownership fix and causal composer-level regression above.
Verification owner: author for that fix; reviewer/tooling for native visual and dedicated screen-reader follow-up; CI/release gate for final exact-head checks. The native/tooling gaps are not additional author action.
The focus gate that stops a background composer from rendering stale suggestions unmounts an overlay the moment its editor blurs, and a mousedown steals focus from a contenteditable unless the handler prevents its default. The controls inside these overlays already did; their containers did not, so any container-targeted press tore the overlay down mid-interaction — the mention list's padding ring on every platform, and its scrollbar wherever a non-overlay one is rendered (Windows WebView2, macOS with "Show scroll bars: Always"), plus the same holes in the channel and emoji lists. Prevent mousedown on each overlay's container, matching the idiom the inner controls already use. Not pointerdown: cancelling that would suppress the compatibility mouse events the suggestion rows select on. The options surface needed one more step. A label hands focus to its control from the click default action, which no mousedown guard can cancel, so clicking the "Automatically mention agents" text still blurred the editor and the overlay vanished before the forwarded click reached the switch. Cancel the label's forwarding and drive the switch directly, keeping the htmlFor association for assistive tech. Cover the container presses and the label in the MentionAutocomplete unit tests, and add an E2E regression test that presses the live mention list's padding ring and then its options label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent re-reviewed the changed head 0802f3a8da110846021d13ad032766b65691205f against base e8172b5baccd0f4a711f5e19a3bc4313708f1c78. The prior keyboard-accessibility blocker is resolved, and I found no remaining author-actionable defect.
The per-form focus ownership keeps sibling composers isolated while allowing focus to remain owned through the mention menu’s Options trigger and switch. Shift+Tab now routes from the editor to those controls without weakening forward-Tab mention selection; moving focus to the thread composer dismisses the stale main-composer menu. Pointer focus preservation and the controls’ button/switch accessibility semantics also remain intact.
Exact-head evidence:
- Desktop package tests: 5,561/5,561 passed.
- Focused persistent-audience E2E suite: 20/20 passed, including keyboard traversal, Options/switch operation and persistence, sibling-composer isolation, pointer behavior, reduced motion, and narrow layout.
- Two causal mutations failed at the intended ownership and Options-focus assertions, then passed after restoring the exact head.
- Desktop check passed with only pre-existing diagnostics outside this diff;
git diff --checkwas clean. - Browser accessibility snapshots exposed expanded
button "Options"andswitch "Automatically mention agents"; visible exact-head evidence confirmed the focused expanded control state.
Residual confidence gap: native Tauri/WKWebView and platform screen-reader traversal were not exercised. Browser keyboard, pointer, accessibility-tree, and exact-head visual evidence cover the changed interaction; no author action is requested for that gap. Changed-head GitHub Desktop Core and Smoke checks were still running when this review was submitted and remain the CI verification owner.
0802f3a to
4d9b41a
Compare
|
I'm not sure if the options of the mention popup should be keyboard accessible honestly. But either way its out of scope of this PR. Reverted the implementation of that |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent re-reviewed the force-pushed exact head 4d9b41a7db4eba954f38ad40d2369abc949fe426 against live base db514b153a8cb17631d6e80b91c95fdc45deb147.
Requesting changes because this head drops the previously approved keyboard-accessibility fix and its causal regression.
[P2] Restore keyboard access to mention Options and its switch
This force-pushed head is an ancestor of approved 0802f3a8…, not a descendant. Relative to that approved state it deletes useComposerFocusOwnership.ts, its tests, and tests/e2e/mention-options-keyboard.spec.ts; restores editor-only overlay gating (MessageComposerAutocompletes.tsx:26-75, MentionAutocomplete.tsx:143-148); and restores the real composer path that consumes every Tab, including Shift+Tab, as mention selection (MessageComposer.tsx:707-730, useMentions.ts:884-940). The isolated unit named “does not intercept Tab from the editor” mounts only MentionAutocomplete and therefore bypasses that parent key handler.
A clean exact-head probe restored the prior composer-level keyboard E2E: after Shift+Tab, the Options trigger was absent and the highlighted person had been selected instead. Keyboard and switch-control users therefore cannot reach or operate Options → Automatically mention agents, contrary to the repository’s WCAG 2.1 AA minimum (VISION.md:206).
Author action: restore per-composer focus ownership plus an accessible keyboard route (or equivalent behavior) that keeps the overlay mounted while Options/the switch owns focus, but hides it when a sibling thread composer receives focus. Restore a real composer E2E proving traversal, switch operation without intermediate unmount, and sibling-composer dismissal. Rebase/update onto the live base as well: GitHub currently reports mergeable: CONFLICTING, mergeStateStatus: DIRTY.
Other exact-head evidence:
- Desktop package suite: 5,558/5,558 passed.
- Desktop check passed with pre-existing diagnostics outside this diff;
git diff --checkwas clean. - Isolated focused runs passed the checked-in stale-send and container-press regressions 2/2; removing the new list-container
mousedown.preventDefault()causally failed its unit regression before exact-head restoration. - One shared-port broad E2E run lost its preview server and produced connection-refused failures; an isolated rerun cleared the relevant checked-in rows, so that harness collision is not attributed as another author defect.
- Native WKWebView/platform screen-reader traversal was not run; that is a reviewer confidence gap, not additional author action.
Verification owner: author for the keyboard fix, causal regression, and conflict resolution; reviewer/CI for fresh-head full gates; release/native tooling for platform AT follow-up.
2 participants
Issue
I would send a message in the thread side pane and the main chat's composer would then open its at-mention completion UI:
mention.menu.shows.at.wrong.time.2.mov
Summary
Root cause
The main channel composer and thread composer share the channel sending state. A thread send toggled the inactive main editor disabled and enabled, causing programmatic editor updates to recompute its stale mention query and remount the mention menu.
Verification