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.33k stars 2.76k forks source link

[HOLD for payment 2024-09-09] [$250] Subscription - Clicking Outside the Modal Doesn't Close It, Causing Inconsistency #47559

Open izarutskaya opened 1 month ago

izarutskaya commented 1 month 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: v9.0.21-1 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+tw23478102034@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Go to Account Settings > Subscription.
  2. Add Payment Card > Click on the three-dot menu > Change Payment Currency.
  3. Select a currency > Click outside the modal. Notice the modal doesn't close.
  4. Repeat: Click the three-dot menu > Change Payment Card > Currency > Click outside. This time, the modal closes, creating inconsistency.

Expected Result:

After opening the Change Payment Currency modal and selecting a currency, clicking outside the modal should close it.

Actual Result:

After opening the Change Payment Currency modal and selecting a currency, clicking outside the modal does not close it. This creates an inconsistency compared to other instances (e.g., when changing payment card>Currency), where clicking outside the modal closes it.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/6e213e8e-b57d-4759-a802-d69974e84962

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a4445f00bf760f92
  • Upwork Job ID: 1824461014015831545
  • Last Price Increase: 2024-08-16
  • Automatic offers:
    • shubham1206agra | Reviewer | 103587586
    • Krishna2323 | Contributor | 103587588
Issue OwnerCurrent Issue Owner: @shubham1206agra
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @puneetlath (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.

izarutskaya commented 1 month ago

We think this issue might be related to the #collect project.

nkdengineer commented 1 month ago

Proposal

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

After opening the Change Payment Currency modal and selecting a currency, clicking outside the modal does not close it. This creates an inconsistency compared to other instances (e.g., when changing payment card>Currency), where clicking outside the modal closes it.

What is the root cause of that problem?

Two places use different ways.

  1. PaymentCardForm, we are using CurrencySelector as a separate page then when we click outside, it's closed https://github.com/Expensify/App/blob/9940e99abca5b0e7f17c8d29a66df50b2345b8a2/src/components/AddPaymentCard/PaymentCardForm.tsx#L296-L301

  2. PaymentCardChangeCurrencyForm, we're using a currency modal. Then when we click outside, only the PaymentCardCurrencyModal is closed https://github.com/Expensify/App/blob/9940e99abca5b0e7f17c8d29a66df50b2345b8a2/src/components/AddPaymentCard/PaymentCardChangeCurrencyForm.tsx#L110

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

    We should handle the currency as a separate page by using CurrencySelector as we do in PaymentCardForm. Since PaymentCardChangeCurrencyForm is used in two places, we can add currencySelectorRoute prop and use a unique route for each place.

What alternative solutions did you explore? (Optional)

Or we can use the same way as we do in PaymentCardChangeCurrencyForm for PaymentCardForm

Krishna2323 commented 1 month ago

Proposal

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

Subscription - Clicking Outside the Modal Doesn't Close It, Causing Inconsistency

What is the root cause of that problem?

We aren't using onBackdropPress={Navigation.dismissModal} prop to dismiss the modal when backdrop is pressed. The same is done here & here. https://github.com/Expensify/App/blob/9940e99abca5b0e7f17c8d29a66df50b2345b8a2/src/components/AddPaymentCard/PaymentCardCurrencyModal.tsx#L51-L59

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

Pass onBackdropPress={Navigation.dismissModal} prop to Modal.

Optional: We can introduce new props to pass onBackdropPress prop from ChangeBillingCurrency & ChangeCurrency.

What alternative solutions did you explore? (Optional)

Krishna2323 commented 1 month ago

Proposal Updated

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

shubham1206agra commented 3 weeks ago

@Krishna2323's proposal looks good to me.

πŸŽ€πŸ‘€πŸŽ€ C+ reviewed

melvin-bot[bot] commented 3 weeks ago

Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

melvin-bot[bot] commented 3 weeks ago

πŸ“£ @shubham1206agra πŸŽ‰ An offer has been automatically sent to your Upwork account for the Reviewer role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 3 weeks ago

πŸ“£ @Krishna2323 πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

puneetlath commented 3 weeks ago

Is this not the default behavior for modals? Should we make it the default if it isn't?

melvin-bot[bot] commented 3 weeks ago

@puneetlath, @shubham1206agra, @Krishna2323 Whoops! This issue is 2 days overdue. Let's get this updated quick!

Krishna2323 commented 3 weeks ago

I will raise the PR today.

shubham1206agra commented 3 weeks ago

Waiting for PR.

shubham1206agra commented 1 week ago

@trjExpensify Can you add payment label for 9th September?

puneetlath commented 5 days ago

I added it. @shubham1206agra can you please complete the checklist?