[HOLD for payment 2022-12-08] [HOLD for payment 2022-11-24] [$500] Highlighted emoji not visible after scrolling down with ArrowDown key after sometime - reported by @sobitneupane

[mvtglobally]

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 Picker.
2. Press down arrow multiple times to reach other categories of emoji (somewhere around "Animals & Nature).

Expected Result:

Highlighted emoji is visible in the screen.

Actual Result:

Highlighted emoji is not visible in the screen.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web

Version Number: 1.1.49-0
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: Any additional supporting documentation

Screen.Recording.2022-03-24.at.17.46.14.mov

Upwork URL: https://www.upwork.com/jobs/~0104371e92e24f551a
Issue reported by: @sobitneupane
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1648127603575109

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @lschurr (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[lschurr]

I was able to reproduce this.

[melvin-bot]

Triggered auto assignment to @chiragsalian (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[chiragsalian]

lol funny bug, should be pretty straightforward to fix externally.

[melvin-bot]

Triggered auto assignment to @trjExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[melvin-bot]

Current assignee @chiragsalian is eligible for the Exported assigner, not assigning anyone new.

[trjExpensify]

Issue on upwork here: https://www.upwork.com/jobs/~01ad54324b48c3dfa9

[parasharrajat]

The PR that introduced the bug has been identified. Link to the PR:

Not sure which one. But I think this issue is since the start of EmojiPicker.

[trjExpensify]

Niceeee! Super great to see this one merged! ❤️ Let's get started on collaborating to check these off:

@parasharrajat @sobitneupane @chiragsalian The PR that introduced the bug has been identified. Link to the PR:

From this comment, we reference this commit in this PR. That was a PR to add this new feature at the time, so is it accurate that this wasn't a regression, just missed in the initial implementation?

A discussion in #contributor-plus 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.

Any suggestions on steps to take to avoid it happening again? Did this code comment suggestion end up getting added to better explain what is happening, for example?

A regression test has been added or updated so that the same bug will not reach production again.

Happy to add this. I think this existing test case can be updated with the following additions from #4:

1. Navigate to a conversation
2. Open the emoji picker
3. Scroll down the list to the bottom verifying each emoji is rendered correctly
4. Close the emoji picker and reopen it again
5. Press the down arrow key multiple times to reach the "Animals & Nature" category
6. Verify the highlighted emoji is visible on the screen with every click.
7. Press the up arrow key multiple times to reach the top row of the emojis
8. Verify the highlighted emoji is visible on the screen with every click.

Thoughts?

[parasharrajat]

Any suggestions on steps to take to avoid it happening again? Did this code comment suggestion end up getting added to better explain what is happening, for example?

My bad, not. I forgot that the thread was live. Will handle this in follow-up.

• The steps look good.

I created a follow-up issue to fix the FlatList issue based on our discussion on the PR https://expensify.slack.com/archives/C049HHMV9SM/p1668550863631129

[parasharrajat]

@sobitneupane Could you please create a small follow-up PR for adding code comments? https://github.com/Expensify/App/pull/8902/files#r1012160276
No need to add screenshots etc. Just comments.

[trjExpensify]

My bad, not. I forgot that the thread was live. Will handle this in follow-up.

^^ was that comment in reference to the below? 😅

Any suggestions on steps to take to avoid it happening again? Did this code comment suggestion end up getting added to better explain what is happening, for example?

[parasharrajat]

Aha, 😄 my mind is running faster than my eyes and fingers.

[melvin-bot]

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

• Fix emoji offset issues #8902

If no regressions arise, payment will be issued on 2022-11-24. 🎊

[trjExpensify]

@sobitneupane did you create this PR? I can't find it.

[sobitneupane]

Oops, missed it. I will create one asap.

[trjExpensify]

Awesome, thanks!

[trjExpensify]

Alright, thanks for taking care of that. Closing! 🎉

[melvin-bot]

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

• [No QA] Add comment for targetOffset calculation #13007

If no regressions arise, payment will be issued on 2022-12-08. 🎊

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:

• [@parasharrajat / @sobitneupane / @chiragsalian] The PR that introduced the bug has been identified. Link to the PR:
• [@parasharrajat / @sobitneupane / @chiragsalian] 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:
• [@parasharrajat / @sobitneupane / @chiragsalian] 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:
• [@trjExpensify] 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:

[trjExpensify]

^^ Ignoring. Triggered by the quick follow-up PR to add a code comment.

[parasharrajat]

Sorry for pinging on a closed issue but payment for this issue was never released to me @trjExpensify. The contract is still active for it.

[trjExpensify]

Can you send me a link to said contract?

[parasharrajat]

Sure.
https://www.upwork.com/nx/wm/workroom/32090010/overview
https://www.upwork.com/jobs/for-8459_~01fe2dfcae8af70e1c

[trjExpensify]

Awesome, thanks. Done!