search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.58k stars 2.92k forks source link

[$150] IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you #52959

Open lanitochka17 opened 4 days ago

lanitochka17 commented 4 days ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 9.0.65.1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Issue found when executing PR https://github.com/Expensify/App/pull/52232

Action Performed:

precondition: user has enabled Expensify card

  1. Open the app and log in
  2. Navigate to the account settings > Security >Copilot
  3. Continue with the flow
  4. On "Let's make sure it is you" step, tap the back icon
  5. Create a new workspace and open the workspace settings
  6. Select More feature and enable Workflows
  7. Open Workflows > toggle on "Make or track payments" option
  8. Tap Connect bank account > any option
  9. On "Let's make sure it is you" step, tap the back icon
  10. Navigate to the workspace with Expensify card enabled
  11. Start issuing a new card
  12. On "Let's make sure it is you" step, tap the back icon
  13. Navigate to account settings > Profile > Contact method
  14. Start adding a new contact method
  15. On "Let's make sure it is you" step, tap the back icon

Expected Result:

User is navigated to the previous screen after tapping the back icon on "Let's make sure it is you" in the flows where code validation is required

Actual Result:

When tapping the back icon on "Let's make sure it is you" step in the Add copilot and Add bank account flows (steps 4 and 9), the keyboard gets dismissed while in the other flows (Issue a new card, Add new contact method, Reveal card details) user is returned to the previous screen immediately (steps 12 and 15)

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/e9500f66-66e4-43b5-88c2-42e9df2752b0

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021861312036312016837
  • Upwork Job ID: 1861312036312016837
  • Last Price Increase: 2024-11-26
Issue OwnerCurrent Issue Owner: @thesahindia
melvin-bot[bot] commented 4 days ago

Triggered auto assignment to @VictoriaExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

NJ-2020 commented 4 days ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

IOS/Android-Verify account-Inconsistent behavior when returning from Let's make sure it is you

What is the root cause of that problem?

It's because the ValidateCodeActionModal is wrapped inside ScrollView component https://github.com/Expensify/App/blob/8d69d60c19830efd6e00cda93966447ad84b360b/src/pages/ReimbursementAccount/BankAccountStep.tsx#L228-L243

I can see like inside Profile page the ValidateCodeActionModal got dismissed properly when we click the back button because the ValidateCodeActionModal is not wrapped by ScrollView https://github.com/Expensify/App/blob/8d69d60c19830efd6e00cda93966447ad84b360b/src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L156-L180

But in the other side like inside Bank account page the ValidateCodeActionModal is not get dismissed properly because it's wrapped by ScrollView, example here: BankAccountPage https://github.com/Expensify/App/blob/8d69d60c19830efd6e00cda93966447ad84b360b/src/pages/ReimbursementAccount/BankAccountStep.tsx#L228-L243 Copilot https://github.com/Expensify/App/blob/8d69d60c19830efd6e00cda93966447ad84b360b/src/pages/settings/Security/AddDelegate/DelegateMagicCodeModal.tsx#L49-L52 https://github.com/Expensify/App/blob/8d69d60c19830efd6e00cda93966447ad84b360b/src/pages/settings/Security/AddDelegate/ConfirmDelegatePage.tsx#L76-L88 https://github.com/Expensify/App/blob/8d69d60c19830efd6e00cda93966447ad84b360b/src/components/HeaderPageLayout.tsx#L101-L107

What changes do you think we should make in order to solve the problem?

We should put the ValidateCodeActionModal outside the ScrollView https://github.com/Expensify/App/blob/8d69d60c19830efd6e00cda93966447ad84b360b/src/pages/ReimbursementAccount/BankAccountStep.tsx#L228-L243 And we can do the same approach for other pages also

Result

https://github.com/user-attachments/assets/7c02470e-b897-444b-93be-c293b75d567a

What alternative solutions did you explore? (Optional)

bernhardoj commented 4 days ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Inconsistent behavior on some pages that use the magic code validation modal.

What is the root cause of that problem?

For the copilot and bank account validation modal, the validation modal is wrapped inside a ScrollView. https://github.com/Expensify/App/blob/a35df5b8a29a21a63b28eeb2d929efc1087ca8ff/src/pages/settings/Security/AddDelegate/ConfirmDelegatePage.tsx#L76-L88 https://github.com/Expensify/App/blob/a35df5b8a29a21a63b28eeb2d929efc1087ca8ff/src/components/HeaderPageLayout.tsx#L101-L107 https://github.com/Expensify/App/blob/a35df5b8a29a21a63b28eeb2d929efc1087ca8ff/src/pages/ReimbursementAccount/BankAccountStep.tsx#L228-L242

ScrollView by default will dismiss the keyboard first when tapping on it, so we need to press the back button again to close the modal. Compared to the issue new card confirm page, the modal is outside the scroll view, so it doesn't have this problem.

What changes do you think we should make in order to solve the problem?

In a normal case, we won't have this problem because a header is usually not wrapped in a scroll view. But in this case, it's a bit different because the modal is put inside the scroll view, so the whole modal is affected. To fix this, we can keyboardShouldPersistTaps="handled" to the scroll view.

<ScrollView keyboardShouldPersistTaps="handled">

It's the same as what we did on the contact method details page. https://github.com/Expensify/App/blob/a35df5b8a29a21a63b28eeb2d929efc1087ca8ff/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.tsx#L244-L273

This is especially useful for cases like the copilot confirm page because it uses HeaderPageLayout and the children is always wrapped with a scroll view. For other cases, if it's possible to put the modal outside the scroll view, then we put it outside the scroll view.

It's technically possible for the copilot confirm page too by doing it like this:

<>
    <HeaderPageLayout ... />
    <DelegateMagicCodeModal ... />
</>

But I think it would be cleaner to just apply keyboardShouldPersistTaps="handled" to the scroll view inside HeaderPageLayout (we can add a new prop to control it if needed).

Also, there are other pages that put the modal inside the scroll view too which we may need to fix too, such as the CodesStep. https://github.com/Expensify/App/blob/a35df5b8a29a21a63b28eeb2d929efc1087ca8ff/src/pages/settings/Security/TwoFactorAuth/Steps/CodesStep.tsx#L172-L186

VictoriaExpensify commented 20 hours ago

Hmm ok so I think this is a pretty minor issue, but because we do want everything to be consistent, we should fix it.

My feeling is that this is a pretty easy fix so I'm going to lower the bounty on this one. If I'm missing anything to do with the complexity here, please let me know.

melvin-bot[bot] commented 20 hours ago

Job added to Upwork: https://www.upwork.com/jobs/~021861312036312016837

melvin-bot[bot] commented 20 hours ago

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

melvin-bot[bot] commented 20 hours ago

Upwork job price has been updated to $150