[$1000] FEATURE REQUEST: Allow navigating the list in the new group dialog with arrow keys

[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 app
2. Navigate to "+" > "New Group"
3. Type anyones name and add
4. Use arrow keys to navigate

Expected Result:

User should be able to user arrow keys

Actual Result:

User is unable to use arrow keys

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.1.37-0 Reproducible in staging?: Y Reproducible in production?: Y Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

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

View all open jobs on GitHub

[MelvinBot]

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

[MelvinBot]

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

[roryabraham]

I have this working on a local branch but am currently scope-creeping and making arrow navigation work on all our OptionsLists, not just NewGroup. The one that's proving trickier is IOUConfirmationList.

[roryabraham]

Just so you know I'm not just making stuff up, here's a draft PR with what I've got so far: https://github.com/Expensify/App/pull/7702 🙃

[roryabraham]

Got my PR done and almost ready for review. It's a bit of a beast so I want to make sure I write up super thorough test / QA steps.

[MelvinBot]

@roryabraham Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MelvinBot]

@roryabraham Still overdue 6 days?! Let's take care of this!

[roryabraham]

Okay, ended up being a very big PR but I think there's a lot of good changes there and it's very thoroughly tested. Excited to see how the review goes.

[mallenexpensify]

Assigned to @rushatgabhane , he's reviewing and testing the PR, job price for doing so will be $500. Rushat, please apply here https://www.upwork.com/jobs/~016a9b6f5ca7f87207

[mallenexpensify]

PR is being reviewed, it's a bit large :)

[roryabraham]

Yeah, I'll try to push it forward ASAP. I might split it into multiple PRs to make it easier for the reviewers too. @rushatgabhane would it be helpful it I split off the KeyboardShortcut library changes from the rest?

[rushatgabhane]

@roryabraham participants being hidden is the main blocker for the PR. If we can fix it, then there isn't much to gain by splitting the PR.

Splitting off KeyboardShortcut lib would be nice for the two tickets waiting on event bubbling to be implemented tho. (https://github.com/Expensify/App/pull/7920#discussion_r816054768, https://github.com/Expensify/App/issues/7623)

[roryabraham]

I'm sure we'll be able to fix the participants being hidden bug, but frankly this just isn't as high priority as some other issues I've got assigned to me at the moment. So I'm going to create a separate PR for the KeyboardShortcut lib upgrades then follow-up with this as soon as I can.

[mallenexpensify]

@roryabraham should @rushatgabhane be compensated additionally for the work reviewing the new PR https://github.com/Expensify/App/pull/8059

[rushatgabhane]

For transparency, during review of original PR, I missed that bug in the first place. So.. 🤷‍♂️

[mallenexpensify]

PR was put on hold, checking to see if it can be taken off https://github.com/Expensify/App/pull/7702#issuecomment-1076820876

[roryabraham]

It can be taken off hold, I just haven't been able to re-prioritize it yet. I could clean it up and get it ready for review then we could treat the known bug (where the first item is not always visible) as a separate bug and make it external. Thoughts @rushatgabhane?

[rushatgabhane]

@roryabraham sure, let's do that

[mallenexpensify]

I'm not sure where this issue is at. Rory or Rushat, can you provide an update?

[rushatgabhane]

@mallenexpensify my best guess would be that Rory has other higher priority issues, so the PR is on a pseudo hold.

[mallenexpensify]

hahahah I love pseudo hold, def hoping to reuse that term

[mallenexpensify]

Guessing this is still on pseudo hold

[roryabraham]

Nope, the PR has been updated recently and is in active review!

[mallenexpensify]

Based on this comment in the PR, it looks like Rory needs to review Marc's review https://github.com/Expensify/App/pull/7702#pullrequestreview-942599372

Also... reminder to self, this is the issue Rushat was hired to review the PR, I believe the compensation is $500, will double check when issuing payment

[roryabraham]

Yeah, quick update – @marcaaron's review was pretty substantial and so I'm working on some of the refactors he asked for in a separate branch. I'm making progress, but not pushing to the branch until I have something I'm confident in.

[rushatgabhane]

@roryabraham I'm guessing the PR will require further C+ review. Feel free to unassign me if that's the case. I'm OOO until May 14.

[mallenexpensify]

I think @roryabraham is working on this

[mallenexpensify]

Looks like @parasharrajat is helping review the PR, which is moving along. Reminder to discuss potential compensation for Rushat and Rajat once this is closed

[mallenexpensify]

Any update @roryabraham or @parasharrajat ? Looks like it's slowly moving along

[mallenexpensify]

Not much action in the PR the past couple weeks https://github.com/Expensify/App/pull/7702

[roryabraham]

Got the PR all cleaned up and ready for review

[roryabraham]

@rushatgabhane has been a 🦸 reviewer on this massive PR. Let's bump his compensation for this issue up from $500 to $1000 (assuming no regressions).

[mallenexpensify]

Whoohoo!! Thanks for the help @rushatgabhane ! Y'all might have to let me know when to issue payment, if the title doesn't get auto-updated with the date

[mallenexpensify]

Looks like it's still being worked on and reviewed, Rory's out this week too

[mallenexpensify]

@roryabraham or @rushatgabhane , can you provide a tl,dr: on:

1. Why this is such a big undertaking
2. Will this help on other parts of the site for navigating via arrow keys? (I'm not asking cuz it's taking a bit but curious who it's a lot of work vs just mimicking the use of tab and shift+tab for keyboard nav)

Also.. for other keyboard nav improvements for group chats, dropped this in #expensify-open-source https://expensify.slack.com/archives/C01GTK53T8Q/p1655918174158159

[roryabraham]

Will this help on other parts of the site for navigating via arrow keys?

Yes, the PR I have been working on implements arrow key navigation in many places on the site.

Why this is such a big undertaking

Because I chose to solve this class of problem almost everywhere in the app, it involves a large refactor of some of our most complex components.

[mallenexpensify]

Because I chose to solve this class of problem almost everywhere in the app, it involves a large refactor of some of our most complex components.

whoooohooo.. thanks @roryabraham (and @rushatgabhane ) keyboard nav and shortcuts FTW!!!

[roryabraham]

Reopening to track C+ payment. I think @rushatgabhane should get $500 for the review of this issue, or $1000 if it makes it to production for a week w/o causing regressions. And @parasharrajat gets $250 according to this comment because he stepped in for a bit while @rushatgabhane was OOO

[rushatgabhane]

Thank you for reopening!

[mallenexpensify]

@roryabraham or @rushatgabhane what's the payment date? I want to update the title so I don't forget. Wanna say July 5? I'm off 2-4 for weekend and holiday

[rushatgabhane]

@mallenexpensify We'll know the date after the PR is on production.

Also, we had one regression so the payment would be half for everyone

• https://github.com/Expensify/App/issues/9606

[mallenexpensify]

Thanks @rushatgabhane , I was looking at https://github.com/Expensify/App/pull/7702, how many related PRs, like #9606 are there?

[rushatgabhane]

@mallenexpensify ~so far there has been one regression~

Two regressions/bugs found so far..

1. 9606

2. https://github.com/Expensify/App/issues/9678

Rest of them aren't regressions

1. https://github.com/Expensify/App/issues/9598
2. https://github.com/Expensify/App/issues/9614
3. https://github.com/Expensify/App/issues/9604
4. https://github.com/Expensify/App/issues/9595

[melvin-bot[bot]]

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

[Christinadobrzyn]

Looks like old job posting in Upwork was closed - so created a new posting here.

Internal - https://www.upwork.com/jobs/~017c647a0e75584044 External - https://www.upwork.com/ab/applicants/1544742427005378560/job-details

Based on this comment, I hired @parasharrajat for $250 and @rushatgabhane for $500. Let me know if something needs to be changed!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[rushatgabhane]

@Christinadobrzyn i guess we can settle this issue

PR merged to production ~20 days ago https://github.com/Expensify/App/pull/7702#issuecomment-1178546074

[Christinadobrzyn]

Paid @rushatgabhane $500 for the job.

I can see that we've only hired @rushatgabhane so I assume this is the only payment? If that's wrong, please let me know.

Closed job in Upwork and closing this as complete.

[parasharrajat]

I think I am left out https://github.com/Expensify/App/issues/7648#issuecomment-1176518162.