[hold for #68311] iOS: Custom emoji overlays text in LHN

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.1.95-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @s77rt
Slack conversation (hyperlinked to channel name): #Expensify Bugs

Action Performed:

1. Open any report
2. Send a long message containing the global create emoji e.g. Test Test Test. It was a good idea. They are agreed that it must be the perfect approach because with a green button :global_create: everything look good.
3. Go back to LHN
4. Verify that the last message is displayed correctly

Expected Result:

Last message to be displayed correctly

Actual Result:

The global create icon overlays the text at the end

Workaround:

Unknown

Platforms:

Select the officially supported platforms where the issue was reproduced:

• Android: App
• Android: mWeb Chrome
• iOS: App
• iOS: mWeb Safari
• iOS: mWeb Chrome
• Windows: Chrome
• MacOS: Chrome / Safari
• MacOS: Desktop

Platforms Tested:

On which of our officially supported platforms was this issue tested:

• Android: App
• Android: mWeb Chrome
• iOS: App
• iOS: mWeb Safari
• iOS: mWeb Chrome
• Windows: Chrome
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[samranahm]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Custom emoji overlays text in LHN

What is the root cause of that problem?

This happen because we use view wrapper on emoji in IOS here

App/src/pages/home/report/comment/TextWithEmojiFragment/index.ios.tsx

Lines 15 to 22 in 4d29406

	{processedTextArray.map(({text, isEmoji}, index) =>
	isEmoji ? (
	<View
	// eslint-disable-next-line react/no-array-index-key
	key={index}
	>
	<Text style={[styles.emojisWithTextFontSizeAligned, containsCustomEmoji(text) && styles.customEmojiFontAlignment]}>{text}</Text>
	</View>

that should only be applied if the emoji is within visible text in LHN and if the custom emoji is after the visible text in alternate text the view wrapper should not applied

What changes do you think we should make in order to solve the problem?

We should refector the component to explicitly check the containsOnlyCustomEmoji and only apply view wrapper on custome_emoji, add alignCustomEmoji as argument here

function TextWithEmojiFragment({message = '', style, alignCustomEmoji = false}: TextWithEmojiFragmentProps & {alignCustomEmoji?: boolean}) {

<Text style={style}>
    {processedTextArray.map(({text, isEmoji}, index) => {
        if (isEmoji && containsOnlyCustomEmoji(text)) {
            if (alignCustomEmoji) {
                return (
                    <Text
                        // eslint-disable-next-line react/no-array-index-key
                        key={index}
                        style={[styles.emojisWithTextFontSizeAligned, styles.customEmojiFontAlignment]}
                    >
                        {text}
                    </Text>
                );
            }
            return (
                <View
                    // eslint-disable-next-line react/no-array-index-key
                    key={index}
                >
                    <Text style={[styles.emojisWithTextFontSizeAligned, containsCustomEmoji(text) && styles.customEmojiFontAlignment]}>{text}</Text>
                </View>
            );
        }
        return convertToLTR(text);
    })}
</Text>

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

What alternative solutions did you explore? (Optional)

we can explicitly check char length and only apply view when required

const charsBeforeEmoji = processedTextArray
    .slice(0, index)
    .reduce((sum, part) => sum + part.text.length, 0);


    // don't render render the normal emoji in view wrapper if the charsBeforeEmoji is more then 50 and we have both custom emoji and normal emoji in text
    if (alignCustomEmoji && charsBeforeEmoji > 50) {
        return null;
    }

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

Results

IOS.mp4

[joekaufmanexpensify]

Left a comment, I think we should just fix this in #68311

[samranahm]

@joekaufmanexpensify we've multiple cases where this bug occurs we should have it separately for C+ review and QA checks.

[joekaufmanexpensify]

I'm not sure I understand the reasoning. Couldn't a C+ review it and QA test it in the existing PR? It seems very related. If we're adding this emoji to the message in chat intro and it isn't appearing correctly there in the LHN preview, that seems like something we want to address in the PR.

[samranahm]

But this actually isn’t part of #68311, since the scope of that PR is to add the global_create emoji in the chat intro lines. We’ve done fixing the wrapper for ios in past and ended up with a regression, so ideally this issue should be handled separately so we can test it thoroughly.

[joekaufmanexpensify]

Hmm, I still don't understand why we can't test this thoroughly if it's in the above PR. Further, if we put this emoji in the chat intro message, then wouldn't this bug appear in the LHN when we display that updated intro message before there are any real messages in the chat?

[s77rt]

Looks like we don't use this custom emoji elsewhere where the bug could be reproduced. I could see why this should be fixed at #68311 level. Let's fix in the existing PR then cc @samranahm

[samranahm]

@s77rt I'll update the PR shortly. Testing the solution.