[$250] iOS - Composer - "Write something..." is not in the middle of the composer

[lanitochka17]

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.14-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch New Expensify app
2. Open any chat

Expected Result:

"Write something..." will be in the middle of the composer

Actual Result:

"Write something..." is not in the middle of the composer

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6556576_1722281092738.ScreenRecording_07-30-2024_03-14-46_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013fd28700fc476d2f
• Upwork Job ID: 1818012292903295394
• Last Price Increase: 2024-07-29

Issue Owner

Current Issue Owner: @marcochavezf

[melvin-bot]

Triggered auto assignment to @marcochavezf (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[lanitochka17]

We think that this bug might be related to #vip-vsp

[roryabraham]

not a back-end deploy blocker, but I think since this negatively affects a very core component of the app it should be treated as a blocker

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[mkhutornyi]

#40692 might be the culprit

[roryabraham]

agree, was just coming to post the same thing. Would love to see this fixed rather than just reverting that PR which took a long time to land

[stephanieelliott]

Cool, so @roryabraham are you thinking we treat this as if it's a regular bug and get proposals on it?

[ShridharGoel]

Proposal

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

iOS - Composer - "Write something..." is not in the middle of the composer.

What is the root cause of that problem?

The value of lineHeightEmojisWithTextComposer it 22, which makes the text shift downwards in the composer.

App/src/styles/index.ts

Line 1714 in 2ff7775

lineHeight: variables.lineHeightEmojisWithTextComposer,

The composerStyle is suitable for emojis, but not when there's only text. We need to handle:

• Emoji
• Emoji with text
• Just text

App/src/components/Composer/index.native.tsx

Lines 76 to 79 in ba25cb3

	const composerStyle = useMemo(
	() => StyleSheet.flatten([style, doesTextContainOnlyEmojis ? styles.onlyEmojisTextLineHeight : styles.emojisWithTextLineHeight]),
	[style, doesTextContainOnlyEmojis, styles],
	);

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

Check if message contains text and emoji both:

function containsTextWithEmojis(message: string): boolean {
    const trimmedMessage = message.replace(/\s+/g, '');
    const emojisRegex = new RegExp(CONST.REGEX.EMOJIS, CONST.REGEX.EMOJIS.flags.concat('g'));
    
    const emojiMatches = trimmedMessage.match(emojisRegex);

    if (!emojiMatches) {
        return false;
    }

    const emojiCodes = [];
    emojiMatches.forEach((emoji) => {
        getEmojiUnicode(emoji).split(' ').forEach((code) => {
            if (!(CONST.INVISIBLE_CODEPOINTS as readonly string[]).includes(code)) {
                emojiCodes.push(code);
            }
        });
    });

    const messageChars = [...trimmedMessage];

    const containsText = messageChars.some((char) => {
        const unicode = getEmojiUnicode(char);
        return !emojiCodes.includes(unicode) && unicode.length > 0 && !(CONST.INVISIBLE_CODEPOINTS as readonly string[]).includes(unicode);
    });

    return containsText;
}

Add containsTextWithEmojis in Composer/index.native.ts:

    const containsTextWithEmojis = useMemo(() => EmojiUtils.containsTextWithEmojis(value ?? ''), [value]);

Update composerStyle in Composer/index.native.ts:

const composerStyle = useMemo(
        () => StyleSheet.flatten([style, doesTextContainOnlyEmojis ? styles.onlyEmojisTextLineHeight : (containsTextWithEmojis ? styles.emojisWithTextLineHeight : styles.lineHeightMarkdownEnabledInput)]),
        [style, doesTextContainOnlyEmojis, styles, containsTextWithEmojis],
    );

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[ShridharGoel]

@roryabraham @francoisl My proposal above should fix this as well.

[ShridharGoel]

Screen.Recording.2024-07-30.at.2.18.15.AM.mov

[marcochavezf]

Hi @mkhutornyi can you look at @ShridharGoel's proposal?

[mkhutornyi]

Testing. @ShridharGoel can you make sure that your change doesn't cause any other regression in different text display scenarios (with/without emojis)?

[hayes65]

I still dont understand how does the proposal above fixes this issue ... the issue doesn't happen when you type an emoji and two characters .. so what is the problem with typing an emoji and one character ?

[melvin-bot]

📣 @hayes65! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[ShridharGoel]

@mkhutornyi I couldn't find any issue yet with the solution. I've tested:

• Emoji
• Emoji with text
• Just text

[mkhutornyi]

@ShridharGoel Looks like you updated proposal without notifying "Proposal updated". Btw your solution isn't safe enough to CP.
I want @VickyStash chime in and find proper solution as said here.

[ShridharGoel]

@mkhutornyi I had updated it soon after posting, before the review started. And there was no other proposal, so I thought that the "Proposal Updated" comment can be skipped.

May I know what issues can be caused by the mentioned changes?

[roryabraham]

Cool, so @roryabraham are you thinking we treat this as if it's a regular bug and get proposals on it?

yeah but still treat is as an hourly for now. That would be my inclination

[mkhutornyi]

@mkhutornyi I had updated it soon after posting, before the review started

Change lineHeightEmojisWithTextComposer to 18

I had started reviewing & testing as soon as you posted ^ 😄

Meanwhile, there was slack comment that @VickyStash would be working on fix tomorrow morning.

[mkhutornyi]

@ShridharGoel's proposal looks promising but let's see if QA team can find another bug related to emoji / composer in addition to 2 known bugs so far, so the fix can cover all of them together (as deploy won't happen today).

[abzokhattab]

Do you know why this happens only on iOS? Although iOS and Android share the same styling logic of the composer @mkhutornyi @roryabraham ?

[mkhutornyi]

QA reported another bugs: #46451 #46453

I think we should revert offending PR since it's causing too many blockers.
cc: @roryabraham

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[stitesExpensify]

Personally I agree with a revert since there are now 3 deploy blockers / regressions linked

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[marcochavezf]

I created a revert here to unblock the deploy today

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[roryabraham]

We CP'd the revert, so this should be fixed now. Will retest on TF...

[roryabraham]

confirmed this is fixed ✅