Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Deactivated custom emoji offered in autocomplete #1113

Closed
gnprice opened this issue Dec 8, 2024 · 0 comments · Fixed by #1114
Closed

Deactivated custom emoji offered in autocomplete #1113

gnprice opened this issue Dec 8, 2024 · 0 comments · Fixed by #1114
Assignees
@gnprice
Labels
a-compose Compose box, autocomplete, attaching files/images
Milestone
M5: Launch

Comments

@gnprice
Copy link
Member

gnprice commented Dec 8, 2024

A custom emoji can be deactivated:

When that happens, it's still valid for existing messages and reactions; but the server will reject new uses, and so it shouldn't be offered in the UI.

Currently we don't take account of that, so we offer deactivated custom emoji the same way as those still active.

The effect is conspicuous on chat.zulip.org, because there are a substantial number of deactivated emoji (57 of them):

$ <tmp/register.json jq '.realm_emoji[] | .deactivated' | sort | uniq -c
    131 false
     57 true

Particularly so after #1112 (implementing #1068), since custom emoji are ranked ahead of Unicode emoji (other than the "popular" emoji).
@gnprice gnprice added the a-compose Compose box, autocomplete, attaching files/images label Dec 8, 2024
@gnprice gnprice added this to the M5: Launch milestone Dec 8, 2024
@gnprice gnprice self-assigned this Dec 9, 2024
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Dec 9, 2024
@gnprice
emoji [nfc]: Make realmEmoji internal to EmojiStoreImpl
2c276b8 
It's a bit of a trap for the unwary (... like me last month, in
ecd2cb5, causing zulip#1113), because it includes deactivated realm emoji
as well as active ones.
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Dec 9, 2024
@gnprice
emoji: Exclude deactivated realm emoji from autocomplete
f09aab1 
Fixes zulip#1113.
@gnprice gnprice mentioned this issue Dec 9, 2024
Merged
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Dec 9, 2024
@gnprice
emoji [nfc]: Compare emoji base ordering with web's, in detail
3da80fd 
I spent some time reverse-engineering in full detail the behavior web
has for building its list of possible emoji to present in the UI (and
so for the order that emoji results have when they don't differ on
any of the criteria used at the ranking step after applying a query).

The first output of that study was discovering zulip#1113, fixed earlier
in this branch.  This records the rest.
@gnprice gnprice mentioned this issue Dec 9, 2024
Merged
chrisbobbe pushed a commit that referenced this issue Dec 9, 2024
@gnprice @chrisbobbe
emoji [nfc]: Make realmEmoji internal to EmojiStoreImpl
29fa03d 
It's a bit of a trap for the unwary (... like me last month, in
ecd2cb5, causing #1113), because it includes deactivated realm emoji
as well as active ones.
chrisbobbe pushed a commit that referenced this issue Dec 9, 2024
@gnprice @chrisbobbe
emoji: Exclude deactivated realm emoji from autocomplete
661fa53 
Fixes #1113.
chrisbobbe pushed a commit that referenced this issue Dec 9, 2024
@gnprice @chrisbobbe
emoji [nfc]: Compare emoji base ordering with web's, in detail
489a699 
I spent some time reverse-engineering in full detail the behavior web
has for building its list of possible emoji to present in the UI (and
so for the order that emoji results have when they don't differ on
any of the criteria used at the ranking step after applying a query).

The first output of that study was discovering #1113, fixed earlier
in this branch.  This records the rest.
shivanshsharma13 pushed a commit to shivanshsharma13/zulip-flutter that referenced this issue Dec 13, 2024
@gnprice @shivanshsharma13
emoji [nfc]: Make realmEmoji internal to EmojiStoreImpl
11c7328 
It's a bit of a trap for the unwary (... like me last month, in
ecd2cb5, causing zulip#1113), because it includes deactivated realm emoji
as well as active ones.
shivanshsharma13 pushed a commit to shivanshsharma13/zulip-flutter that referenced this issue Dec 13, 2024
@gnprice @shivanshsharma13
emoji: Exclude deactivated realm emoji from autocomplete
7c0396d 
Fixes zulip#1113.
shivanshsharma13 pushed a commit to shivanshsharma13/zulip-flutter that referenced this issue Dec 13, 2024
@gnprice @shivanshsharma13
emoji [nfc]: Compare emoji base ordering with web's, in detail
27d0037 
I spent some time reverse-engineering in full detail the behavior web
has for building its list of possible emoji to present in the UI (and
so for the order that emoji results have when they don't differ on
any of the criteria used at the ranking step after applying a query).

The first output of that study was discovering zulip#1113, fixed earlier
in this branch.  This records the rest.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gnprice gnprice

Labels
a-compose Compose box, autocomplete, attaching files/images
Projects
Flutter app
Status: Done
Milestone
M5: Launch
Development

Successfully merging a pull request may close this issue.

Exclude deactivated realm emoji from autocomplete gnprice/zulip-flutter
1 participant
@gnprice