[HOLD for payment 2023-01-05] [$4000] Emoji Picker list jumps while scrolling through arrow keys reported by @parasharrajat

[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. Login to the app on web.
2. Navigate to any report.
3. Open the Emoji Picker.
4. User down arrow key to scroll the list until you reach the section Travel & Places.
5. Now keep on scrolling down via arrow key .
6. Notice that list jumps and highlighted emoji is out of focus.

Expected result:

Highlighted emoji should remain in focus and list does not jump positions.

Actual:

Emoji List jump positions and highlighted emoji goes out of focus.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.2.28-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:

Recording.941.mp4

screen-2022-11-16_03.48.45.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011a004a1208ec7fec
• Upwork Job ID: 1598460283393093632
• Last Price Increase: 2022-12-15

[melvin-bot]

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

[JmillsExpensify]

Taking this one given that I'm on the emoji tracking issue. Not treating this as an accessibility issue.

[JmillsExpensify]

I'm wavering a bit based on my last comment. This bug seems to only affect that one section right? If this is a clear and quick fix, then I'm more amenable to addressing it now, even though we're not focused on keyboard/accessibility at the moment. Getting another opinion from engineering.

[melvin-bot]

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

[JmillsExpensify]

@Gonals Please see above for a second opinion. Thanks!

[parasharrajat]

This issue does not come under A11y/navigation issues. This is a bug with emoji list viewing. This does not affect only one section. It might affect more sections we add them. It is just a coincidence that the section lies where the bug occurs. The main issue is with the Flat list scrolling/rendering. Due to virtual scrolling, something bad is happening with our scrolling logic.

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[JmillsExpensify]

Ok well I think that's also my point here. Is this the only place this issue is happening with FlatList. I'm less and less interested is solving issues like this one as if this is the only place this is happening, and I think whatever solution we come up with, let's make sure that we aren't working on this same thing somewhere else in the repo in a silo.

[Gonals]

@Gonals Please see above for a second opinion. Thanks!

It took me quite a bit to actually see the issue 😅.

If the behavior is only happening in this specific section of the emoji list, I honestly don't think it is a bit deal. If we have a quick fix, great, otherwise... 🤷.

If we start seeing scrolling issues with our flatlists elsewhere, then we'd certainly need to tackle it with more urgency

[parasharrajat]

Why do we think that this is not a big deal? IMO, It is affecting user experience, and thus should be fixed.

[JmillsExpensify]

To be clear, I think we should eventually fix it, but I think the wrinkle is that if it's in FlatList, then we're likely moving away from that soon in favor of WishList. So I think the best thing to do is put this issue on hold until that shakes out.

[JmillsExpensify]

Still feel the same as last week, though open to differing opinions.

[parasharrajat]

Make sense #12772 (comment) 👍

[JmillsExpensify]

Given that we've decided that WishList is quite far away, I'm open to re-evaluating this one. @stitesExpensify curious to get your take on this issue since you're also working on the larger emoji initiative. Maybe we even take this internal via either you or @Gonals to see if a quick fix exists?

[stitesExpensify]

I am not able to reproduce this issue:

2022-12-01_11-46-59.mp4

[parasharrajat]

I think you have to scroll a little further in the travel & places section

[stitesExpensify]

🤦 yep you're right

[JmillsExpensify]

Yeah it's real tough to reproduce. It's a bug, though I do think it's pretty low value. Removing the FlatList hold, though I concern I have is whether the solution is straightforward around the scrolling logic. @stitesExpensify @Gonals shall we open up to External in any case?

[Pujan92]

Thanks, applied for the job and PR is ready for review.

[JmillsExpensify]

It looks like only @Santhosh-Sellavel is assigned for review. @Luke9389 should you also be on the PR, or do we need to pull bear so that others are assigned? Let's help @Pujan92 get this merged in three business days!

[JmillsExpensify]

Actually...something weird is going on? Looks like @Gonals was chosen but I'm pretty sure he's OOO now and also not assigned.

[Pujan92]

Actually...something weird is going on? Looks like @Gonals was chosen but I'm pretty sure he's OOO now and also not assigned.

I think it is my mistake and happens due to the incorrect way of linking the issue in the PR. Updated the PR.

[JmillsExpensify]

Looks like the linked PR is ready to merge. Left a comment for @Gonals to confirm in the PR.

[melvin-bot]

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

• Prevent emoji Picker list jumping while scrolling through arrow keys #13787

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

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:

• [@Santhosh-Sellavel / @Luke9389] The PR that introduced the bug has been identified. Link to the PR:
• [@Santhosh-Sellavel / @Luke9389] 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:
• [@Santhosh-Sellavel / @Luke9389] 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:
• [@JmillsExpensify] 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:

[JmillsExpensify]

Today is payday on this issue, so I'm circling back to close the loop on a payment summary. Looking at the PR, I'm going to go ahead and award C/C+ the bonus for urgency. Any delay in merging the PR was caused on our side due to the holidays. That means that payouts will look like this:

• Issue reporter: @parasharrajat - $250
• Contributor: @Pujan92 - $6,000 (incl 50% bonus)
• Contributor: @Ollyws - $1,000 (proposal assistance)
• Contributor+: @Santhosh-Sellavel - $6,000 (incl 50% bonus).

Upwork job is here. An offer has been extended to @Pujan92, @Santhosh-Sellavel has been invited, as has @parasharrajat. Thanks!

[Ollyws]

@JmillsExpensify There was also $1000 compensation for me as per: #12772 (comment)
Thanks!

[JmillsExpensify]

Ah thank you! Can you please apply to the Upwork job? I'll issue an offer once you apply.

[Ollyws]

@JmillsExpensify Applied, thanks!

[JmillsExpensify]

@Pujan92, @Ollyws and @parasharrajat have been paid. @Santhosh-Sellavel was sent an offer (I'll pay out the bonus when payment is issued).

[JmillsExpensify]

Then @Santhosh-Sellavel @Luke9389 can either of you kick-off and complete your items on the BZ checklist? I've already created an issue for regression testing in the Expensify/Expensify repo.

[Luke9389]

I'm not sure if this is a regression. I think this behavior has always been here and we just recently noticed it. If that's the case, should we check the "The PR that introduced the bug has been identified." box?

I'll dig in to see if I can find an offending PR but I'd still like to know what to do in the case that there isn't one.

[JmillsExpensify]

If there isn't a regression, then all good. We should be able to cover this moving forward with the regression test we'll add moving forward.

[Luke9389]

Ok, I didn't see anything that indicates this is a regression, so I checked us off.

[JmillsExpensify]

@Santhosh-Sellavel has been paid out, and all items are successfully checked off in the BZ checklist. Closing this issue as a result.