-
Notifications
You must be signed in to change notification settings - Fork 365
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
feat(payment): PAYPAL-4692 fixed credit card loading glitch #2120
feat(payment): PAYPAL-4692 fixed credit card loading glitch #2120
Conversation
1a81486
to
119b009
Compare
); | ||
|
||
const loadingOverlay = component.find(LoadingOverlay); | ||
setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you try how it will be working with process.nextTick()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It did not work, the problem is we need to wait until initializePayment finish and then check loading overlay, but in test we have updated isLoading prop after we get LoadingOverlay component because of initializePayment async. So i've used setTimeout to rearrange call stack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment must be here: #2120 (comment)
); | ||
|
||
const loadingOverlay = component.find(LoadingOverlay); | ||
setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for explanation. Than you can try to do something like this. It should be work because after nextTick
we update component and try checking the loadingOverlay's isLoading
prop with a new updated value
setTimeout(() => { | |
await new Promise((resolve) => process.nextTick(resolve)); | |
component.update(); | |
const loadingOverlay = component.find(LoadingOverlay); | |
expect(loadingOverlay.prop('isLoading')).toBe(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
119b009
to
a9ddd90
Compare
@@ -89,6 +90,7 @@ class CreditCardPaymentMethod extends Component< | |||
> { | |||
state: CreditCardPaymentMethodState = { | |||
isAddingNewCard: false, | |||
isPreloaderOn: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This component is used by a quite a few providers, have you tested if the other providers work fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This state sets to false whenever any of providers finish initialization, so should work fine for any provider that uses this component. I've checked PaypalCommerce and Braintree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please test WP access and Hosted credit card as well. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by WP access? Could you explain please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@animesh1987 I've tested PaypalCommerce, Braintree and regular Credit Card with no provider onboarded
What?
Fixed credit card loading glitch
Why?
To fix bug
Testing / Proof
Before
Screen.Recording.2024-12-09.at.13.23.23.mov
After
Screen.Recording.2024-12-09.at.13.25.31.mov
@bigcommerce/team-checkout