[HOLD for payment 2023-07-10] [$1000] Web - Chat - Empty space is added after adding an emoji by typing in web platform

[kbecciv]

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

---

Action Performed:

1. Login to expensify web
2. Go to any chat and type ":smiley":" and notice that the position marker is right next to the smiley emoji
3. Go to the expensify app and open any chat
4. Type ":smiley":" and notice that there is a spaces between the position marker and the smiley emoji

Expected Result:

There should always be a space added after an emoji.

Actual Result:

Inconsistent spacing after a smiley (or any emoji) on different platforms. A space is added in the Android mobile app, but no space is added next to the emoji in web, the desktop app, or in the iOS app.

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.30-5
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
Notes/Photos/Videos: Any additional supporting documentation

2023_06_26_06_42_17.1.mp4

Expensify/Expensify Issue URL:
Issue reported by: @SofoniasN
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687428313003479

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012ef8342a4645c7a1
• Upwork Job ID: 1673543813405294592
• Last Price Increase: 2023-06-27

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[sakluger]

I was able to reproduce this in the Android mobile app - that's the only platform that has the extra space in front of the emoji. All other platforms have no space after the emoji.

I'm not sure if it's possible, but ideally when we fix this, we can check to see if there are similar spacing issues in other places (like maybe we use the same component in different places).

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[getusha]

Proposal

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

What is the root cause of that problem?

Currently we do not have the check if the emoji is at last and insert it conditionally which replaceEmojis inside EmojiUtils had this implemented first.

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

We should make sure to add space after emoji selection/insertion if the emoji is being inserted at the end.

App/src/pages/home/report/ReportActionCompose.js

Lines 669 to 677 in 09a7432

	addEmojiToTextBox(emoji) {
	this.updateComment(ComposerUtils.insertText(this.comment, this.state.selection, emoji));
	this.setState((prevState) => ({
	selection: {
	start: prevState.selection.start + emoji.length,
	end: prevState.selection.start + emoji.length,
	},
	}));
	}

we will have to check if the emoji is being inserted last and update the comment accordingly as the following code:

        this.updateComment(ComposerUtils.insertText(this.comment, this.state.selection, isEmojiAtEnd ? `${emoji} ` : emoji));

Also we have to make sure we update the selection correctly with the condition above

        this.setState((prevState) => ({
            selection: {
                start: ... + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0),
                end:  ... + (isEmojiAtEnd ? CONST.SPACE_LENGTH : 0),
            },
        }));

we also have to remove isSmallScreenWidth including it's condition

App/src/libs/EmojiUtils.js

Line 227 in 09a7432

function replaceEmojis(text, isSmallScreenWidth = false, preferredSkinTone = CONST.EMOJI_DEFAULT_SKIN_TONE) {

What alternative solutions did you explore? (Optional)

N/A

[getusha]

here is similar issue: #20705
I was bumping to open it since it was not consistent.

[bernhardoj]

Here is a previous thread that discuss the expected behavior on adding emojis.

[melvin-bot]

📣 @liondancing! 📣
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. 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.
2. 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.
3. 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>

[narefyev91]

Proposal from @getusha looks good to me #21584 (comment)
🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[bernhardoj]

@getusha proposal removes the space when adding an emoji from the suggestion, but this issue mentions the space from typing the complete emoji code (in mobile), i.e. :smile:.

I'm not posting any proposal because based on the thread I posted here, the behavior is already working as expected, except we decide to change it

[getusha]

@getusha proposal removes the space when adding an emoji from the suggestion, but this issue mentions the space from typing the complete emoji code (in mobile), i.e. 😄.

@bernhardoj after the change, it will not add space on every method. am i missing something? thanks

[bernhardoj]

Typing the complete emoji code will be handled by EmojiUtils.replaceEmojis

App/src/libs/EmojiUtils.js

Lines 245 to 249 in e5f00f8

	// If this is the last emoji in the message and it's the end of the message so far,
	// add a space after it so the user can keep typing easily.
	if (isSmallScreenWidth && i === emojiData.length - 1 && text.endsWith(emojiData[i])) {
	emojiReplacement += ' ';
	}

That's why the space is there only on mobile screens.

[narefyev91]

@bernhardoj as i understand correctly we should fix both issues to have the same behaviour in choosing emoji and typing the full one.

[luacmartins]

I agree with @narefyev91.

[melvin-bot]

📣 @getusha You have been assigned to this job by @luacmartins!
Please apply to this job in Upwork 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 📖

[chiragxarora]

I've reported the same issue in past, I think we need to update the reporter in the issue here.
https://expensify.slack.com/archives/C049HHMV9SM/p1681555924437289?thread_ts=1681555924.437289&cid=C049HHMV9SM

[sakluger]

Hey @getusha @narefyev91 @luacmartins - I just noticed that we discussed this previously in Slack, and the space is added intentionally after an emoji. I just updated the GH issue description - could you please update your proposal and PR to make sure that we add a space in all platforms? Thanks!

[getusha]

@narefyev91 @luacmartins I have updated my proposal to make it consistent across platforms and the PR is ready

[melvin-bot]

🎯 ⚡️ Woah @narefyev91 / @getusha, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @getusha got assigned: 2023-06-27 15:54:51 Z
• when the PR got merged: 2023-06-28 16:12:43 UTC

On to the next one 🚀

[sakluger]

Nice work everyone! I sent out offers through upwork to @SofoniasN and @getusha. @narefyev91 works for Callstack so we pay outside of Upwork.

[chiragxarora]

I've reported the same issue in past, I think we need to update the reporter in the issue here.
https://expensify.slack.com/archives/C049HHMV9SM/p1681555924437289?thread_ts=1681555924.437289&cid=C049HHMV9SM

@sakluger

[SofoniasN]

Hey @chiragxarora your bug report mention space after emoji inconsistencies with in a platform but the issue i reported and @getusha fixed is the inconsistencies across platforms.

@sakluger couldnt tell if the upwork offer was pulled back due to this issue if it was i hope this clarifies it, Thanks in advance.

[SofoniasN]

Hey guys, any new updates on the issue @sakluger? Thanks in advance

[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 1.3.35-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix space inconsistency inserting emoji #21749

If no regressions arise, payment will be issued on 2023-07-10. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@narefyev91] The PR that introduced the bug has been identified. Link to the PR:
• [@narefyev91] 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:
• [@narefyev91] A discussion in #expensify-bugs 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:
• [@narefyev91] Determine if we should create a regression test for this bug.
• [@narefyev91] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[sakluger]

Hey @chiragxarora - This same question/issue has come up 4 or 5 times over the past 8 months, and it was reported by someone different every time. Originally, it was raised here, then here, and I think one or two other times.

I'm still going to award the reporting bonus to @SofoniasN because their report got us talking about this behavior again, so I think it's fair to reward them. It's definitely a tricky situation, but I think that's the most fair decision in this case.

[SofoniasN]

@sakluger Thank you for the thoughtful consideration!

[sakluger]

@SofoniasN sorry, I'm not sure what happened, I didn't take back the offer. I just tried resending, let me know if you didn't receive it.

[SofoniasN]

@sakluger No worries. I have accepted the offer, I am waiting on your approval!

Thanks

[sakluger]

Hey @narefyev91 could you please complete the BZ checklist? Thanks!

[sakluger]

I sent payments to @SofoniasN and @getusha (with efficiency bonus). Just waiting for the BZ checklist before closing out this issue. Given that this issue has come up a few times now, I think it would be helpful to document the correct behavior by adding a regression test.

[narefyev91]

• The PR that introduced the bug has been identified. Link to the PR - Fix space inconsistency inserting emoji #21749
• 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 - no important code changes were define
• A discussion in #expensify-bugs 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 - no discussion
• Determine if we should create a regression test for this bug - we are good with existing QA steps in PR