[$500] Chat - When pressing the left/right arrow keys after typing '@', the text selection does not move

[lanitochka17]

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

---

Issue found when validating #38383
Version Number: 1.4.57-2
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
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1, Open any chat
2, Type a mention to open up the mention suggestion
3, Press the left/right arrow key
4, Observe that the text selection does not move to the left/right

Expected Result:

when right left arrows are pressed, suggestions highlights should reflect the change corresponding to cursor movement in the compose box as it does with emojis

Actual Result:

when right left arrows are pressed, suggestions highlights are not changing corresponding to cursor movement in the compose box. But it does with emojis.

NOTE: Emoji behavior also shown in the video

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

Screen.Recording.2024-03-27.at.11.47.04.PM.mp4

Recording.1496.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013f02f19e8b3faf16
• Upwork Job ID: 1775203737590730752
• Last Price Increase: 2024-04-02
• Automatic offers:
• eh2077 | Reviewer | 0
• bernhardoj | Contributor | 0

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

[melvin-bot]

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

[lanitochka17]

Production:

Recording.1497.mp4

[lanitochka17]

@stitesExpensify 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

[shubham1206agra]

@lanitochka17 You might be confused here. The staging behaviour is correct indeed.

[eh2077]

@lanitochka17 You might be confused here. The staging behaviour is correct indeed.

Hi @stitesExpensify , I saw your #38383 (comment) here but I think I agree with @shubham1206agra that this should be the expected behaviour. I tried to search old issues to find a regression test to support my point but couldn't find such one.

@lanitochka17 Are you able to search our existing regression tests to see if we have any tests related to this case?

[bernhardoj]

I think QA expects the mention highlight to move along with the text selection (as stated by @stitesExpensify in #38383 (comment)) just like emoji, but because on prod we can't move the selection at all, QA put a deploy blocker.

The highlight of the mention/emoji is based on the prefix props,

App/src/pages/home/report/ReportActionCompose/SuggestionMention.tsx

Line 323 in 5a8cb7f

prefix={suggestionValues.mentionPrefix}

the prefix props for mention coming from suggestionValues.mentionPrefix which contains the text after @, however, to correctly update the highlight of the suggestion, the prefix should consider the selection position, just like we did with emoji suggestion.

App/src/pages/home/report/ReportActionCompose/SuggestionEmoji.tsx

Line 230 in 5a8cb7f

prefix={value.slice(suggestionValues.colonIndex + 1, selection.start)}

For mention suggestion, the prefix tries to contain the whole string. (notice the leftString)

App/src/pages/home/report/ReportActionCompose/SuggestionMention.tsx

Lines 225 to 234 in 5a8cb7f

	let suggestionEndIndex;
	if (indexOfFirstSpecialCharOrEmojiAfterTheCursor === -1) {
	// We didn't find a special char/whitespace/emoji after the cursor, so we will use the entire string
	suggestionEndIndex = value.length;
	} else {
	suggestionEndIndex = indexOfFirstSpecialCharOrEmojiAfterTheCursor + selectionEnd;
	}
	const afterLastBreakLineIndex = value.lastIndexOf('\n', selectionEnd - 1) + 1;
	const leftString = value.substring(afterLastBreakLineIndex, suggestionEndIndex);

We can update it to always use selectionEnd (or selectionStart, there is currently inconsistencies on the selection usage, even on emoji suggestion)

const leftString = value.substring(afterLastBreakLineIndex, selectionEnd);

But this will cause this issue again where the suggestion won't show if we type @@ and put the selection between the @, @|@.

And that's because we check whether the cursor is before a mention code (@)

App/src/pages/home/report/ReportActionCompose/SuggestionMention.tsx

Lines 264 to 266 in 5a8cb7f

	const isCursorBeforeTheMention = valueAfterTheCursor.startsWith(suggestionWord);
	if (!isCursorBeforeTheMention && isMentionCode(suggestionWord)) {

We should remove it as we already update the leftString to slice it only until selectionEnd.

If we want to fix it, I can create a proposal.

[eh2077]

@bernhardoj I got your point. Hmm, I just feel that it also makes sense to capture valid prefix as longer as possible. But, yes, it's definitely better to make the behaviour consistent between emoji and mention.

[lanitochka17]

@eh2077 I'll look again, but so far I don't see any TC related to this issue.

[eh2077]

@stitesExpensify I'm happy to be the C+ of this issue if it's going to be external.

[melvin-bot]

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

[melvin-bot]

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

[bernhardoj]

I can't do the test yet because, on the latest main, the suggestion doesn't work.

Screen.Recording.2024-04-05.at.11.32.38.mov

[eh2077]

@bernhardoj I believe the latest main is ready for you to start working on the PR.

[bernhardoj]

Yeah, it works now, thanks for the bump!

[bernhardoj]

PR is ready

cc: @eh2077

[melvin-bot]

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

[eh2077]

@stitesExpensify Can you help to add BZ here to handle the payment? I think it's due for payment. Maybe add a Bug label will help to trigger the process.

[melvin-bot]

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

[stitesExpensify]

Hi @kadiealexander ! The work here is done, we just need to pay :)

[melvin-bot]

@stitesExpensify, @bernhardoj, @kadiealexander, @eh2077 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[eh2077]

@kadiealexander This issue is due for payment. Can you help with it?

[eh2077]

Checklist

• [@eh2077] The PR that introduced the bug has been identified. Link to the PR: fix: Chat - App closes mention suggestions when cursor is between double @ #29239
• [@eh2077] 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: https://github.com/Expensify/App/pull/29239/files#r1598813236
• [@eh2077] 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: N/A
• [@eh2077] Determine if we should create a regression test for this bug. No. Because this is a minor case and it's caught by QA

[melvin-bot]

@stitesExpensify, @bernhardoj, @kadiealexander, @eh2077 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@stitesExpensify, @bernhardoj, @kadiealexander, @eh2077 Eep! 4 days overdue now. Issues have feelings too...

[eh2077]

Not overdue, this is waiting for payment.

cc @kadiealexander

[kadiealexander]

Payouts due:

• Contributor: $500 @bernhardoj
• Contributor+: $500 @eh2077

Upwork job is here.