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(ras-acc): performance enhancements and small fixes #1871

Merged
merged 3 commits into from
Sep 8, 2024

Conversation

chickenn00dle
Copy link
Contributor

@chickenn00dle chickenn00dle commented Sep 4, 2024

All Submissions:

Changes proposed in this Pull Request:

This PR makes some small fixes and performance updates.

This PR was opened while digging into potential issues with RAS-ACC loading times. We found that the issue wasn't specific to RAS-ACC but wanted to push these changes up anyway.

How to test the changes in this Pull Request:

  • Smoke test modal checkout flows

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?

@@ -7,7 +7,7 @@
* @return {void}
*/
export function domReady( callback ) {
if ( typeof document === 'undefined' ) {
if ( typeof document === 'undefined' || typeof callback !== 'function' ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addresses a random browser error where callback was undefined in some circumstances.

Comment on lines +410 to +413
// Signal checkout registration.
form.appendChild(
createHiddenInput( newspackBlocksModal.checkout_registration_flag, '1' )
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only want this called whenever the auth modal is opened by checkout.

@chickenn00dle chickenn00dle marked this pull request as ready for review September 5, 2024 03:34
@chickenn00dle chickenn00dle requested a review from a team as a code owner September 5, 2024 03:34
Copy link
Contributor

@laurelfulford laurelfulford left a comment

Choose a reason for hiding this comment

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

Looks good!

I tested this alongside Automattic/newspack-plugin#3391, so it included making purchases when already logged in and when I needed to sign up/sign in, and all worked well!

@chickenn00dle chickenn00dle merged commit f7c54d0 into epic/ras-acc Sep 8, 2024
12 checks passed
@chickenn00dle chickenn00dle deleted the fix/performance-enchancements branch September 8, 2024 22:34
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