Fix/make-emojis-larger-in-other-message-contexts #15611

josemak25 · 2023-03-02T17:43:07Z

Details

This solution covers several related scenarios:

Scenario 1

Action Performed:

Add an emoji with other plain text characters in a message in the composer.

Expected Result:

The emoji should be larger (19pt) compared to the default size for text (15pt) in the composer. The emoji and text should appear vertically centered with respect to each other, such that the baseline of the emoji is below the plain text and the middle of the text vertically aligns with the middle of the emoji (like the right-hand-side of #4114 (comment).

Solution does not cover this case as handling emojis inside the input is a bit tricky

Scenario 2

Action Performed:

Send a message with emojis and text.

Expected Result:

The emoji should be larger (19pt) compared to the default size for text (15pt) in the chat history. The emoji and text should appear vertically centered with respect to each other, such that the baseline of the emoji is below the plain text and the middle of the text vertically aligns with the middle of the emoji (like the right-hand-side of #4114 (comment)

Solution covers this case

Scenario 3

Action Performed

Draft a message (of any length) with only an emoji.

Expected Result:

The emoji should appear very large (27pt) in the composer.

Solution does not cover this case as handling emojis inside the input is a bit tricky

Scenario 4

Action Performed

Send a message (of any length) with only emoji.

Expected Result:

The emoji should appear very large (27pt) in the report history.

Solution covers this case

Scenario 5

Action Performed

Go to Settings -> Profile and add emoji(s) to your last name 😎

Expected Result:

The emoji should appear larger (19pt) than the rest of the text (15pt) in the text input. The emoji and text should appear vertically centered with respect to each other, such that the baseline of the emoji is below the plain text and the middle of the text vertically aligns with the middle of the emoji (like the right-hand-side of #4114 (comment)

Solution does not cover this case as handling emojis inside the input is a bit tricky

Scenario 6

Action Performed

Go to Settings -> Profile and add emoji(s) to your last name 😎.
Save your profile.
Open the Settings RHP and look at your name:

Expected Result:

The emoji should appear larger (21pt) than the rest of the text (17pt) in the text input. The emoji and text should appear vertically centered with respect to each other, such that the baseline of the emoji is below the plain text and the middle of the text vertically aligns with the middle of the emoji (like the right-hand-side of #4114 (comment)

Solution covers this case

Scenario 7

Action Performed

Go to Settings -> Profile and add emoji(s) to your last name 😎.
Save your profile.
Open a chat with another user and send them a message. Take a look at your message in the report history.

Expected Result:

The emoji should appear larger (19pt) than the rest of the test (15pt). The emoji and text should appear vertically centered with respect to each other, such that the baseline of the emoji is below the plain text and the middle of the text vertically aligns with the middle of the emoji (like the right-hand-side of #4114 (comment)

Solution covers this case

Fixed Issues

PROPOSAL: #4114

Tests

The emoji and text should appear vertically centered with respect to each other, such that the baseline of the emoji is below the plain text and the middle of the text vertically aligns with the middle of the emoji
Draft a message (of any length) with only emoji the emoji should appear very large (27pt) in the composer.
Go to Settings -> Profile and add emoji(s)[😎] to your last name or first name and save your profile. now open the settings RHP and look at your name, the emoji appear to be bigger in size than the text.

Verify that no errors appear in the JS console

Offline tests

QA Steps

The emoji and text should appear vertically centered with respect to each other, such that the baseline of the emoji is below the plain text and the middle of the text vertically aligns with the middle of the emoji
Draft a message (of any length) with only emoji the emoji should appear very large (27pt) in the composer.
Go to Settings -> Profile and add emoji(s)[😎] to your last name or first name and save your profile. now open the settings RHP and look at your name, the emoji appear to be bigger in size than the text.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Chrome Web

Chrome.Web.Recording.mov

Safari Web

Safari.Web.Recording.mov

Mobile Web - Chrome

Mobile.Web.-.Chrome.mov

Mobile Web - Safari

Mobile.Web.-.Safari.mp4

Desktop

Desktop.mov

iOS

iOS.mp4

Android

Android.mov

MelvinBot · 2023-03-02T17:43:13Z

@roryabraham @parasharrajat One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

src/components/TextEmoji.js

src/styles/styles/index.js

src/styles/variables/index.js

src/styles/styles/styles.android.js

roryabraham · 2023-03-02T19:06:44Z

src/styles/styles/styles.android.js

+    },
+};
+
+export default {...baseStyles, ...styles};


Since baseStyles is a very large object and shouldn't be imported anywhere else, I'd recommend using Object.assign for better performance:

export default Object.assign(baseStyles, styles);

Yea I used lodash assign method as required by the app standards

export default _.assign(baseStyles, styles);

did you forget to push this change?

src/styles/styles/styles.ios.js

roryabraham · 2023-03-02T19:07:09Z

src/styles/styles/styles.ios.js

+    },
+};
+
+export default {...baseStyles, ...styles};


Suggested change

export default {...baseStyles, ...styles};

export default Object.assign(baseStyles, styles);

Yea I used lodash assign method as required by the app standards

export default _.assign(baseStyles, styles);

src/components/TextEmoji.js

roryabraham · 2023-03-02T19:11:24Z

src/libs/EmojiUtils.js

+        return false;
+    }
+    const trimmedMessage = Str.replaceAll(message.replace(/ /g, ''), '\n', '');
+    return Boolean(trimmedMessage.match(CONST.REGEX.EMOJIS));


Suggested change

return Boolean(trimmedMessage.match(CONST.REGEX.EMOJIS));

return CONST.REGEX.EMOJIS.test(trimmedMessage);

roryabraham · 2023-03-02T19:41:09Z

src/CONST.js

@@ -806,6 +806,7 @@ const CONST = {

        // eslint-disable-next-line max-len, no-misleading-character-class
        EMOJIS: /[\p{Extended_Pictographic}\u200d\u{1f1e6}-\u{1f1ff}\u{1f3fb}-\u{1f3ff}\u{e0020}-\u{e007f}\u20E3\uFE0F]|[#*0-9]\uFE0F?\u20E3/gu,
+        EMOJI_SURROGATE: /(?:[^\uD800-\uDBFF]|^)[\uDC00-\uDFFF]|[\uD800-\uDBFF](?![\uDC00-\uDFFF])/,


Can you explain what this regex represents? I was not able to figure it out

From the algorithm, we were expected to capture emojis in a text, though while doing this we need to be careful when we capture emojis that are surrogate pairs.
Surrogate pair Emojis are emojis that can be made up of multiple values that are overlayed on each other to get the final displayed representation.
An example of this is the cloud face emoji made up of 2 different emoji (face emoji and cloud emoji) combined together.
If we split the cloud face emoji we would end up with both separate emojis, so from our algorithm, we need to identify these sorts of surrogate pairs and have them combined back together to maintain the original emoji

Read more on surrogate pair characters

Awesome, thanks for that explanation.

Maybe what we should do then is:

Update the emoji regex to capture chunks of 1 or more emojis

For each chunk containing emojis, use the emoji surrogate regex to capture and "squash" surrogate pairs

Also we should include some comments to explain what this regex is about

@roryabraham

Update the emoji regex to capture chunks of 1 or more emojis

How do I do this from your implementation?

the d flag works for the web but not on android and ios platforms as they both crash

Have logs for this crash? Could this be because the d flag is not implemented in the Hermes JS engine, or something else?

Edit: Yep, it seems that the hasIndices flag is not implemented in Hermes: https://github.com/facebook/hermes/blob/417190297242f14fed540378390d28926c2a7f7e/include/hermes/Regex/RegexTypes.h#L213-L343. Shoot ... did not see that coming.

Edit 2: Created facebook/hermes#932, but we need to decide if we should HOLD this PR on that or not

@roryabraham @parasharrajat

the d flag works for the web but not on android and ios platforms as they both crash

Have logs for this crash? Could this be because the d flag is not implemented in the Hermes JS engine, or something else?

Edit: Yep, it seems that the hasIndices flag is not implemented in Hermes: https://github.com/facebook/hermes/blob/417190297242f14fed540378390d28926c2a7f7e/include/hermes/Regex/RegexTypes.h#L213-L343. Shoot ... did not see that coming.

Edit 2: Created facebook/hermes#932, but we need to decide if we should HOLD this PR on that or not

@roryabraham @parasharrajat this would prolong this task by extra few days should we try to fix this regex issue.
I would suggest we go with the former implementation which worked perfectly avoiding the need for this regex d fix also if it helps I would put a comment on it explaining every step of the algorithm and also remove all the other functions that aren't needed.

/** * Get all the emojis in the message * @param {String} text * @returns {Array} */ function getAllEmojiFromText(text) { // return an empty array when no text is passed if (!text) { return []; } // Unicode Character 'ZERO WIDTH JOINER' (U+200D) is usually used to join surrogate pair together without breaking the emoji const zeroWidthJoiner = '\u200D'; // https://codepoints.net/U+200D?lang=en const splittedMessage = text.split(''); const result = []; let wordHolder = ''; // word counter let emojiHolder = ''; // emoji counter const setResult = (word, isEmoji = false) => { // for some weird reason javascript sees the empty string `"` as a word with `length = 1` // this is caused after splitting the text empty spaces are added to both the start and the end of all emojis and text // given the empty space is close to a text then its length is counted as 0 // while if it's before or after an emoji then it's counted as 1, so we remove the word where word.length equals 1 // NOTE: this does not affect a single character element example typing `[i | J]` cause after splitting its empty word.length is calculated as 0 if (!isEmoji && word.length === 1) { return; } result.push({text: word, isEmoji}); }; _.forEach(splittedMessage, (word, index) => { if (CONST.REGEX.EMOJI_SURROGATE.test(word) || word === zeroWidthJoiner) { setResult(wordHolder); wordHolder = ''; emojiHolder += word; } else { setResult(emojiHolder, true); emojiHolder = ''; wordHolder += word; } if (index === splittedMessage.length - 1) { setResult(emojiHolder, true); setResult(wordHolder); } }); // remove none text characters like '' only return where text is a word or white space ' ' return _.filter(result, res => res.text); } /** * Validates that this message contains has emojis * * @param {String} message * @returns {Boolean} */ function hasEmojis(message) { const splitText = getAllEmojiFromText(message); return _.find(splitText, chunk => chunk.isEmoji) !== undefined; } /** * Validates that this message contains only emojis * * @param {String} message * @returns {Boolean} */ function containsOnlyEmojis(message) { const splitText = getAllEmojiFromText(message); return _.every(splitText, chunk => chunk.isEmoji); }

roryabraham

There are a number of other changes I want to see included in this PR.

Unit Test Coverage

EmojiUtils desperately needs some unit test coverage. That's almost always a good idea when we are dealing with complex regexes. Furthermore, it's probably a good place to start to establish a baseline before making the other changes listed here.

Unused Method

Next, I noticed that EmojiUtils.trimEmojiUnicode is unused, so we should remove it

Refactoring EmojiUtils

Third, I noticed some inefficiencies and unnecessarily complex code in the following three methods:

containsOnlyEmoji
hasEmojis
getAllEmojisFromText

I think we should refactor these methods such that the string is only traversed by a regex exactly once when any one of the three methods is called.

I've done some research, and I think this should be possible in a few steps.

Step 1

Refactor CONST.REGEX.EMOJIS such that it will match not just a single emoji at a time, but 1 or more. This means that if we have an input string like so:

Hello world 🙂🙂🙂 what a great day ☀️

const input = 'Hello world U+1F600U+1F600U+1F600 what a great day U+2600';

then we do this:

input.split(CONST.REGEX.EMOJIS);

We would get a result like this:

['Hello world ', ':slightly_smiling_face::slightly_smiling_face::slightly_smiling_face:', ' what a great day ', ':sunny:']

result = ['Hello world ', 'U+1F600U+1F600U+1F600', ' what a great day ', 'U+2600']

Furthermore, it should ignore whitespace or 0-length characters if they are surrounded by emoji. So doing a split on this input:

const input = 'Hello world U+1F600     U+1F600     U+1F600 what a great day U+2600'

would produce this output:

result = ['Hello world ', 'U+1F600     U+1F600     U+1F600', ' what a great day ', 'U+2600']

Step 2

Add the d flag to CONST.REGEX.EMOJIS such that the result of a regular expression match should contain the start and end indices of the substrings of each capture group.
Wrap the regex in a capture group such that the chunks of 1 or more potentially-whitespace-separated emojis are captured.

Step 3

Refactor EmojiUtils.getAllEmojisFromText so that it works something like this (this hasn't been properly tested, but should be enough to give you the idea).

note: when the regex has the global flag then it becomes stateful where exec just gives you one match at a time.
note: while exec is used multiple times, the entire string is only traversed once.

function getAllEmojiFromText(text) {
    if (!text) {
        return [];
    }

    const splitText = [];
    let reResult;
    let lastMatchIndexEnd = 0;
    do {
        // Look for an emoji chunk in the string
        reResult = CONST.REGEX.EMOJIS.exec(text);

        // If we reached the end of the string and it wasn't included in a previous match
        // the chunk between the end of the last match and the end of the string is plain text
        if (reResult === null && lastMatchIndexEnd !== text.length - 1) {
            splitText.push({
                text: text.slice(lastMatchIndexEnd, text.length - 1),
                isEmoji: false,
            })
            continue;
        }

        const matchIndexStart = reResult.indices[0][0];
        const matchIndexEnd = reResult.indices[0][1];

        // The chunk between the end of the last match and the start of the new one is plain-text
        splitText.push({
            text: text.slice(lastMatchIndexEnd, matchIndexStart),
            isEmoji: false,
        });
        
        // Everything captured by the regex itself is emoji + whitespace
        splitText.push({
            text: text.slice(matchIndexStart, matchIndexEnd),
            isEmoji: true,
        });

        lastMatchIndexEnd = matchIndexEnd;
    } while (reResult !== null);

    return splitText;
}

Step 4

Refactor EmojiUtils.hasEmojis like so:

function hasEmojis(message) {
    const splitText = getAllEmojiFromText(message);
    return _.find(splitText, chunk => chunk.isEmoji) !== undefined;
}

Step 5

Refactor EmojiUtils.containsOnlyEmojis like so:

function containsOnlyEmojis(message) {
    const splitText = getAllEmojiFromText(message);
    return _.every(splitText, chunk => chunk.isEmoji);
}

src/styles/styles/styles.native.js

src/styles/variables/variables.native.js

src/styles/styles/index.js

src/components/MenuItem.js

josemak25 · 2023-03-03T18:58:13Z

There are a number of other changes I want to see included in this PR.

Unit Test Coverage

EmojiUtils desperately needs some unit test coverage. That's almost always a good idea when we are dealing with complex regexes. Furthermore, it's probably a good place to start to establish a baseline before making the other changes listed here.

Unused Method

Next, I noticed that EmojiUtils.trimEmojiUnicode is unused, so we should remove it

Refactoring EmojiUtils

Third, I noticed some inefficiencies and unnecessarily complex code in the following three methods:

containsOnlyEmoji

hasEmojis

getAllEmojisFromText

I think we should refactor these methods such that the string is only traversed by a regex exactly once when any one of the three methods is called.

I've done some research, and I think this should be possible in a few steps.

Step 1

Refactor CONST.REGEX.EMOJIS such that it will match not just a single emoji at a time, but 1 or more. This means that if we have an input string like so:

Hello world 🙂🙂🙂 what a great day ☀️
const input = 'Hello world U+1F600U+1F600U+1F600 what a great day U+2600';
then we do this:
input.split(CONST.REGEX.EMOJIS);
We would get a result like this:

['Hello world ', '🙂🙂🙂', ' what a great day ', '☀️']
result = ['Hello world ', 'U+1F600U+1F600U+1F600', ' what a great day ', 'U+2600']
Furthermore, it should ignore whitespace or 0-length characters if they are surrounded by emoji. So doing a split on this input:
const input = 'Hello world U+1F600     U+1F600     U+1F600 what a great day U+2600'
would produce this output:
result = ['Hello world ', 'U+1F600     U+1F600     U+1F600', ' what a great day ', 'U+2600']
Step 2

Add the d flag to CONST.REGEX.EMOJIS such that the result of a regular expression match should contain the start and end indices of the substrings of each capture group.

Wrap the regex in a capture group such that the chunks of 1 or more potentially-whitespace-separated emojis are captured.

Step 3

Refactor EmojiUtils.getAllEmojisFromText so that it works something like this (this hasn't been properly tested, but should be enough to give you the idea).

note: when the regex has the global flag then it becomes stateful where exec just gives you one match at a time.

note: while exec is used multiple times, the entire string is only traversed once.
function getAllEmojiFromText(text) {
    if (!text) {
        return [];
    }

    const splitText = [];
    let reResult;
    let lastMatchIndexEnd = 0;
    do {
        // Look for an emoji chunk in the string
        reResult = CONST.REGEX.EMOJIS.exec(text);

        // If we reached the end of the string and it wasn't included in a previous match
        // the chunk between the end of the last match and the end of the string is plain text
        if (reResult === null && lastMatchIndexEnd !== text.length - 1) {
            splitText.push({
                text: text.slice(lastMatchIndexEnd, text.length - 1),
                isEmoji: false,
            })
            continue;
        }

        const matchIndexStart = reResult.indices[0][0];
        const matchIndexEnd = reResult.indices[0][1];

        // The chunk between the end of the last match and the start of the new one is plain-text
        splitText.push({
            text: text.slice(lastMatchIndexEnd, matchIndexStart),
            isEmoji: false,
        });
        
        // Everything captured by the regex itself is emoji + whitespace
        splitText.push({
            text: text.slice(matchIndexStart, matchIndexEnd),
            isEmoji: true,
        });

        lastMatchIndexEnd = matchIndexEnd;
    } while (reResult !== null);

    return splitText;
}
Step 4

Refactor EmojiUtils.hasEmojis like so:
function hasEmojis(message) {
    const splitText = getAllEmojiFromText(message);
    return _.find(splitText, chunk => chunk.isEmoji) !== undefined;
}
Step 5

Refactor EmojiUtils.containsOnlyEmojis like so:
function containsOnlyEmojis(message) {
    const splitText = getAllEmojiFromText(message);
    return _.every(splitText, chunk => chunk.isEmoji);
}

@roryabraham from my test with your solution adding the d flag works for the web but not on android and ios platforms as they both crash

EMOJIS: /[\p{Extended_Pictographic}\u200d\u{1f1e6}-\u{1f1ff}\u{1f3fb}-\u{1f3ff}\u{e0020}-\u{e007f}\u20E3\uFE0F]|[#*0-9]\uFE0F?\u20E3/dgu,

Also, the solution does not satisfy surrogate pair emojis, more context on this here #15611 (comment)

Crash screenshot for mobile platforms with `d flag`

Screenshot of suggested refactor not covering surrogate pair emojis on the web

Screenshot of my initial PR covering surrogate pair emojis on all platforms

roryabraham · 2023-03-03T19:23:14Z

the d flag works for the web but not on android and ios platforms as they both crash

Have logs for this crash? Could this be because the d flag is not implemented in the Hermes JS engine, or something else?

Edit: Yep, it seems that the hasIndices flag is not implemented in Hermes: https://github.com/facebook/hermes/blob/417190297242f14fed540378390d28926c2a7f7e/include/hermes/Regex/RegexTypes.h#L213-L343. Shoot ... did not see that coming.

Edit 2: Created facebook/hermes#932, but we need to decide if we should HOLD this PR on that or not

fbmal7 · 2023-04-11T17:10:38Z

@fabioh8010 I just ran 262 against latest main and it reported no errors. It reported 19274 passing tests, so it seems like for some reason you are running more tests.

roryabraham · 2023-04-11T17:16:20Z

Maybe we should continue discussion on this topic either in an upstream draft PR or in facebook/hermes#932 ?

fabioh8010 · 2023-04-11T18:00:04Z

@roryabraham @fbmal7 Yes, it's better. I'll create a draft PR tomorrow and will let you know, so we can have continue discussions there.

fabioh8010 · 2023-04-12T11:03:37Z

@fbmal7 @roryabraham

I published a Draft PR here -> facebook/hermes#968

fabioh8010 · 2023-04-14T10:39:32Z

Update: PR is being reviewed, I left some questions there after the first review and I'll address all of them once I get feedback.

fabioh8010 · 2023-04-18T19:04:31Z

Update: PR reviewed and some changes made, also I made it ready to review.

fabioh8010 · 2023-04-28T10:45:46Z

Update: PR reviewed again and I need to make some more changes to keep the implementation closer to ECMAScript specifications. I'll be OOO for some days and will return on May 4th to start working on these changes in the following days.

twisterdotcom · 2023-05-15T08:01:51Z

Got some issues Held on this. How is the review going?

fabioh8010 · 2023-05-15T08:25:24Z

Hi @twisterdotcom , I addressed the last review comments on Friday and I'm now waiting for his feedback.

Latest update -> facebook/hermes#968 (comment)

fabioh8010 · 2023-06-09T08:12:01Z

Update: Addressed more requested changes on the PR, waiting for another review now.

pecanoro · 2023-06-12T17:06:04Z

Is there any reason why this is on HOLD?

roryabraham · 2023-06-13T02:42:44Z

Yes, @pecanoro it's on HOLD for a new regex feature in the Hermes JS engine: facebook/hermes#968

More context in this comment

fabioh8010 · 2023-06-20T21:53:59Z

Update: Looks like we are coming to an end in the Hermes PR 🎉 Meta's team is discussing how to merge those changes.

twisterdotcom · 2023-06-21T11:34:33Z

@fabioh8010 yessss!

fabioh8010 · 2023-07-01T17:03:31Z

Update: Still waiting for Meta's team decision about how to merge the Hermes PR.

jeet-dhandha · 2023-07-28T22:51:44Z

Don't know much context of the PR but if you are looking for the Regex to get all the emojis.

Check this comment:
#23153 (comment)

fabioh8010 · 2023-07-28T22:57:50Z

@jeet-dhandha The Hermes engine doesn't support unicode property escapes -> https://hermesengine.dev/docs/regexp#supported-syntax

So this type of regex will crash on mobile platforms.

fabioh8010 · 2023-07-28T22:58:40Z

Update: Hermes PR is expected to land on RN 0.73 -> facebook/hermes#968 (comment)

roryabraham · 2023-10-26T17:47:47Z

This is blocked on the RN 0.73 release. They just published 0.73-rc.3 a few days ago

pecanoro · 2023-12-18T15:18:07Z

It seems RN 0.73 has finally been released!

roryabraham · 2023-12-18T16:09:15Z

Yes, and we're upgrading E/App to 0.73.1 in #31558

pecanoro · 2024-01-22T12:13:58Z

Just wondering, can we finally remove the HOLD in this PR?

roryabraham · 2024-01-24T07:08:00Z

Yep, I think we can lift the HOLD here 🎉

twisterdotcom · 2024-01-24T12:31:35Z

Who is responsible for this now? @avijeetpandey?

roryabraham · 2024-01-24T17:45:12Z

Not sure, asked in slack: https://expensify.enterprise.slack.com/archives/C03UK30EA1Z/p1706080610772919

roryabraham · 2024-01-24T19:08:45Z

Closing this out per this comment

josemak25 added 9 commits March 2, 2023 18:35

add platform based styles

7702aa6

add platform based variables

9f61d17

add emoji surrogate regex to emoji regex constants

d869d26

use TextEmoji on initial settings page

799d4ec

use TextEmoji on details page

17aaa12

use TextEmoji on report action item fragment

713a6cf

add TextEmoji util methods

35881af

use TextEmoji on menu item component

a1d7e40

add isMenuItem props to TextEmoji to fix tooltip menuitem item

7f60e6c

josemak25 requested a review from a team as a code owner March 2, 2023 17:43

melvin-bot bot requested review from parasharrajat and roryabraham and removed request for a team March 2, 2023 17:43

josemak25 mentioned this pull request Mar 2, 2023

Fix/make emojis larger in other contexts #15516

Closed

54 tasks

roryabraham requested changes Mar 2, 2023

View reviewed changes

parasharrajat suggested changes Mar 3, 2023

View reviewed changes

src/styles/styles/styles.native.js Outdated Show resolved Hide resolved

src/styles/variables/variables.native.js Outdated Show resolved Hide resolved

src/styles/styles/index.js Outdated Show resolved Hide resolved

avijeetpandey reviewed Mar 3, 2023

View reviewed changes

src/components/MenuItem.js Outdated Show resolved Hide resolved

roryabraham mentioned this pull request Mar 3, 2023

hasIndices flag for regex facebook/hermes#932

Closed

josemak25 added 2 commits March 6, 2023 10:48

add PR changes and left comments on suggested changes

a75333a

add fix to empty char check

445ac39

josemak25 requested review from parasharrajat and roryabraham and removed request for parasharrajat and roryabraham March 6, 2023 12:14

roryabraham changed the title ~~Fix/make-emojis-larger-in-other-message-contexts~~ [HOLD] Fix/make-emojis-larger-in-other-message-contexts Apr 11, 2023

roryabraham changed the title ~~[HOLD] Fix/make-emojis-larger-in-other-message-contexts~~ Fix/make-emojis-larger-in-other-message-contexts Jan 24, 2024

roryabraham closed this Jan 24, 2024

fabioh8010 mentioned this pull request Apr 24, 2024

Emojis larger in other contexts than just single character messages #40692

Merged

53 tasks

	export default {...baseStyles, ...styles};
	export default Object.assign(baseStyles, styles);

	return Boolean(trimmedMessage.match(CONST.REGEX.EMOJIS));
	return CONST.REGEX.EMOJIS.test(trimmedMessage);

Fix/make-emojis-larger-in-other-message-contexts #15611

Fix/make-emojis-larger-in-other-message-contexts #15611

Conversation

josemak25 commented Mar 2, 2023 • edited by github-actions bot Loading

Details

Scenario 1

Action Performed:

Expected Result:

Scenario 2

Action Performed:

Expected Result:

Scenario 3

Action Performed

Expected Result:

Scenario 4

Action Performed

Expected Result:

Scenario 5

Action Performed

Expected Result:

Scenario 6

Action Performed

Expected Result:

Scenario 7

Action Performed

Expected Result:

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

MelvinBot commented Mar 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roryabraham Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josemak25 Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

roryabraham left a comment

Choose a reason for hiding this comment

Unit Test Coverage

Unused Method

Refactoring EmojiUtils

Step 1

Step 2

Step 3

Step 4

Step 5

josemak25 commented Mar 3, 2023

Unit Test Coverage

Unused Method

Refactoring EmojiUtils

Step 1

Step 2

Step 3

Step 4

Step 5

roryabraham commented Mar 3, 2023 • edited Loading

fbmal7 commented Apr 11, 2023

roryabraham commented Apr 11, 2023 • edited Loading

fabioh8010 commented Apr 11, 2023

fabioh8010 commented Apr 12, 2023

fabioh8010 commented Apr 14, 2023

fabioh8010 commented Apr 18, 2023

fabioh8010 commented Apr 28, 2023

twisterdotcom commented May 15, 2023

fabioh8010 commented May 15, 2023

fabioh8010 commented Jun 9, 2023

pecanoro commented Jun 12, 2023

roryabraham commented Jun 13, 2023 • edited Loading

fabioh8010 commented Jun 20, 2023 • edited Loading

josemak25 commented Mar 2, 2023 •

edited by github-actions bot

Loading

roryabraham Mar 3, 2023 •

edited

Loading

josemak25 Mar 6, 2023 •

edited

Loading

roryabraham commented Mar 3, 2023 •

edited

Loading

roryabraham commented Apr 11, 2023 •

edited

Loading

roryabraham commented Jun 13, 2023 •

edited

Loading

fabioh8010 commented Jun 20, 2023 •

edited

Loading