[$250] Emoji - Inconsistency between Android and mWeb emojis are not highlighted on android during search

[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: 1.4.77-0
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
Email or phone of affected tester (no customers): PR passed?
Issue reported by: Applause - Internal Team

Issue found when executing PR #42407

Action Performed:

1. Open the app and log in
2. Open any report
3. Tap the emoji picker
4. Enter a search word that would return only one result and not in recently used, e.g. panda, fox, pizza etc
5. Notice emoji not highlighted on android

Expected Result:

Emoji should be highlighted on Android

Actual Result:

Inconsistency between Android and mWeb emojis are not highlighted on android during search

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

Bug6495091_1716986811031.WhatsApp_Video_2024-05-29_at_17.38.39.mp4

Bug6495091_1716986811024.WhatsApp_Video_2024-05-29_at_17.38.23.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fb05f9d997d92b6c
• Upwork Job ID: 1795838319085740032
• Last Price Increase: 2024-05-29
• Automatic offers:
• ishpaul777 | Reviewer | 102601204
• bernhardoj | Contributor | 102601205

[melvin-bot]

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

[lanitochka17]

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

[bernhardoj]

Proposal

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

There is an inconsistency on how we highlight the first emoji when searching.

What is the root cause of that problem?

On web, we have a logic to highlight the first emoji when searching, however, this logic doesn't present for native.

App/src/components/EmojiPicker/EmojiPickerMenu/index.tsx

Line 262 in e177a69

const shouldFirstEmojiBeHighlighted = index === 0 && highlightFirstEmoji;

App/src/components/EmojiPicker/EmojiPickerMenu/index.tsx

Line 283 in e177a69

isHighlighted={shouldFirstEmojiBeHighlighted || shouldEmojiBeHighlighted}

The way it works is, if we are searching, we set highlightFirstEmoji to true,

App/src/components/EmojiPicker/EmojiPickerMenu/index.tsx

Line 127 in e177a69

setHighlightFirstEmoji(true);

we reset it whenever the user uses keyboard to move to other emojis,

App/src/components/EmojiPicker/EmojiPickerMenu/index.tsx

Lines 78 to 80 in e177a69

	if (highlightFirstEmoji) {
	setHighlightFirstEmoji(false);
	}

or when the user hover over an emoji,

App/src/components/EmojiPicker/EmojiPickerMenu/index.tsx

Lines 272 to 274 in e177a69

	onHoverIn={() => {
	setHighlightEmoji(false);
	setHighlightFirstEmoji(false);

both of these things don't exist in native.

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

We can just copy the logic from web file to native file. We set highlightFirstEmoji when searching and clear it otherwise. We don't need to clear it when hovering or using keyboard key because both things don't exist in native.

Or, we can just the highlight condition to

const shouldFirstEmojiBeHighlighted = index === 0;

Why? When we are not searching, we are showing a Frequently used header. The header takes the 0 to 7 index, so the first emoji index will be 8. If we are searching, the header is hidden, thus the first emoji index will be 0.

Let me know which one we want to do.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01fb05f9d997d92b6c

[melvin-bot]

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

[melvin-bot]

@lschurr, @ishpaul777 Huh... This is 4 days overdue. Who can take care of this?

[ishpaul777]

Will review today 🙇‍♂️

[ishpaul777]

@bernhardoj's Proposal LGTM!

we can just the highlight condition to

const shouldFirstEmojiBeHighlighted = index === 0;
Why? When we are not searching, we are showing a Frequently used header. The header takes the 0 to 7 index, so the first emoji index will be 8. If we are searching, the header is hidden, thus the first emoji index will be 0.

makes sense to me 👍

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

[bernhardoj]

@techievivek @ishpaul777 While working on the PR, I'm questioning whether we should do this or not. In web, we can use keyboard to navigate and select the emoji, so it makes sense to highlight the first emoji so the user can press Enter to select it, but on native, we don't support keyboard control, so even if we highlight it, pressing Enter will do nothing. Do we still want to do it?

[ishpaul777]

hey @bernhardoj I am not 100% sure whats should expected behaviour i lean on @techievivek for a decision here.

[melvin-bot]

@lschurr, @techievivek, @bernhardoj, @ishpaul777 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[lschurr]

@techievivek could you take a look here?

[melvin-bot]

@lschurr @techievivek @bernhardoj @ishpaul777 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!

[ishpaul777]

we are awaiting input from @techievivek

[techievivek]

Yeah, I don't know why it needs to be highlighted. It makes sense for web but not for mobile. Let's confirm with design team on this behaviour. Thanks.,

[melvin-bot]

Triggered auto assignment to @dannymcclain (Design), see these Stack Overflow questions for more details.

[techievivek]

Hi, @dannymcclain. Can you check the GH and confirm if we really need to fix the behavior here? i.e., we currently highlight search results on the web but not on mobile. Do we need to highlight the search results on phone as well?

[dannymcclain]

I think we highlight the emoji on desktop because you can use your keyboard (return/enter) to add it to the composer. That's not really as common of a pattern on mobile—I mean I guess technically we could implement it that way... but it's just not as common on mobile devices to use the return key to do things like that. So I think we can leave this behavior as is and close this.

[techievivek]

Great, thanks for confirming. We were on same boat so closing this out.