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

[NT-969] Show full-screen "processing" state during create/update backing requests #1150

Merged
merged 7 commits into from
Apr 13, 2020

Conversation

ifbarrera
Copy link
Contributor

πŸ“² What

Adds a full-screen "processing" view that displays when a create or update backing request is in flight, to prevent users from interacting with the app during critical requests.

πŸ€” Why

We want to guarantee that the user can't submit another backing request while one is in flight.

πŸ›  How

  • added ProcessingView which is added to the app's mainWindow when either the createBacking or updateBacking requests are in flight
  • the ProcessingView is removed from the hierarchy when the request terminates

πŸ‘€ See

Trello, screenshots, external resources?

Pledging with Card Apple Pay iPad Update Backing
5mZW7yzcRD XLqBEGNOtB oDsk4GR0GW-2 BzMYkIAf9j

βœ… Acceptance criteria

  • Run the app on the simulator, switch to staging. Back any project using a stored card. You should see the "Processing..." state momentarily before seeing the Thank You screen.
  • Run the app on the simulator, switch to staging. Back any project using Apple Pay. You should see the "Processing..." state momentarily before seeing the Thank You screen.
  • Navigate to a backed project, and tap "Manage". Tap "Change your payment method" and update your payment method. You should see the "Processing..." screen momentarily before seeing the success banner.

⏰ TODO

  • update "Processing..." string

Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

Very nice, @ifbarrera . I just left a minor comment and a suggestion.

@@ -744,6 +764,7 @@ public class PledgeViewModel: PledgeViewModelType, PledgeViewModelInputs, Pledge
public let paymentMethodsViewHidden: Signal<Bool, Never>
public let pledgeAmountViewHidden: Signal<Bool, Never>
public let pledgeAmountSummaryViewHidden: Signal<Bool, Never>
public let processingViewIsHidden: Signal<Bool, Never>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you alpha here?

|> ksr_addSubviewToParent()
|> ksr_constrainViewToEdgesInParent()

self.processingView.activityIndicator.startAnimating()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could start animating the indicator in the ProcessingView's init method. This way we don't need to expose its UIActivityIndicatorView property and it's one less thing to worry about, since we don't need to start/stop the animation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

|> \.numberOfLines .~ 0
|> \.text %~ { _ -> String in
var processingString = Strings.project_checkout_finalizing_title()
processingString.append("...")
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering the localizations, I wonder if it's ok to append the ... to the string.

Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

🚒 β›΅ 🏊

@ifbarrera ifbarrera merged commit 6a9ed36 into master Apr 13, 2020
@ifbarrera ifbarrera deleted the NT-969-pledge-processing branch April 13, 2020 14:41
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