-
Notifications
You must be signed in to change notification settings - Fork 3k
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 2023-06-29] [$1000] Arrow key shortcut stops working #19700
Comments
Triggered auto assignment to @johncschuster ( |
Bug0 Triage Checklist (Main S/O)
|
The cause is pretty narrowed down here:
I think this would have been easier to fix if we were dealing with functional components. There out of the box memoization with Class components. The recommended way is to use the library |
FYI - No Slack thread for this one. |
Triggered auto assignment to @flodnv ( |
From @aldo-expensify's investigation it seems like @roryabraham you caused this in #18883, can you have a look please and let us know what you think? |
Also, Vit was recommending me to get help from one of our external expert agencies to work on this (here). I think that could be a good idea. |
Looking into this now |
I'm not sure I yet see how this is related to #18883, because the |
Found this lib which honestly seems pretty slick |
I'm pretty sure you could fix this particular bug by playing with the priorities during subscribe, but I still think this should be fix in a way it stops unsubscribing/subscribing every cycle. |
Interesting, the focus trap thing could help solving the problem we have with key event listeners that keep listening even out of view (see https://expensify.slack.com/archives/C01GTK53T8Q/p1685138015892469 / #18419) |
This seems like the exact scenario the upcoming |
In only found this: reactjs/rfcs#220 Is this going to be implemented? Looks good, but I think for the moment, if that is not near to be released, this can be done with |
Yeah, it's just an RFC now, and we should be able to simulate it. |
Put up a draft PR with a potential fix: #20016. More testing and discussion is needed since it uses an experimental React feature which is explained in good detail here: https://react.dev/learn/separating-events-from-effects#declaring-an-event-function |
📣 @gedu You have been assigned to this job by @roryabraham! |
I'll just wait for the PR then, thanks for the early review @roryabraham! |
@roryabraham @aldo-expensify Hey, while I was adding the I was improving the callback for
Doing I want to point this out because as we start using hooks, these issues could arise in many places. What do you think? |
The |
Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:
On to the next one 🚀 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.30-5 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 2023-06-29. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
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:
|
I think this is ready for payment now |
|
Bug: Arrow key selection stops working on the workspace invite members page Regression Steps
👍 or 👎 |
@Santhosh-Sellavel Upwork invite sent. Can you please ping me when you've accepted? |
@Santhosh-Sellavel has requested payment via NewDot 🎉 |
Paid |
Thanks, @anmurali! |
Then, since @gedu works for Callstack, we are all set to close this 🎉 |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Coming from: https://github.com/Expensify/App/pull/18883/files#r1206010822
Action Performed:
Expected Result:
Pressing UP or DOWN should change the highlighted row
Actual Result:
Pressing UP or DOWN doesn't do anything
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number:
Reproducible in staging?:
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: