Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PAY-1954] Loading User Settings And Cancelling Payment Sheet #1738

Merged
merged 10 commits into from Sep 26, 2022

Conversation

msadoon
Copy link
Contributor

@msadoon msadoon commented Sep 21, 2022

📲 What

Due to the time required to get a setup intent token and for show the payment sheet. Also after the payment sheet is dismissed there is a lag between showing the newly created payment method. That lag is caused by the network to create the new payment source. We might want to let the user know that their actions are being process via a "loading" state.

There's also the necessity to cancel the payment sheet, if its about to be presented but another user action occurs to cancel its appearance (ie. click on "Add new card", click on "Edit" while waiting for the sheet to be presented.)

🤔 Why

User should know that any action taken while loading the payment sheet, cancels its appearance.
User should also know any action taken to change the state of the page will cancel the sheet's appearance.

🛠 How

New output on PaymentMethodsViewModel cancelAddNewCardLoadingState to inform the PaymentMethodsFooterView through a new delegate that only cancels the loading state on that view.
The view itself has a button that whenever clicked and the feature flag is on, will show the loading state.

In the view model, all actions that cancel the loading state on the view go through shouldCancelPaymentSheetAppearance to update cancelAddNewCardLoadingState if they are true. And they are true only when user clicks, Edit, deletes an existing card or the payment methods get updated.

👀 See

Before 🐛

Simulator.Screen.Recording.-.iPhone.8.-.2022-09-22.at.16.10.01.mp4

After 🦋

Simulator.Screen.Recording.-.iPhone.8.-.2022-09-22.at.16.04.35.mp4

| | |

✅ Acceptance criteria

  • Add new card shows a loading state before and after the payment sheet appears.
  • Before payment sheet appears the loading state and appearance of the payment sheet can be cancelled by another user action (ie. Edit, delete of a card).

⏰ TODO

  • Add loading state to "Add new card"
  • Write tests.

@msadoon msadoon self-assigned this Sep 21, 2022
@msadoon msadoon added this to the release-5.5.0 milestone Sep 21, 2022
@msadoon msadoon added the WIP label Sep 21, 2022
@msadoon msadoon marked this pull request as ready for review September 22, 2022 20:02
@msadoon msadoon requested review from scottkicks and a team September 22, 2022 20:02
@msadoon msadoon added needs review and removed WIP labels Sep 22, 2022
@codecov
Copy link

codecov bot commented Sep 23, 2022

Codecov Report

Merging #1738 (ca21e18) into main (ce821ad) will increase coverage by 3.62%.
The diff coverage is 96.87%.

❗ Current head ca21e18 differs from pull request most recent head f0ff08f. Consider uploading reports for the commit f0ff08f to get more accurate results

@@            Coverage Diff             @@
##             main    #1738      +/-   ##
==========================================
+ Coverage   85.35%   88.98%   +3.62%     
==========================================
  Files        1275      881     -394     
  Lines      114938    79425   -35513     
  Branches    30377    20877    -9500     
==========================================
- Hits        98110    70678   -27432     
+ Misses      15776     8004    -7772     
+ Partials     1052      743     -309     
Impacted Files Coverage Δ
Library/ViewModels/PaymentMethodsViewModel.swift 98.20% <93.33%> (-0.51%) ⬇️
...rary/ViewModels/PaymentMethodsViewModelTests.swift 94.76% <100.00%> (+0.23%) ⬆️
Library/MockPushRegistration.swift 0.00% <0.00%> (-100.00%) ⬇️
Library/TestHelpers/MockOptimizelyClient.swift 76.92% <0.00%> (-7.70%) ⬇️
Library/TestHelpers/TestCase.swift 81.15% <0.00%> (-7.25%) ⬇️
...hods/Controller/PaymentMethodsViewController.swift
...aymentMethods/Views/PaymentMethodsFooterView.swift
...tures/RewardsCollection/Views/Cells/PillCell.swift
...overy/Controller/DiscoveryPageViewController.swift
...ManageViewPledgeRewardReceivedViewController.swift
... and 391 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@msadoon msadoon merged commit d100e91 into main Sep 26, 2022
@msadoon msadoon deleted the pay-1954/loading-user-settings branch September 26, 2022 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants