[$500] Web - Chat - Composer with emoji is expanded with an empty line after expanding the composer

[lanitochka17]

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: 1.4.58-4
Reproducible in staging?: Y
Reproducible in production?: N
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. Go to staging.new.expensify.com
2. Go to any chat
3. Enter an emoji with a multi-line message
4. Expand the composer

Expected Result:

No new line should be added at the end of the message

Actual Result:

A new empty line is added at the end of the message

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

Screen.Recording.2024-04-02.at.11.12.12.a.m.mov

Bug6434258_1711977454174.bandicam_2024-04-01_21-10-50-729.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016de42cbd35e27ee2
• Upwork Job ID: 1775310993834332160
• Last Price Increase: 2024-04-16

[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]

@marcochavezf FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

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

[shahinyan11]

Proposal

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

Web - Chat - Composer with emoji is expanded with an empty line after returning from Settings

What is the root cause of that problem?

Here we calculate number of lines . When we add an emoji to the text, the calculation does not work correctly and returns 1 line more. This is because the calculation is based on the following variables lineHeight, paddingTopAndBottom,textInput.current.scrollHeight, maxLines where lineHeight always belongs to the text. But currently the fontSize of the emojis is larger than the fontSize of the text. And when we add emojis, the paddingTopAndBottom and textInput.current.scrollHeight values decrease and calculation will not work correctly

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

Set the same font size for emojis and text. To do that we can add emoji: {fontSize: variables.fontSizeNormal} in markdownStyle object

What alternative solutions did you explore? (Optional)

Add below code on this line

if(extractEmojis(value ?? '').length > 0 && generalNumberOfLines > 1){
    generalNumberOfLines--
}

What alternative solutions did you explore? (Optional)

Add ability for getNumberOfLines function to receive one more param ( containsEmoji ) and update calculation logic there

[marcochavezf]

I found out this is introduced by #38955, reverting the offending PR here

[ishpaul777]

@marcochavezf I can raise a fix as offending PR author. it was pointed out here on weekend #38955 (comment) but slipped of mind

[ishpaul777]

Can we wait for fixing PR instead of full revert please

[marcochavezf]

Sure, we can wait

[marcaaron]

@ishpaul777 @marcochavezf can you give an ETA on when this will be ready?

[ishpaul777]

PR is ready for review #39378

[marcochavezf]

Original PR reverted, requested CP here

[marcaaron]

Asking for re-test

[marcochavezf]

Tested on staging and looks good

Screen.Recording.2024-04-01.at.9.38.15.p.m.mov

So we can close it out

[melvin-bot]

Current assignee @allroundexperts is eligible for the External assigner, not assigning anyone new.

[allroundexperts]

@allroundexperts could you review as C+ @shahinyan11's proposal when you have a chance?

I don't think its the correct RCA. Can someone please explain WHY adding return String(route.params?.reportID || 0); in function getReportID(route: ReportScreenNavigationProps['route']): string { fixes the issue?

[shahinyan11]

Can someone please explain WHY adding return String(route.params?.reportID || 0); in function getReportID(route: ReportScreenNavigationProps['route']): string { fixes the issue?

@allroundexperts It will not fix the current issue . The steps to reproduce was updated in issue and adding return String(route.params?.reportID || 0); would fix the old one

[melvin-bot]

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

[bfitzexpensify]

@allroundexperts thoughts on #39361 (comment)?

[allroundexperts]

@allroundexperts thoughts on #39361 (comment)?

It doesn't make a lot of sense to me. It would be awesome if we can get some more proposals.

[shahinyan11]

@allroundexperts Don't you agree that the line calculation is not working correctly due to the size of the emoji?

[melvin-bot]

@marcochavezf @allroundexperts @bfitzexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[marcochavezf]

Can someone please explain WHY adding return String(route.params?.reportID || 0); in function getReportID(route: ReportScreenNavigationProps['route']): string { fixes the issue?

Yeah I agree this a weird workaround, but yeah I think that was a solution for the previous problem

Don't you agree that the line calculation is not working correctly due to the size of the emoji?

One question that comes to my mind is, why do we have a different size for emojis and text?

[shahinyan11]

One question that comes to my mind is, why do we have a different size for emojis and text?

This is also a question for me. But it is a fact that line calculation does not work correctly when adding emoji.

@allroundexperts @marcochavezf The problem is in MarkdownTextInput component. It has numberOfLines prop.
It is logical that when the numberOfLines changes, the input should react to this and update count of lines according to the numberOfLines prop. But now it works little strange. For example, when numberOfLines is updated with the following sequence 1 -> 2, the input lines are not updated, But when numberOfLines is updated with the following sequence 1 -> undefined -> 2 the input lines are updated.
And here we have a case where currentNumberOfLines can be set to undefined if isComposerFullSize is true. And isComposerFullSize, in turn, at some point had the value true when using return String(route.params?.reportID || '') but never had the value true when using return String(route.params?.reportID || 0)

[melvin-bot]

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

[marcochavezf]

And here we have a case where currentNumberOfLines can be set to undefined if isComposerFullSize is true

Oh interesting, I don't know why it's undefined, seems the numberOfLines is a flawed design because I think the value can be relative to the size of the composer or the screen

[shahinyan11]

@marcochavezf What's interesting is that the input reacts to lines count updates and displays the lines corresponding to the numberOfLines value only when the previous NumberOfLines value was undefined and the new value is any number.

And I also want to tag @allroundexperts. I am sorry but In my option he is very passive as C+ contributor.

[allroundexperts]

@shahinyan11 Sorry, I'm not sure what you're expecting from me here. I think I should be reviewing the proposals rather than correcting the already made proposals. That'd be unfair to other contributors.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[shahinyan11]

@allroundexperts for your information the change I suggested was implemented here what led to the problem being solved.

[shahinyan11]

@marcochavezf @allroundexperts @bfitzexpensify It's just a shame how my efforts on this issue were appreciated

[allroundexperts]

@shahinyan11 I see that the solution you proposed in-fact got applied by Software mansion. I think you should get a partial compensation here. @marcochavezf Would you agree?

[marcochavezf]

@allroundexperts for your information the change I suggested was implemented here what led to the problem being solved.

Oh thanks for pointing it out @shahinyan11, and yeah I agree that in this case we should make a partial payment. @bfitzexpensify could you handle that?

[bfitzexpensify]

Agreed, I think a $250 payment is correct here. @shahinyan11 can you please apply to this Upwork job?

[shahinyan11]

@bfitzexpensify I have applied

[bfitzexpensify]

Payment complete, closing this out.