[Due for payment 2025-02-25] [$250] Android - Chat - Emoji sent at the bottom of multiline message, overlaps the word above

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.70-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Open the Expensify app.
2. Open any chat.
3. Compose a message with several lines and place an emoji in the last line.
4. Send the message.
5. Verify that the message is correctly displayed and that the emoji is not mixed with another line´s words.

Expected Result:

Emoji placed at the bottom of a multiline message should be displayed complete and without overlapping any of the above lines.

Actual Result:

Emoji placed at the bottom of a multiline message, is displayed overlapping words of the line above it, when sent to chat.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6682623_1733199780274.Emoji_multiline.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021864062988355959260
• Upwork Job ID: 1864062988355959260
• Last Price Increase: 2024-12-10
• Automatic offers:
• dominictb | Contributor | 105309249

Issue Owner

Current Issue Owner: @slafortune

[melvin-bot]

Triggered auto assignment to @slafortune (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.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021864062988355959260

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha (External)

[dominictb]

Proposal

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

Android - Chat - Emoji sent at the bottom of multiline message, overlaps the word above

What is the root cause of that problem?

We use marginVertical: -7 in

App/src/styles/index.ts

Lines 1744 to 1747 in ce282d3

	emojisWithTextFontSizeAligned: {
	fontSize: variables.fontSizeEmojisWithinText,
	marginVertical: -7,
	},

And use it in

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

Lines 15 to 19 in ce282d3

	isEmoji ? (
	<Text
	// eslint-disable-next-line react/no-array-index-key
	key={index}
	style={styles.emojisWithTextFontSize}

so the emoji overlaps the words

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

We shouldn't apply marginTop: -7 so we can update this line

App/src/styles/index.ts

Line 1746 in ce282d3

marginVertical: -7,

to

marginBottom: -7

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

What alternative solutions did you explore? (Optional)

Result

N/A

[JoshIri360]

Proposal

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

Emojis placed at the bottom of a multiline message are displayed overlapping the words of the line above when sent to chat.

What is the root cause of that problem?

The root cause is that the last line of text does not adjust its height dynamically to fit the size of the emoji, leading to visual misalignment and overlap.

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

Add a zero-width space (\u200B) as a buffer: This ensures proper rendering and spacing for the emoji in the last line.

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

What alternative solutions did you explore? (Optional)

Adjusting marginBottom or lineHeight did not resolve the issue effectively as it added spacing to the top of emojis not at the bottom.

Result

[dominictb]

The reason we add marginVertical: -7 is to fix the following issue (The emoji is too close with the above text)

Adding marginBottom: -7 is enough since it can fix the issue and doesn't change the previous code too much

[bernhardoj]

Proposal

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

The emoji overlaps with the message above it.

What is the root cause of that problem?

The emoji overlaps because we apply a vertical margin of -7.

App/src/styles/index.ts

Lines 1744 to 1747 in 361a024

	emojisWithTextFontSizeAligned: {
	fontSize: variables.fontSizeEmojisWithinText,
	marginVertical: -7,
	},

This was most likely added as a workaround of the alignment issue which happens if there is a nested view inside a text.

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

Lines 16 to 22 in 361a024

	isEmoji ? (
	<View
	// eslint-disable-next-line react/no-array-index-key
	key={index}
	>
	<Text style={styles.emojisWithTextFontSizeAligned}>{text}</Text>
	</View>

While the reason we added the nested View is probably to fix the emoji cut-off at the top on iOS, which is a common issue we faced before that requires increasing the line height.

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

First, remove the vertical margin here.

App/src/styles/index.ts

Lines 1744 to 1747 in 361a024

	emojisWithTextFontSizeAligned: {
	fontSize: variables.fontSizeEmojisWithinText,
	marginVertical: -7,
	},

Then, in TextWithEmojiFragment, apply a bigger line height for each line that contains emojis.

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

Lines 9 to 29 in 361a024

	function TextWithEmojiFragment({message = '', style}: TextWithEmojiFragmentProps) {
	const styles = useThemeStyles();
	const processedTextArray = useMemo(() => EmojiUtils.splitTextWithEmojis(message), [message]);
	return (
	<Text style={style}>
	{processedTextArray.map(({text, isEmoji}, index) =>
	isEmoji ? (
	<View
	// eslint-disable-next-line react/no-array-index-key
	key={index}
	>
	<Text style={styles.emojisWithTextFontSizeAligned}>{text}</Text>
	</View>
	) : (
	convertToLTR(text)
	),
	)}
	</Text>
	);
	}

We can do that by:

1. Split the message by new line
2. For each line, we split the text with emojis
3. If the split from step 2 is empty, that means there is no emoji and we don't need to apply the line height
4. Else, we apply the bigger line height and apply the current rendering/mapping logic

const messageSplitByNewline = useMemo(() => message.split('\n'), [message]);
const processedTextArray = useMemo(() => messageSplitByNewline.map((m) => EmojiUtils.splitTextWithEmojis(m)), [messageSplitByNewline]);

return processedTextArray.map((lineTextArray, lineIndex) =>
    lineTextArray.length === 0 ? (
        <Text
            style={style}
            key={lineIndex}
        >
            {convertToLTR(messageSplitByNewline[lineIndex])}
            {lineIndex < processedTextArray.length - 1 ? `\n` : ''}
        </Text>
    ) : (
        <Text
            style={[style, {lineHeight: 25}]}
            key={lineIndex}
        >
            {lineTextArray.map(({text, isEmoji}, index) =>
                isEmoji ? (
                    <Text
                        key={index}
                        style={[styles.emojisWithText]}
                    >
                        {text}
                    </Text>
                ) : (
                    convertToLTR(text)
                ),
            )}
            {lineIndex < processedTextArray.length - 1 ? `\n` : ''}
        </Text>
    ),
);

iOS

Android

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

N/A, it's a visual UI issue

[melvin-bot]

@slafortune, @getusha Eep! 4 days overdue now. Issues have feelings too...

[muttmuure]

Not a quality issue

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@slafortune, @getusha Still overdue 6 days?! Let's take care of this!

[slafortune]

@getusha is there a submitted proposal that should be accepted here?

[getusha]

Asked the reason for the vertical margin to make sure https://github.com/Expensify/App/pull/47547/files#r1881603912

[getusha]

@bernhardoj Applying a larger line height would make it look off.

[getusha]

I think we can safely change it to marginBottom to fix this issue. @dominictb's proposal looks good to me.

🎀 👀 🎀 C+ Reviewed!

[melvin-bot]

Triggered auto assignment to @pecanoro, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

@pecanoro, @slafortune, @getusha, @dominictb Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

This issue has not been updated in over 15 days. @pecanoro, @slafortune, @getusha, @dominictb eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[pecanoro]

@dominictb @getusha Is the PR ready for review?

[slafortune]

@dominictb @getusha do you have an update?

[dominictb]

@slafortune @pecanoro We're waiting for @getusha for the final review

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.99-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Emoji overlaps the word above #54207

If no regressions arise, payment will be issued on 2025-02-25. 🎊

For reference, here are some details about the assignees on this issue:

• @getusha requires payment through NewDot Manual Requests
• @dominictb requires payment automatic offer (Contributor)

[melvin-bot]

@getusha / @dominictb @slafortune @getusha / @dominictb The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[slafortune]

@getusha can you please complete this checklist when you get a moment?

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment:

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion:

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

Test:

Do we agree 👍 or 👎

[slafortune]

@dominictb Role Contributor has been paid via Upworks $250

[getusha]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: Enlarge emojis in other contexts than just single character messages #47547 (comment)

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion: N/a

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

I don't think it's an important bug that affects important user flow. It's a visual issue on iOS.
Do we agree 👍 or 👎

[slafortune]

@getusha Role C+ requires payment through NewDot Manual Requests $250