Skip to content

fix(desktop): highlight typed @ mentions and let Enter choose them - #7323

Closed
loganj wants to merge 1 commit into
fix/mention-picker-stack-5bd14d7a-4from
fix/mention-editor-split-359a5af1
Closed

loganj wants to merge 1 commit into
fix/mention-picker-stack-5bd14d7a-4from
fix/mention-editor-split-359a5af1

Conversation

@loganj

@loganj loganj commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

🤖

Summary

Fixes three native-editor bugs that surfaced while typing @ mentions:

  • Typing a registered name now highlights it as a mention immediately. Before, the mention highlight (the colored label) only caught up when the suggestion list opened or refreshed — so typing another teammate's exact registered name, or deleting characters off the end of one, left it looking like plain text. Highlight decorations are now rebuilt from each text change's own document state, without depending on the picker.
  • Enter with suggestions open chooses the suggestion — and only that. Before, pressing Enter while the mention list was open could both pick the highlighted row and split the paragraph, leaving a stray blank line. The editor now consumes Enter whenever the autocomplete is open, so the list's own handler chooses and the paragraph never splits.
  • The caret settles correctly after a long team-wrapped mention. After a team mention — whose wrapper adds a closing parenthesis around a member label that may itself end in a long key — the caret could settle one boundary off, so the next typed character landed inside the mention instead of after it. The boundary scan now recognizes the wrapper; internal spaces in labels and deliberate ArrowLeft/click movement are still respected.

Part of the mention-chooser stack on the shared #7190 recovery prerequisite: #7190#7196#7323#7197#7239#7240. This feature stack remains separate from #7191#7192. This PR builds directly on #7196.

Related issue

Continues the merged mention-editor work from #7124 (authorize remote mentions at publication) and #7128 (preserve spacing after multi-word mentions). No separate tracking issue for this slice.

Testing

  • Unit tests: mentionHighlightExtension.test.mjs extended for incremental typing of a second registered label, suffix deletion, separator survival across qualified-label refresh, and the team wrapper around key-ending labels.
  • Browser tests: mentions.spec.ts covers highlight-while-typing without opening the picker, Enter choosing without a paragraph split, and caret settlement; persistent-agent-audience.spec.ts carries the one-time-DM fixture, corrected to assert no premature signing, an empty post-send draft, and the exact final recipient.
  • Earlier independent-root evidence is historical; current shared-recovery composition validation is recorded below.

Extraction validation update

  • This stack builds on the shared fix(desktop): keep your draft editable when an @ mention cannot be sent #7190 recovery baseline, separately from 🤖 fix(desktop): consolidate fresh mention action admission and stable choices #7191🤖 fix(desktop): show an agent in @ search after you add it to a channel #7192. Chooser, cold-error settlement and ranking production behavior are unchanged by the fixture repairs.
  • Desktop lint/format, TypeScript, explicit shared-base file-size checks and E2E builds pass. Targeted send-flow/mention-presentation checks pass. Earlier package 6035/6035 and isolated browser 8/8 remain historical evidence for unchanged semantic inputs, not fresh runs of this composition.
  • The fresh composition probe passed 7/8 initially. The Welcome failure was traced to mock parity: create dropped the starter team ID, and add-members omitted the normal membership event. The fixture now preserves team ID, deliberately seeds a same-name collision and delivers that event. Exactly three starter creates prove reuse. The original ambiguous-submit error, retained draft, no-publication and exact-current-starter completion assertions remain.
  • Welcome now passes separately on the earliest owning fix(desktop): check access again when you select or pin an @ mention #7196 prefix and final descendant with matching E2E builds. Removing only the fixture membership event fails the roster precondition; restoring it passes. This is not one combined clean eight-test run and does not establish a production freshness dependency on lane B.
  • Earlier full browser sweep remains 138/141, with separate separator corrections 2/2 and editor 4/4. No fresh full-suite or all-prefix runtime claim.
  • Independent Welcome fixture review passed; earlier helper/shared-base and semantic reviews are retained. DCO passes and this PR is conflict-free. Current-head CI passed. The stack remains gated; no ready/merge claim.

Move the existing later-slice DM-loading fixture correction to the editor prefix. Assert no premature signing and exact final recipients without treating a one-time mention as an automatic address.

Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj
loganj force-pushed the fix/mention-editor-split-359a5af1 branch from ed5a2f4 to 75f61d7 Compare September 8, 2026 18:50
@loganj
loganj marked this pull request as ready for review September 9, 2026 15:59
@loganj
loganj requested a review from a team as a code owner September 9, 2026 15:59

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking findings at exact head 75f61d7d473829fe11795f96870e20ca8da87d99 against base b4a4f6f2436a41245bdd8fdbd38004e55c894467.

I traced the changed mention-decoration and Enter-selection behavior through the composer, send flow, recipient extraction, and membership revalidation boundaries:

  • The Enter handler consumes ProseMirror splitBlock while autocomplete is active, leaving the composer wrapper to resolve the current suggestion (desktop/src/features/messages/lib/useRichTextEditor.ts:389-408, desktop/src/features/messages/MessageComposer.tsx:731-772).
  • Decoration recomputation examines changed textblocks across step documents, and team-wrapper matching remains bounded to zero or one closing ) (desktop/src/features/messages/lib/mentionHighlightExtension.ts:377-402,612-662,686-706).
  • Visual highlighting does not manufacture recipient identity. Recipient extraction remains tied to selected/current member candidates (desktop/src/features/messages/lib/extractMentionPubkeys.ts:110-145), with identities captured before async preparation (desktop/src/features/messages/lib/useMentionSendFlow.ts:747-755).
  • Membership/authorization is checked during preparation and again immediately before publish (desktop/src/features/messages/lib/useMentionSendFlow.ts:459-465,576-598); agent admission fails closed (desktop/src/features/messages/lib/agentMentionRevalidation.ts:49-95). Composer reuse/navigation is guarded by captured owner/revision state, with failure restoration (desktop/src/features/messages/lib/useMentionSendFlow.ts:401-429,747-819).

Exact-head validation:

  • git diff --check — pass.
  • just desktop-check — pass; warnings were outside this PR diff.
  • just desktop-typecheck — pass.
  • just desktop-build — pass.
  • Changed helper tests — 50/50 pass.
  • Full Desktop package rerun — 6040/6040 pass. Earlier runs each exposed a different unrelated timing failure that passed on rerun; I treat those as flaky confidence signals, not PR-caused gate failures.
  • Isolated Playwright Stream and Forum Enter journeys — 2/2 pass, asserting one paragraph, no signing, and exact @bob insertion.
  • Exact-head CI — Desktop Core, four Desktop Smoke E2E shards, Desktop relay/integration E2E, macOS/Windows builds, Semgrep, zizmor, and DCO are green.

Residual risk: native Tauri/WebView input delivery and accessibility semantics were not observed in a native harness. This PR introduces no new interactive component or semantic owner, and browser/package/CI evidence passed, so this remains non-blocking reviewer/tooling risk.

Author action: none.
Verification owner: reviewer/tooling for optional native WebView/accessibility observation; none for code.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Both assigned review lanes clear exact head 75f61d7d473829fe11795f96870e20ca8da87d99.

The Enter interception correctly prevents ProseMirror paragraph splitting while the composer selects the current suggestion. Decoration rebuilding covers changed text blocks and recipient identity remains derived from selected current candidates, with authorization and membership revalidated before publication and stale composer operations fenced by owner/revision.

Validation: full Desktop package rerun 6,040/6,040; typecheck, lint, and build; changed helper tests 50/50; isolated Enter Playwright journeys 2/2; clean git diff --check. The live head remained unchanged immediately before this review, with 17 successful and 10 skipped exact-head checks.

Residual risk: no native WebView/accessibility observation. One unrelated timing test failed in an initial run in each lane and passed on rerun; neither reproduced. These are confidence gaps, not author defects.

@loganj

loganj commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by consolidated #7191 (new head 0973992d, parent unchanged #7190 bb8cca21).

This PR's highlight-typed-mention and Enter-selection behavior is included in #7191's consolidated landing unit: highlight settlement, native selection/deletion and duplicate-recipient witnesses are retained there, so this content no longer needs to land as an independent slice between #7196 and #7197.

Closing as superseded is not a claim that this branch's prior review state or CI is transferred to the new unit. The branch fix/mention-editor-split-359a5af1 and this PR's text are retained. Active stack: #7190#7191#7192.

@loganj loganj closed this Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants