App: $500 - Audit forms and fix inconsistencies with focus, tab and shift + tab behavior 4/4

You can go to Settings > Payments page. Then Click Add payment method > Add bank account. Then follow on till the end when the bank account is added to the account. Use testing creads for Plaid which you can get from their docs.

@luacmartins Could you please suggest if we are fine with focus-trapping in our pages? I think It would be needed to move the focus to RHN.

I think it makes sense to use focus trapping with the RHN since it blocks the rest of the app until we dismiss it.

Thank you both! I appreciate that extra context a lot. I’ve proposed this compensation in Slack and will issue payment accordingly!

I definitely agree with @mdneyazahmad. Technically, no PR was done for https://github.com/Expensify/App/issues/7917 but it is still priced at 2k. Main work was done for 2 and 4. 3 was put on hold during the PR review.

So I think 500 for C+ in this issue is less. But I am fine with it as it was the price when the job was assigned (based on the process).

@MitchExpensify first of all all of these issues are of similar nature, maybe there was not require to break into 4, but it was broken into 4 to keep it simple.

1. As these are different issue, I think the price should not be adjusted with others otherwise no need to break.
2. As you can see the amount paid for 2/4 is just 250, but @parasharrajat better knows the effort and time required to complete that issue. Still I did not ask for a raise, because I thought, I can adjust with others.
3. The task required to do in this issue is to audit and fix any inconsistency. I have done the audit in the past. As these issues are similar. It has has to wait for the common PR. I would have started the 1st PR from any issue. But I thought 2/4 was little easy, therefore PR primarily addresses that issue.
4. We can also keep in mind the time this issues take.

I think the original price for this issue is justified. Maybe @parasharrajat has some input.

Thank you

@MitchExpensify Can you open an internal discussion about the above in Slack? 🙇‍♀️

Payment for this issue is still pending. There was multiple PR involved for these issues titled Audit forms and fix inconsistencies with focus, tab, and shift + tab behavior. I agree with @mdneyazahmad that a lot of research and time was spent on finding these solutions.

Thanks for hearing me out!

Taking this off hold.

Still waiting on https://github.com/Expensify/App/pull/10965

@MitchExpensify This is not complete. There is one more PR for this issue that will be created after https://github.com/Expensify/App/pull/10643 is merged. You can remove the payment label. Thanks!

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

• https://github.com/Expensify/App/pull/8874

If no regressions arise, payment will be issued on 2022-09-08. 🎊

We have finalised the solution here. I will update the PR soon. @parasharrajat checkbox PR is updated.

Kind of like that you can suggest a better approach. I think we have some more questions like handling it for multiple buttons and need some more discussion around that. Should we discuss on the same pr already drafted?

@MitchExpensify

“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”

We are moving ahead with this option, any further comment related to this option will be made on this PR. Thank you.

Discussion is on going in this slack thread.

So the old contract needs to be cancelled as a duplicate.

I’d prefer to manage payment centrally in the new listing @parasharrajat

Posted a possible solution on the slack thread.

Let me see if I understood the problem correctly, we can’t use onEnter because it creates too many secondary unexpected behaviors, right? I am worried that if we remove pressOnEnter and we try to find another solution for only these cases, we are just looking for a hacky solution instead of solving it for all cases. @mdneyazahmad Can you bring up this issue in #expensify-open-source to see what the team thinks?

Use isVisible prop on the LoginForm and PasswordForm to control pressOnEnter.

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

Thanks for pointing it out. I am looking into that issue on another GH https://github.com/Expensify/App/issues/7623#issuecomment-1085901192.

Hmm maybe this thread might help?

Update: I will create pr by tomorrow.

Hired @parasharrajat for eventual C+ payment @mdneyazahmad ready to accept your proposal once made! Thanks

@mdneyazahmad Feel free to apply for the job in Upwork and start working on a PR for this!

No worries. Take your time. I will make sure that you are assigned to another issue as well if you submit a good proposal.

When a modal opens, we will find the first focusable element and focus it.

I don’t think we need this. Just the focus should cycle as you said. And on the first tab press, the focus should be on the first focusable element on the page and follow the proper focus pattern of the browser.

does it mean we should use this library?

Exactly, my question as well. Haha…

@parasharrajat I also think the same we should trap it and it is the recommended pattern for a modal. Consider the case when you open a modal and press multiple tabs. The focus will be lost and will be focused on some another element that UI does not allow to interact with.

https://user-images.githubusercontent.com/77761491/157665747-fe51984a-7b87-45ac-a5a9-012bbb9364e4.mov

Ah, we basically decided to break the issues into four smaller issues since we weren’t sure about milestones. So now just proceed as normal, posting onto Upwork, adding exported, etc!