Skip to content

Commit

Permalink
[lib] Make sure all default emojis pass onlyOneEmojiRegex
Browse files Browse the repository at this point in the history
Summary:
Turns out that [macOS](mathiasbynens/emoji-regex#28 (comment)) [appends](mathiasbynens/emoji-regex#68) the `U+FE0F` the character to some Unicode emojis when you select them from the native OS emoji selector. It's not clear why Apple does this, or why it only happens for a certain set of emoji.

This still counts as [valid emoji Unicode](mathiasbynens/emoji-regex#28 (comment)). However, our `onlyOneEmojiRegex` thinks it's two emojis.

Our implementation of `onlyOneEmojiRegex` involves introspecting into the RegExp string that `emoji-regex` uses, and is not an officially supported approach by that package. `emoji-regex` supports matching emojis in text, and checking if the text includes only emoji. But checking for precisely one emoji is more complicated, and our approach (which is basically just extracting the raw RegExp and putting it inside of `/^()$/`) doesn't work in some scenarios where `U+FE0F` is suffixed.

Luckily we don't use the native macOS emoji selector in any of our UIs, but it does look like @Ginsu used it to select some of the emojis. The diff adds a unit test to make sure all of the default emojis pass `onlyOneEmojiRegex`, and fixes all failing emojis.

Test Plan: I noticed that a test username of `at4` would match up with an anchor emoji as the default, and the anchor emoji was failing to be set. After this diff everything worked

Reviewers: ginsu, atul

Reviewed By: atul

Subscribers: tomek, ginsu

Differential Revision: https://phab.comm.dev/D8145
  • Loading branch information
Ashoat committed Jun 7, 2023
1 parent 8d9e6fd commit 746427b
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/shared/avatar-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ const defaultEmojiAvatars: $ReadOnlyArray<ClientEmojiAvatar> = [
{ color: selectedThreadColors[0], emoji: '🌹', type: 'emoji' },
{ color: selectedThreadColors[1], emoji: '🌸', type: 'emoji' },
{ color: selectedThreadColors[7], emoji: '🌻', type: 'emoji' },
{ color: selectedThreadColors[3], emoji: '⭐', type: 'emoji' },
{ color: selectedThreadColors[3], emoji: '⭐', type: 'emoji' },
{ color: selectedThreadColors[4], emoji: '🌟', type: 'emoji' },
{ color: selectedThreadColors[5], emoji: '🍏', type: 'emoji' },
{ color: selectedThreadColors[6], emoji: '🍎', type: 'emoji' },
Expand Down Expand Up @@ -213,7 +213,7 @@ const defaultEmojiAvatars: $ReadOnlyArray<ClientEmojiAvatar> = [
{ color: selectedThreadColors[5], emoji: '🛶', type: 'emoji' },
{ color: selectedThreadColors[6], emoji: '⛵️', type: 'emoji' },
{ color: selectedThreadColors[7], emoji: '🚤', type: 'emoji' },
{ color: selectedThreadColors[8], emoji: '⚓', type: 'emoji' },
{ color: selectedThreadColors[8], emoji: '⚓', type: 'emoji' },
{ color: selectedThreadColors[6], emoji: '🏰', type: 'emoji' },
{ color: selectedThreadColors[0], emoji: '🎡', type: 'emoji' },
{ color: selectedThreadColors[1], emoji: '💎', type: 'emoji' },
Expand Down Expand Up @@ -345,6 +345,7 @@ function useENSResolvedAvatar(
}

export {
defaultEmojiAvatars,
getRandomDefaultEmojiAvatar,
getDefaultAvatar,
getAvatarForUser,
Expand Down
13 changes: 13 additions & 0 deletions lib/shared/emojis.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// @flow

import { defaultEmojiAvatars } from './avatar-utils.js';
import { onlyOneEmojiRegex } from './emojis.js';

describe('onlyOneEmojiRegex', () => {
Expand All @@ -22,4 +23,16 @@ describe('onlyOneEmojiRegex', () => {
it('should not match for (that is 🔥)', () => {
expect('that is 🔥').not.toMatch(onlyOneEmojiRegex);
});

it('should match all defaultEmojiAvatars', () => {
for (const emojiAvatar of defaultEmojiAvatars) {
const { emoji } = emojiAvatar;
expect(emoji).toMatch(onlyOneEmojiRegex);
}
});

it('should not match when U+FE0F suffixed', () => {
// See D8145 for more context
expect('⚓️').not.toMatch(onlyOneEmojiRegex);
});
});

0 comments on commit 746427b

Please sign in to comment.