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

feat(payment): PAYPAL-4692 fixed credit card loading glitch #2120

Merged

Conversation

andriiVitvitskyi1990
Copy link
Contributor

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

@andriiVitvitskyi1990 andriiVitvitskyi1990 force-pushed the PAYPAL-4692 branch 3 times, most recently from 1a81486 to 119b009 Compare December 12, 2024 16:00
@andriiVitvitskyi1990 andriiVitvitskyi1990 marked this pull request as ready for review December 12, 2024 16:16
@andriiVitvitskyi1990 andriiVitvitskyi1990 requested a review from a team as a code owner December 12, 2024 16:16
);

const loadingOverlay = component.find(LoadingOverlay);
setTimeout(() => {
Copy link
Contributor

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()?

Copy link
Contributor Author

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

Copy link
Contributor

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(() => {
Copy link
Contributor

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

Suggested change
setTimeout(() => {
await new Promise((resolve) => process.nextTick(resolve));
component.update();
const loadingOverlay = component.find(LoadingOverlay);
expect(loadingOverlay.prop('isLoading')).toBe(false);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -89,6 +90,7 @@ class CreditCardPaymentMethod extends Component<
> {
state: CreditCardPaymentMethodState = {
isAddingNewCard: false,
isPreloaderOn: true,
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor

@animesh1987 animesh1987 Dec 17, 2024

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.

Copy link
Contributor Author

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

Copy link
Contributor Author

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

@andriiVitvitskyi1990 andriiVitvitskyi1990 merged commit 928c973 into bigcommerce:master Dec 26, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants