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

fix(modal-checkout): move checkout_error trigger #1905

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

miguelpeixe
Copy link
Member

All Submissions:

Changes proposed in this Pull Request:

The checkout_error may not always run because the script might end up running after the DOMContentLoaded event dispatch. This PR tweaks the approach so the trigger runs in the modal script, which runs inside domReady() and is ensured to always run.

How to test the changes in this Pull Request:

  1. While on the epic/ras-acc branch, use the checkout button to purchase a subscription you already own (make sure the subscription product is configured to restrict such purchase)
  2. Click the checkout button a few times to confirm it sometimes hangs in the loading state and sometimes goes through with the correct error message (try also throttling the internet connection in a few attempts)
  3. Checkout this branch, click the checkout button several times and confirm the error message renders consistently

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Copy link
Contributor

@chickenn00dle chickenn00dle left a comment

Choose a reason for hiding this comment

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

Great catch!

@miguelpeixe miguelpeixe merged commit 3f5c91c into epic/ras-acc Oct 14, 2024
10 checks passed
@miguelpeixe miguelpeixe deleted the fix/checkout-error-trigger branch October 14, 2024 18: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.

2 participants