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

Show native loading spinner only on initial load #38

Closed
wants to merge 1 commit into from

Conversation

josemiguel-alvarez
Copy link
Contributor

@josemiguel-alvarez josemiguel-alvarez commented Oct 23, 2023

Closes https://github.com/Shopify/checkout-sdk-issues/issues/196
Closes https://github.com/Shopify/checkout-sdk-issues/issues/207

What are you trying to accomplish?

Instead of displayed the native spinner on every navigation, we should show the native loading spinner only on initial load.

23-29-d0iko-ei3m2.mp4

Before you deploy

  • I have added tests to support my implementation
  • I have read and agree with the contributing documentation readme
  • I have read and agree with the code of conduct documentation readme
  • I have updated any documentation related to these changes.
  • I have updated the README (if applicable).

@josemiguel-alvarez josemiguel-alvarez self-assigned this Oct 23, 2023
@josemiguel-alvarez josemiguel-alvarez requested a review from a team as a code owner October 23, 2023 13:32
@@ -25,7 +25,6 @@ import UIKit
import WebKit

protocol CheckoutViewDelegate: AnyObject {
func checkoutViewDidStartNavigation()
Copy link
Contributor

Choose a reason for hiding this comment

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

We will most likely need this method when we implement the next spinner solution so I would leave this delegate method for now, just remove the default behaviour here

}
spinner.startAnimating()
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't we want to do this under line 110? Does this work with preloading?

@josemiguel-alvarez
Copy link
Contributor Author

@markmur is actively working on the SDK Loading issue and will take over this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants