Web - Chat - Top of the highlight is not even when highlight emojis

[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: 1.4.58.0
Reproducible in staging?: Yes
Reproducible in production?: No
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: Applause Internal Team
Slack conversation:

Action Performed:

1.Go to staging.new.expensify.com/

2.Go to any chat

3.Inset multiple emojis

4.Highlight all the emojis

Expected Result:

The top of the highlight will be even

Actual Result:

The top of the highlight is not even

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

20240329_233415.mp4

Add any screenshot/video evidence

View all open jobs on GitHub

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 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.

[m-natarajan]

We think this bug might be related to #vip-vsb

[m-natarajan]

@kevinksullivan 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.

[MahmudjonToraqulov]

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

Web - Chat - Top of the highlight is not even when highlight emojis

What is the root cause of that problem?

ExtrafontSize: 20px is added for emoji so we're getting this issue.

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

We should remove thisfontSize: 20px and it's solved.

What alternative solutions did you explore? (Optional)

N/A

Result

[thienlnam]

Not a blocker - added to the main tracking issue

[tomekzaw]

I think this might be actually the correct behavior as there is a space character between each pair of emojis.

Also, emojis have larger font size than the text.

Here's how it looks on Firefox with some text added in between:

cc @thienlnam

[melvin-bot]

@kevinksullivan, @thienlnam Still overdue 6 days?! Let's take care of this!

[kevinksullivan]

@thienlnam think tis fits into VSB as a very, very low bug?

[thienlnam]

Ah okay, so tested it and I think the problem here is with the emoji size in the composer. Emojis when sent without text are larger than if you send them with text

But the size of the emoji with text in the composer is larger than the text, so it ends up creating the odd highlighting. But when you highlight the sent text, it doesn't have a problem because the emojis are smaller

@kevinksullivan Yeah I'd place this as a very very low bug in vip-vsb

[tomekzaw]

I think the problem here is with the emoji size in the composer.

Here's the full story:

1. Currently (both on main and prod), the font size of any emoji in the composer is always set to 20pt. That's because 20pt is the default value of emoji.fontSize in Live Markdown library and I forgot to override it in E/App.

2. There's already a PR from @Skalakid that sets the correct font size from variables.emojiSize:

• Add default font size for emojis in live markdown input #39597

3. Once the above PR gets merged, the font size of any emoji in the composer will be always set to variables.emojiSize. Is this the expected behavior? Correct me if I'm wrong but based on this discussion, I assume that the emoji.fontSize should depend on the current message – if it contains only emojis, the font size should be larger.

cc @thienlnam

[thienlnam]

Ah yup that's correct - okay sweet in that case this should be "fixed" when that PR is merged so I'll go ahead and close

[tomekzaw]

@thienlnam What about point 3? Do we want to increase/decrease the font size of emojis in the composer depending on whether the current message contains any text or only emojis?

[thienlnam]

Oh my bad, but emoji.fontSize should depend on the current message

For example the first size is larger since there are only emojis, and then the second message with text is smaller

[tomekzaw]

No problem, I think we can easily achieve that. We can set emoji.fontSize in markdownStyle based on the current message directly in React code.

Currently, we have a useMarkdownStyle hook that returns the markdownStyle object – we can pass the current text as an argument and calculate emoji.fontSize based on whether it contains only emojis or not.

The only problem is that the font size will be calculated on the JS thread but the formatting needs to be applied on the UI thread while typing – there's a possibility of a "font size jump" when the result of the predicate changes. I believe this is not a huge problem for now (please let me know what you think @thienlnam).

There are two options how we can improve that in the future:

1. We can extend Live Markdown so that we can pass two different fontSize values and MarkdownTextInput will use the appropriate value based on its contents. (I don't like this option as it introduces some confusion)
2. We can extend the JS parser so that it does not apply emoji style to the emojis if the current text includes characters (glyphs?) other than emojis. This way, we can set emoji.fontSize once and this setting will be used only for emoji-only messages.

[thienlnam]

Yeah that sounds good, the font size jump doesn't sound like a problem right now - so let's just try that first and improve it later if there are any concerns

[tomekzaw]

Do you want to merge #39597 first and then come up with another PR or should we implement this directly in #39597?

[thienlnam]

If we could implement it directly that would be great

[tomekzaw]

Sure thing, I'll leave it up for @Skalakid since he's the author of the PR.

[melvin-bot]

@kevinksullivan @thienlnam this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[tomekzaw]

fyi #39597 has been merged.

[melvin-bot]

This issue has not been updated in over 15 days. @kevinksullivan, @thienlnam 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!

[kevinksullivan]

Think this is all set @thienlnam ?