Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[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 #8459

Closed
mvtglobally opened this issue Apr 4, 2022 · 80 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2

Comments

@mvtglobally
Copy link

mvtglobally commented Apr 4, 2022

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

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Apr 4, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 4, 2022

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

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Apr 4, 2022
@lschurr
Copy link
Contributor

lschurr commented Apr 4, 2022

I was able to reproduce this.

@lschurr lschurr added Engineering Improvement Item broken or needs improvement. labels Apr 4, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 4, 2022

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

@lschurr lschurr removed their assignment Apr 4, 2022
@melvin-bot melvin-bot bot added the Overdue label Apr 6, 2022
@chiragsalian
Copy link
Contributor

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

@melvin-bot melvin-bot bot removed the Overdue label Apr 6, 2022
@chiragsalian chiragsalian added the External Added to denote the issue can be worked on by a contributor label Apr 6, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 6, 2022

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

@chiragsalian chiragsalian added Weekly KSv2 and removed Daily KSv2 labels Apr 6, 2022
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Apr 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 7, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 7, 2022

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

@melvin-bot melvin-bot bot changed the title Highlighted emoji not visible after scrolling down with ArrowDown key after sometime - reported by @sobitneupane [$250] Highlighted emoji not visible after scrolling down with ArrowDown key after sometime - reported by @sobitneupane Apr 7, 2022
@trjExpensify
Copy link
Contributor

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

@melvin-bot melvin-bot bot added the Overdue label Apr 15, 2022
@trjExpensify trjExpensify changed the title [$250] Highlighted emoji not visible after scrolling down with ArrowDown key after sometime - reported by @sobitneupane [$500] Highlighted emoji not visible after scrolling down with ArrowDown key after sometime - reported by @sobitneupane Apr 19, 2022
@parasharrajat
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

parasharrajat commented Nov 15, 2022

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
Copy link
Member

@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
Copy link
Contributor

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
Copy link
Member

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

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Nov 17, 2022
@melvin-bot melvin-bot bot changed the title [$500] Highlighted emoji not visible after scrolling down with ArrowDown key after sometime - reported by @sobitneupane [HOLD for payment 2022-11-24] [$500] Highlighted emoji not visible after scrolling down with ArrowDown key after sometime - reported by @sobitneupane Nov 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 17, 2022

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:

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 23, 2022
@trjExpensify
Copy link
Contributor

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

@sobitneupane
Copy link
Contributor

Oops, missed it. I will create one asap.

@trjExpensify
Copy link
Contributor

Awesome, thanks!

@trjExpensify
Copy link
Contributor

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

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Dec 1, 2022
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2022-11-24] [$500] Highlighted emoji not visible after scrolling down with ArrowDown key after sometime - reported by @sobitneupane [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 Dec 1, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 1, 2022

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:

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
Copy link

melvin-bot bot commented Dec 1, 2022

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:

@trjExpensify
Copy link
Contributor

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

@parasharrajat
Copy link
Member

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
Copy link
Contributor

Can you send me a link to said contract?

@trjExpensify
Copy link
Contributor

Awesome, thanks. Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests