Add complete reaction toggles to the message menu - #185
Conversation
b3c474c to
b618613
Compare
Co-authored-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz> Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
b618613 to
37b5107
Compare
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Source review of 665a8c3b7899dd23ec80958b70257d49585a5ff5 against 222605777f8808617026ea33a0aad266bdaff4c6: no substantiated code blockers found. Reviewed reaction toggles, own-duplicate removal, author checks, event-local emoji identity, failure/retry and channel/DM/thread synchronization, with independent deletion-boundary review.
This is not merge readiness or approval. Exact-head CI still fails the unchanged warm-switch budget: 120.5 ms versus <100 ms. All four browser journey shards passed, but that does not resolve the performance gate. Resolve or explicitly disposition the measurement failure with causal/equivalent-state evidence; do not weaken the budget merely to make this review green.
Source-only on Blox; no PR code, local tests, packaged app, or live-relay workflow was executed. Existing CI was read, not rerun.
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
…embers-dialog * morganm/channel-members-support: Preserve member-add recovery across dialog lifetimes fix: restore avatar presence controls and active-input sensing (#198) Add legacy diff messages with inline and expanded viewing (#202) Edit the latest own message with Up in the existing composer (#192) Add complete reaction toggles to the message menu (#185) Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz> # Conflicts: # src/shared/design-system/ui/Dialog.tsx
…o-player-polish * origin/main: (38 commits) Fix diff content fallback, keyboard scrolling and edit selection (#205) Standardize form controls and field feedback across Buzz (#174) Keep image review downloads and external opens distinct (#144) Verify media review comments (#166) Follow system appearance (#210) Add rich composer formatting and spoiler rendering (#203) feat: show roster-backed channels and managed instances in profiles (#188) Add new direct message flow (#156) Remove Home, start in Messages, and keep Channels enabled (#194) fix: restore avatar presence controls and active-input sensing (#198) Add legacy diff messages with inline and expanded viewing (#202) Edit the latest own message with Up in the existing composer (#192) Add complete reaction toggles to the message menu (#185) feat: add persistent community navigation rail (#191) test: add margin to warm-switch performance gate (#195) Add composer attachments and compatible media preparation (#183) Add reply and copying to the shared message menu (#182) fix: avoid idle workspace re-renders from activity and label churn (#186) feat: add devtools trace capture to web profiling (#180) Add optional channel templates, teams and personal group defaults (#181) ... Signed-off-by: Zach Marley <zmarley@squareup.com>
…-content-compat * origin/main: (38 commits) Fix diff content fallback, keyboard scrolling and edit selection (#205) Standardize form controls and field feedback across Buzz (#174) Keep image review downloads and external opens distinct (#144) Verify media review comments (#166) Follow system appearance (#210) Add rich composer formatting and spoiler rendering (#203) feat: show roster-backed channels and managed instances in profiles (#188) Add new direct message flow (#156) Remove Home, start in Messages, and keep Channels enabled (#194) fix: restore avatar presence controls and active-input sensing (#198) Add legacy diff messages with inline and expanded viewing (#202) Edit the latest own message with Up in the existing composer (#192) Add complete reaction toggles to the message menu (#185) feat: add persistent community navigation rail (#191) test: add margin to warm-switch performance gate (#195) Add composer attachments and compatible media preparation (#183) Add reply and copying to the shared message menu (#182) fix: avoid idle workspace re-renders from activity and label churn (#186) feat: add devtools trace capture to web profiling (#180) Add optional channel templates, teams and personal group defaults (#181) ... Signed-off-by: Zach Marley <zmarley@squareup.com> # Conflicts: # docs/channels.md
What this does
Adds three personalized quick reactions and the full emoji picker to the message menu. Reactions show a count of people; selecting your reaction again removes it instead of adding another copy. Custom emoji keep the image that was attached to the original reaction.
This is PR 2 of 3. The menu foundation (#182) has merged; this branch is now rebased onto
mainat222605777f8808617026ea33a0aad266bdaff4c6. Message editing and other management controls remain in PR 3.Why it matters
People can react from channels, DMs, and threads without leaving the conversation. Failed changes expose a retry action, including when the last reaction disappears. Counts and your selected state stay synchronized between the timeline and thread.
How it works
Reuses the existing emoji contribution, design-system controls, session delivery queue, and retry/reconnect behavior. Grouped reactions retain authors and event IDs so removal deletes only your events, including duplicates. A shared author-only deletion operation also supports PR 3's message management; the broker admits its bounded shape without replacing workflow-deletion validation.
Quick choices use this viewer/community's local usage history, with three defaults and unavailable custom emoji filtered out. Mounted shortcuts do not reshuffle on each selection; new row mounts, catalog changes, reloads, or cross-window preference changes can refresh them. The menu stays visible while its picker is open, using the trigger's existing expanded state rather than adding another popup owner.
Latest shared-picker update
At
7c925972aca0001ff58d0e601554cc1317d6e145, reaction buttons remain eagerly available, while the emoji plugin coordinates one active picker. Inactive rows no longer mount picker state/popover machinery. Switching targets replaces the picker; Escape restores focus only to the active, connected, enabled trigger. Removed/disabled rows release the picker. Composer behavior and quick-choice ranking are unchanged.Validation on this clean commit: complete reactions/emoji/GIF browser files 12/12 passed across Chromium and WebKit; full Vitest 2,486/2,486 passed in 239 files; mandatory push hooks passed (types, 1,348 related tests, design-system guards, formatting/lint and security checks). First push was blocked by the unchanged MessageActionBar keyboard-focus unit test; the full suite and subsequent unmodified hook run passed. This is an observed intermittent failure, not a claimed repair.
One browser scenario was added (two engine cases), no cases removed: real pointer/keyboard retargeting, single visible portal, Escape focus return, remount and archive cleanup. Browser-only justification is actual popup positioning, pointer interaction and focus lifecycle. The shared-owner exclusivity assertion has not been mutation-tested against the parent; no fail-then-pass claim for that new assertion. Independent read-only review cleared the bounded lifecycle changes.
Local pre-commit probe on the same production delta: three serial runs per engine retained immediately present hover buttons; warm switching medians were 39.0 ms Chromium / 63.5 ms WebKit. This does not establish a hosted-CI performance fix. Main now independently contains #195, which retains a 100 ms target but uses a 200 ms failure ceiling; this PR does not edit that gate. A green hosted result must not be attributed solely to the picker change.
Hosted checks for this new head are pending. PR is ready for review, not asserted merge-ready. No merge performed. Live relay and packaged-app acceptance remain unverified.
Historical verification (previous head)
Previous pushed head:
665a8c3b7899dd23ec80958b70257d49585a5ff5; the following results predate the shared-picker update.At this head, mandatory hooks passed: TypeScript, 1,348 related tests in 91 files, design-system types/guards, staged formatting/lint, and sadscan. Hosted run https://github.com/block/buzz-app/actions/runs/35936296156 completed: JavaScript (2,486 tests in 239 files), Rust/tool integration, all four Chromium/WebKit journey shards, and DCO passed. Browser measurements failed on the second warm Beta switch: 120.5 ms against the unchanged <100 ms requirement. The dependent CI-required check therefore failed. That run was not merge-ready; the PR is now ready for review with a newer head.
At its direct parent
37b5107e3eb4fa2e66085b48a6db704029e4f4f5, the complete messages/presence/reactions/message-actions browser files passed 28/28 in Chromium and WebKit; isolated channel-opening measurements passed 4/4 across both engines. The only subsequent change is Biome formatting in the thread navigation fixture, repairing the hosted formatting failure. Browser behavior was not changed by that formatting commit.The rebased fixture now advertises reaction removal, scopes the media-comment picker to the exact reply, and selects an unowned reaction while retaining signed event/target and canonical thread-root assertions. The modified-link test observes that Buzz leaves the browser default uncanceled and does not route internally. It deliberately does not test loading a modified-click background tab: an app-free cross-origin Chromium crash was reproduced. Real unmodified external popup navigation remains covered. Independent read-only review cleared both bounded repairs. No timeout or assertion-count relaxation.
The previous hosted run also exposed intermittent large-thread presence timing; that production/test path remains unchanged. Both hosted WebKit shards now pass, but one green run does not establish deterministic timing.
Performance diagnosis: the base main run (
35934216275, base22260577) measured warm switches at 42.5–50.6 ms; this PR run measured 83.7/120.5 ms before failing. Cross-run numbers alone do not isolate causality. A local research-only comparison on the same checkout, with 3x CPU throttling and 12 switches, measured median warm paint 92.25 ms with quick reaction controls versus 76.75 ms with just those controls omitted (first-visible medians 87.85/72.60 ms). This implicates control rendering as additional work, but does not prove removing that work alone would meet hosted CI. Temporary probes/omissions were removed; no assertions or time limits were changed in committed code. At that historical head, no production performance fix had been made.Previous UI follow-up evidence at
b3c474ca:Review follow-up: quick emoji now use the existing small icon buttons, matching adjacent actions; removing a focused last reaction returns focus to that message’s persistent menu trigger; empty reaction rows add no spacing while retry remains mounted and unclipped. No custom control styling or new browser cases. The spacing and keyboard-removal assertions failed before the fixes in both engines. At this head, all 8 cases in the full reactions/message-actions browser files passed in Chromium/WebKit, including failed first-reaction retry and failed-removal rollback; mandatory types, 1,345 related tests and design-system checks passed. Princess Donut’s targeted read-only review found no remaining issue.
The broader verification below was run at the preceding head
fc187bc; it is not a claim that the full suite was rerun for this UI-only follow-up.message-actions,reactions,emoji, andtypeaheadbrowser files. Covers real menu/picker wiring, channel/thread synchronization, DM add/remove, actual signed reaction/deletion publication, custom emoji, failed-removal retry after remount, focus return, touch, and narrow/intermediate/wide layouts.Browser coverage changes
Before the shared-picker update above, no browser cases were added or removed. Extended the existing reactions journey for first reactions, true removal, retry, historic custom-emoji identity, picker visibility and responsive bounds. Extended the existing channel/thread and DM message-action journeys for representative full-app wiring. Browser-only justification: real portal/pointer/focus/layout behavior and integration through the production broker; author/retry/reconnect matrices remain in lower-layer tests. No replacement coverage was deleted.
Fail-then-pass evidence: the picker visibility assertion failed in both engines before the CSS fix; the row-identity regression failed when reaction event identity was not compared. Both pass at the checked head. Initial full-app reactions failed because the fixture only admitted messages/read-state; the fixture now strictly validates signed reaction/removal targets, channel, and ownership before retention/live echo. Assertions and error allowlists were not relaxed.
Local macOS timing, two workers, Chromium + WebKit: the affected menu/reaction files passed in 11.2s (browser test durations 0.7–3.7s each). Before fixture admission they aborted on unexpected kind 7, so there is no meaningful passing baseline or speedup claim. Emoji/typeahead files passed in 34.2s. These are local functional-run timings including fixture setup, not isolated setup measurements or hosted CI performance results.
Remaining gates
The latest hosted checks and required human/code-owner review remain gates; the historical warm-switch failure is not claimed fixed by local measurements. Full browser suite and native builds were not run locally; existing CI owns broader automated validation. Packaged-app and live-relay acceptance remain unverified. No merge performed.
Visual check
Synthetic fixture: personalized shortcuts, grouped custom reaction, and the existing picker remaining open beside the message.
Narrow full-app layout: