-
-
Notifications
You must be signed in to change notification settings - Fork 827
Upgrade emojibase
and twemoji
#7286
Upgrade emojibase
and twemoji
#7286
Conversation
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
Signed-off-by: Šimon Brandner <[email protected]>
7d1966e
to
9ae1787
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I don't think we can merge this because Twemoji hasn't been updated to Emoji 14 yet https://github.com/twitter/twemoji/releases. But LGTM otherwise.
Co-authored-by: Tulir Asokan <[email protected]>
This seems to also break autocompleting emojis with non-prefix strings, e.g. 🧐 ( |
I figured out the problem, the fields used by emoji autocompletion aren't type-checked: diff --git a/src/autocomplete/EmojiProvider.tsx b/src/autocomplete/EmojiProvider.tsx
index 960b0f9475..55d58a8953 100644
--- a/src/autocomplete/EmojiProvider.tsx
+++ b/src/autocomplete/EmojiProvider.tsx
@@ -75,7 +75,7 @@ export default class EmojiProvider extends AutocompleteProvider {
shouldMatchWordsOnly: false,
});
this.nameMatcher = new QueryMatcher(SORTED_EMOJI, {
- keys: ['emoji.annotation'],
+ keys: ['emoji.label'],
// For removing punctuation
shouldMatchWordsOnly: true,
}); |
Signed-off-by: Šimon Brandner <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonder if we can prove the typing here using something like https://stackoverflow.com/questions/45415436/nested-keyof-object-paths-using-dot-notation
…jibase Signed-off-by: Šimon Brandner <[email protected]>
Blocked on twitter/twemoji#515 |
Signed-off-by: Šimon Brandner <[email protected]>
Twemoji has now been upgraded, so after a review this should be ready to go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just pending an oob manual test of the new Twemoji fonts
After going through the emoji, it seems nothing is broken |
Type: task
Fixes emoji for
:D
and allows emoji for capital:P
This change is marked as an internal change (Task), so will not be included in the changelog.
Preview: https://61e5413fc5b56620e97d4035--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.