[HOLD for payment 2023-01-03] Requires two arrow presses to navigate emoji’s via keyboard

[kavimuru]

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. open emoji selector
2. search (first result is auto selected)
3. press right arrow key once (input box seems to unselect but selected emoji doesn’t change)
4. press right arrow key again (selected emoji moves right)

Expected result:

Selected emoji would move on first right arrow press.

Actual Result:

Requires 2 presses to navigate to next one

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.2.36-2
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Screen.Recording.2022-12-07.at.9.49.30.AM.mov

Recording.1061.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018a426d984f01623e
• Upwork Job ID: 1612894342292516864
• Last Price Increase: 2023-01-10

[melvin-bot]

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

[syedsaroshfarrukhdot]

Proposal

We can set highlightedIndex : -1 so that onSearch first emoji doesn't get highlighted automatically. This issue is as input is currently focused and we are highlighting the first emoji when search term is typed.

diff --git a/src/components/EmojiPicker/EmojiPickerMenu/index.js b/src/components/EmojiPicker/EmojiPickerMenu/index.js
index 2d97a2443..38e699a4b 100755
--- a/src/components/EmojiPicker/EmojiPickerMenu/index.js
+++ b/src/components/EmojiPicker/EmojiPickerMenu/index.js
@@ -386,7 +386,7 @@ class EmojiPickerMenu extends Component {
         ));
 
         // Remove sticky header indices. There are no headers while searching and we don't want to make emojis sticky
-        this.setState({filteredEmojis: newFilteredEmojiList, headerIndices: [], highlightedIndex: 0});
+        this.setState({filteredEmojis: newFilteredEmojiList, headerIndices: [], highlightedIndex: -1});
         this.setFirstNonHeaderIndex(newFilteredEmojiList);
     }

After Fix :-

Screen.Recording.2022-12-08.at.4.13.55.AM.mov

[stitesExpensify]

Hi @syedsaroshfarrukhdot thank you for your proposal, however this issue has not been marked as external because I intend to fix it myself. To avoid spending time in the future on issues that are not being opened for contributors to work on, I would recommend not researching issues that don't have the external and help wanted labels.

[syedsaroshfarrukhdot]

To avoid spending time in the future on issues that are not being opened for contributors to work on, I would recommend not researching issues that don't have the external and help wanted labels.

I understand that and I also used to wait before till external label used to apply before starting to work on solution. Recently, Many issues get proposal before external label is applied and get even hired for that proposal external label is then applied only to hire them on the job. So, I thought it is the way to move forward to be the first one to propose correct solution on the issues. 😅

[stitesExpensify]

I apologize for any confusion we may have caused you in the past regarding the "correct" process for getting chosen on an issue. We have been having some internal discussion to make the rules more clear, consistent, and fair for contributors, and will soon start locking issues until the external label is applied.

Thank you for your eagerness to help out, it is much appreciated!

[stitesExpensify]

More updates are in the original slack thread

[stitesExpensify]

Okay so here's where we landed:

• We don't want to set the index to -1 because then when you hit enter the emoji you typed in won't get added to the composer
• The problem isn't that it takes 2 presses, but that your first press feels like it didn't do anything.
• The first press gets rid of focus and now lets you move your highlight around, and the second press actually moves it.

To show this to the user, we will show the highlight currently in the code on no presses press, and then show a different highlight on the first press to show that now you can move the emoji around

[stitesExpensify]

Newest version

2022-12-16_15-52-22.mp4

[melvin-bot]

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

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.43-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Change emoji highlighting type when using keyboard navigation #13485

If no regressions arise, payment will be issued on 2023-01-03. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• 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:

• [@stitesExpensify] The PR that introduced the bug has been identified. Link to the PR: Add Emoji Picker #1991 (This is not a regression, it was in the initial implementation)
• [@stitesExpensify] 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: (not a regression)
• [@stitesExpensify] 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: (Not a regression)
• [@miljakljajic] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: https://expensify.testrail.io/index.php?/tests/view/3053160

[miljakljajic]

Sorry for the hold up whilst I was OOO. I will make this payment and review the regression test steps today.

[miljakljajic]

Ah - I thought this was reported by a contributor, but I was wrong. No payment required. There is an existing regression test for this.

[miljakljajic]

@stitesExpensify feel free to close when you've checked off your items. Thank you!

[aimane-chnaif]

@miljakljajic C+ payment is due for internal PR review

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~018a426d984f01623e

[melvin-bot]

Current assignee @aimane-chnaif is eligible for the Internal assigner, not assigning anyone new.

[mallenexpensify]

@aimane-chnaif can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~018a426d984f01623e

[aimane-chnaif]

@mallenexpensify accepted, thanks

[mallenexpensify]

@aimane-chnaif paid! thanks.
@miljakljajic I checked off the BZ box and added the link to testrail, closing!