Skip to content

msglist: Adding 🙂 emoji reaction fails #1495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chrisbobbe opened this issue May 2, 2025 · 2 comments · May be fixed by #1506
Open

msglist: Adding 🙂 emoji reaction fails #1495

chrisbobbe opened this issue May 2, 2025 · 2 comments · May be fixed by #1506
Assignees
Labels
a-msglist The message-list screen, except what's label:a-content server-11 Things new in Zulip Server 11.0

Comments

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented May 2, 2025

Reproduced on CZO just now:

  1. Long-press a message
  2. Tap the 🙂 emoji in the list of emoji at the top of the message action sheet
  3. See error dialog (unexpected):
Image
"zulip_version":"10.0-550-g2182f40bd5"
"zulip_feature_level":380
@chrisbobbe chrisbobbe changed the title msglist: Adding 🙂 emoji fails msglist: Adding 🙂 emoji reaction fails May 2, 2025
@chrisbobbe chrisbobbe added the a-msglist The message-list screen, except what's label:a-content label May 2, 2025
@gnprice
Copy link
Member

gnprice commented May 2, 2025

Yeah, we should fix this.

Presumably this is due to the recent server change to make U+1F642 🙂 be called :slight_smile: instead of :smile::

The fix will probably involve updating this code:

  static final _popularCandidates = _generatePopularCandidates();

  static List<EmojiCandidate> _generatePopularCandidates() {
    // …
    return [
      // This list should match web:
      //   https://github.com/zulip/zulip/blob/83a121c7e/web/shared/src/typeahead.ts#L22-L29
      candidate('1f44d', '👍', ['+1', 'thumbs_up', 'like']),
      candidate('1f389', '🎉', ['tada']),
      candidate('1f642', '🙂', ['smile']),

The first step will be some refactoring to make that no longer static — it'll need to depend on zulipFeatureLevel.

@gnprice gnprice added the server-11 Things new in Zulip Server 11.0 label May 2, 2025
@gnprice gnprice added this to the M5a: Launch blockers milestone May 2, 2025
@chrisbobbe chrisbobbe self-assigned this May 6, 2025
@tomlin7
Copy link
Contributor

tomlin7 commented May 8, 2025

Hi there, I have a workaround for this issue. But I see this has already been assigned to the author himself. Let me know if you've implemented this already.

chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 8, 2025
For zulip#1495, we'll want to keep the hard-coded list of emoji codes but
get the names from ServerEmojiData. This refactor prepares for that
by separating where we source the codes and the names.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 8, 2025
…t it

This brings the tests closer to being representative, because this
data (and more) is part of the app's setup for an account on
startup, via fetchServerEmojiData.

For zulip#1495, the app will start depending on this data for the names
of popular emoji. So, prepare for that by filling in the data in
tests that would break without it.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 8, 2025
The server change in zulip/zulip#34177, renaming `:smile:` to
`:slight_smile:`, broke the corresponding reaction button in the
message action sheet. We've been sending the add/remove-reaction
request with the old name 'smile', which modern servers reject.

To fix, take the popular emoji names from ServerEmojiData, so that
we'll use the correct name on servers before and after the change.

API-design discussion:
  https://chat.zulip.org/#narrow/channel/378-api-design/topic/.23F1495.20smile.2Fslight_smile.20change.20broke.20reaction.20button/near/2170354

Fixes: zulip#1495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-msglist The message-list screen, except what's label:a-content server-11 Things new in Zulip Server 11.0
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

3 participants