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

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

@mdneyazahmad please note that 4. Space toggles checkboxes/dropdowns. was added to the list of expected behavior.

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

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?

I’m just catching up here. Are all of these changes being handled in one PR?

@parasharrajat thanks for your review. Let’s merge this asap. I will be active on the PR.

@parasharrajat do you have an ETA for reviewing this issue?

@michaelhaxhiu just updated the label on this to daily! What needs to happen to get this over the finish line?

@parasharrajat PR is ready for review.

Consensus in the thread is to move forward with:

“Continue using a single global keyboard event handler library, patching it with the solution from https://github.com/Expensify/App/pull/9749 so that local event handlers can override the global ones”

Working on this PR

@mdneyazahmad do you mind responding to @roryabraham on the Slack Discussion that you posted/linked above? Thanks!

I see - we will discuss in slack to arrive at our next steps. Let’s post the slack link in that GH after you have posted (just to keep visibility!).

@michaelhaxhiu As said here. It has some issue with Button onPressEnter. This is the issue. You can read last few comments on this issue to fully understand the related issue. Let me know what do you think. Thank you!

Seems like that issue was fixed already.

@luacmartins I think that is not related to this issue.

PR in review for fixing checkbox, still awaiting PRs for other items

Ah I see! Yea, I think in that case we should NOT submit the form and just perform the action of the selected button (on both space and enter press).

@michaelhaxhiu I am ooo and will be back in 1 - 2 days. I am sorry for not informing about it. tab and shift + tab works as expected. It has some other issue that I mentioned here https://github.com/Expensify/App/issues/7916#issuecomment-1080175468

I think this is the problem here in workspace invite member.

This form contains OptionList accessibility of this component seems broken❌

1. Arrow key does not work.
2. It does not toggle selection with space.
3. It toggles with enter.
4. When you press enter on selected field it will toggle as well as submit the form (related to the other issue mentioned).

https://user-images.githubusercontent.com/77761491/165665581-ce182bd5-5a89-4581-8c6d-865c1a1804ce.mp4

https://user-images.githubusercontent.com/77761491/165665601-ba9c6881-8f81-4329-b62f-fd749a0b1b70.mp4

https://user-images.githubusercontent.com/77761491/165665825-410478e3-0880-4868-8e37-e026fb3ca9b5.mp4

I think you can move ahead with pressOnEnter in this issue. Do you need anything else?

@mdneyazahmad Actually, we’ll have to leave those bank account forms to an internal audit. Just address the other forms for now

@michaelhaxhiu applied

Thanks for doing the audit @mdneyazahmad. There are some great points. For this issue, we will only be focusing on fixing

Tab navigates to the next input.
Shift + tab navigates to the previous input.
Enter submits the form.

RequestorStep, ValidationStep

Try opening the URLs directly on the Web.

---

In my opinion, @mdneyazahmad is up for the task.

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

But this point talks about tabbing cycles through for this to work we need focus trapping. But we decided about to do that as a separate issue. Just leaving a note for CME.

cc: @thienlnam

🎀 👀 🎀 C+ reviewed

I am waiting for @mdneyazahmad’s new proposal with focus-trap. But I see that this issue is more about audits. @mdneyazahmad Would you like to do the audit? Focus-trap will not be part of it.

Reviewing the proposal on another issue.

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

@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.