[HOLD for payment 2023-02-03] [$250] Audit forms and fix inconsistencies with focus, tab and shift + tab behavior 2/4

[dylanexpensify]

We should audit all our forms and fix any inconsistencies with focus, tab, shift + tab and enter behavior. The expected behavior is as follows:

1. Tab navigates to the next input.
2. Shift + tab navigates to the previous input.
3. Enter submits the form.
4. Space toggles checkboxes/dropdowns.

Note: We should make sure that tabbing cycles through the form in an order that makes sense, usually top to bottom.

Here's a list of forms to be audited:

• RequestorStep.js (INTERNAL)
• ValidationStep.js (INTERNAL)
• Workspace - Invite new members
• Workspace - General settings
• Set password

Upwork job link: https://www.upwork.com/jobs/~0197285630ce0fa6fa https://www.upwork.com/jobs/~01a2053d2c6b3432b4

[MelvinBot]

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

[MelvinBot]

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

[dylanexpensify]

Note: decision to split this main issue up into smaller issues came from this convo

[MelvinBot]

@michaelhaxhiu, @nickmurray47 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[michaelhaxhiu]

Note: decision to split this main issue up into smaller issues came from this convo

Does this job need to be posted to upwork in a particular fashion basically? Would it be 1 job for everything or 5 jobs (linked to 1 GH issue)?

[nickmurray47]

@michaelhaxhiu I personally think it should be 1 job since I assume the fix will be the same for all forms. I can see the argument for splitting it up into multiple jobs but I think whichever contributor suggests the best fix should just do all of the forms in one go.

[michaelhaxhiu]

cool thanks nick. @dylanexpensify said the same in a DM.

[michaelhaxhiu]

Posted to upwork and added to GH body. Link here too: https://www.upwork.com/jobs/~01a2053d2c6b3432b4

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to @thienlnam (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[mdneyazahmad]

Proposal

These 4 related issues are very similar. I proposed the solution here #7918 (comment)

[parasharrajat]

Reviewing the proposal on another issue.

[thienlnam]

Any contributor interested in doing this audit? Involves going through the forms and making sure keyboard actions make sense and making updates to those that don't

[puneetlath]

Thanks @mdneyazahmad. @parasharrajat looks like this is now waiting on you, is that right?

[parasharrajat]

PR is failing for one scenario which is important IMO. Started a discussion on the PR for a new approach. let see how far we can go with that.

[melvin-bot]

@puneetlath, @parasharrajat, @thienlnam, @mdneyazahmad Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@puneetlath, @parasharrajat, @thienlnam, @mdneyazahmad 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[puneetlath]

@mdneyazahmad @parasharrajat can you guys provide a summary of where we're at with this? And also a link to the PR?

[JmillsExpensify]

Also I'm left wondering whether we should instead close this issue? It's focused on tabbing behavior and other keyboard navigation that is not a focus of our bug fixing machine.

[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]

At the very least, it's not a focus for WAQ so putting a hold on this issue until we agree on next steps.

[melvin-bot]

@puneetlath, @parasharrajat, @thienlnam, @mdneyazahmad Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[mdneyazahmad]

PR #12370 is slowly moving on. It was stuck on loading page.

[melvin-bot]

@puneetlath, @parasharrajat, @thienlnam, @mdneyazahmad Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[puneetlath]

Do we think we can get it to the finish line this week?

[melvin-bot]

@puneetlath, @parasharrajat, @thienlnam, @mdneyazahmad Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[puneetlath]

@parasharrajat @luacmartins @thienlnam how close do you think we are on this one? Can we get it closed off in the next couple of days?

[parasharrajat]

Yes, I am back to my primary location and try to finish asap.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Fix form accessibility 2/4 #12370

If no regressions arise, payment will be issued on 2023-02-03. 🎊

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 / @puneetlath / @thienlnam] The PR that introduced the bug has been identified. Link to the PR: n/a not a bug, this is new behavior we decided to implement
• [@parasharrajat / @puneetlath / @thienlnam] 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: n/a not a bug, this is new behavior we decided to implement
• [@parasharrajat / @puneetlath / @thienlnam] 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: n/a not a bug, this is new behavior we decided to implement
• [@puneetlath] 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: n/a not a bug, this is new behavior we decided to implement

[puneetlath]

My sense is that there wasn't really a PR that caused a specific bug. Rather we developed form guidelines and then went and implemented them. Do you agree with that @parasharrajat @thienlnam?

[parasharrajat]

Yeah, these are new changes to change the existing form behavior.

[puneetlath]

For me:
@mdneyazahmad contract: https://www.upwork.com/nx/wm/workroom/30032384/overview
@parasharrajat contract: https://www.upwork.com/nx/wm/pre-hire/c/8577561/offer/23159468

[puneetlath]

All paid. Thanks for your contributions everyone!