fix(reactions): show reactors list and render emoji glyph#102
Merged
Conversation
The reactor dialog always showed "No one yet." even when a reaction had a non-zero count. The list-users endpoint returns bare user-id strings, but the client filtered the response with whereType<AccordUser>(), discarding every string. Resolve each id to an AccordUser (with an id-only fallback) so reactors render and the list matches the badge count. Also send the unicode emoji glyph rather than the shortcode name as the REST token for add/remove/list, matching how the server keys reactions by emoji_name (so the same emoji aggregates across clients), and render the glyph in the dialog header instead of the literal ":shortcode:". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two bugs in the message reaction UI, found from a screenshot where a
👍reaction showed1on the badge but the reactor dialog said "No one yet." and the header readReacted with :thumbsup:.GET .../reactions/{emoji}returns{ "data": [user_id strings] }— bare ID strings, not user objects. The client ran the response throughdeserializeArray(AccordUser.fromJson)(which only mapsMapelements) and thenwhereType<AccordUser>(), so every string was discarded → always empty.reactionUsersnow parses the IDs and resolves each to anAccordUserviaclient.users.fetch, with an id-only stub fallback so a failed fetch doesn't drop a reactor (keeping the list consistent with the badge count).thumbsup) as the REST token. The server keys reactions byemoji_namewith no shortcode↔glyph conversion, so a client storingthumbsupwould create a separate aggregate from a client storing the glyph👍. Add/remove/list now send the resolved glyph, and the dialog header renders the glyph instead of the literal:shortcode:(custom emoji still show:name:).Why
Verified against the
accordserversource:add/remove/listbind the URL-decoded emoji path param directly to theemoji_namecolumn, and the list handler returns user-id strings. Sending the glyph is the canonical form (server tests use URL-encoded glyphs) and ensures reactions aggregate across clients.Reviewer notes
emojiId != null) are unchanged: token staysname:idand the header stays:name:.thumbsupwon't match a glyph (👍) list query, so legacy seeded rows may still show "No one yet." Reactions added through the app store the glyph and round-trip correctly.Test plan
:thumbsup:.:name:in the header.🤖 Generated with Claude Code