Skip to content

feat: improve performance of modal checkout #1607

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.

Sign up for GitHub

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

Jump to bottom
Merged
adekbadek merged 8 commits into from
Nov 30, 2023
Merged

feat: improve performance of modal checkout #1607

adekbadek merged 8 commits into from
Nov 30, 2023

Conversation

@adekbadek
Copy link
Member

@adekbadek adekbadek commented Nov 27, 2023

All Submissions:

Changes proposed in this Pull Request:

Improves the performance of the modal checkout by skipping unnecessary markup.

It also fixes a PHP Notice caused by changes in #1601.

How to test the changes in this Pull Request:

  1. On master, start a new session (or clear the cookies)
  2. Trigger the modal checkout, copy the URL of the modal's iframe and visit it directly*
  3. Observe the issues (deactivate Perfmatters if active, because it will make it more difficult to inspect):
    1. Complianz cookie banner is displayed in the iframe
    2. reCaptcha badge, too
    3. Newspack Ads' JS is loaded (if configured)
    4. Newspack print styles are loaded
    5. GA is loaded
    6. If Jetpack "WooCommerce Analytics" module is active**, a whole bunch of React and Redux JS is loaded
    7. WP mediaelement scripts and styles are loaded
    8. PHP Notice is logged, notifying that "Function wc_get_template was called incorrectly"
  4. Switch to this branch, and to Feat/some filters newspack-plugin#2768 and feat: filter JS and print styles enqueuing newspack-theme#2207 on Plugin and Theme repos
  5. Observe errors gone 💨
  6. Complete a transaction using the modal checkout, verify that it works as before

* copy(document.querySelector('iframe[name="newspack_modal_checkout"]').contentWindow.location.href) in the JS console
** /wp-admin/admin.php?page=jetpack_modules

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?

@adekbadek adekbadek requested a review from a team as a code owner November 27, 2023 15:03
@dkoo dkoo changed the title Feat/modal checkout perf feat: improve performance of modal checkout Nov 27, 2023
miguelpeixe
miguelpeixe requested changes Nov 28, 2023
View reviewed changes
Copy link
Member

@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

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

Works well! Should be ok to merge after feedback on the reCAPTCHA badge visibility and the Automattic/newspack-plugin#2768 and Automattic/newspack-theme#2207 are approved.

src/modal-checkout/checkout.scss Outdated
Comment on lines 270 to 272
.grecaptcha-badge {
display: none !important;
}
Copy link
Member

@miguelpeixe miguelpeixe Nov 28, 2023

Choose a reason for hiding this comment

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

According to reCAPTCHA's FAQ, the badge visibility should be managed through visibility: hidden; and only if the form includes the following:

This site is protected by reCAPTCHA and the Google
    <a href="https://policies.google.com/privacy">Privacy Policy</a> and
    <a href="https://policies.google.com/terms">Terms of Service</a> apply.

The modal redesign is taking that into account and we should have a good placement for the required text soon. What do you think of leaving this change to the redesign?

More on this in the comments of 1205234045751551-as-1205933702651188

Copy link
Member Author

@adekbadek adekbadek Nov 29, 2023

Choose a reason for hiding this comment

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

Ok! If that's covered in future work, it does not have to be included here.

@adekbadek adekbadek merged commit a48d190 into master Nov 30, 2023
@adekbadek adekbadek deleted the feat/modal-checkout-perf branch November 30, 2023 07:32
matticbot pushed a commit that referenced this pull request Nov 30, 2023
@semantic-release-bot
chore(release): 2.3.0-alpha.1 [skip ci]
94fc68c 
# [2.3.0-alpha.1](v2.2.2...v2.3.0-alpha.1) (2023-11-30)

### Bug Fixes

* modal checkout template markup ([#1608](#1608)) ([4d593db](4d593db))
* **modal-checkout:** prevent initial render of details table ([#1601](#1601)) ([06d4ccd](06d4ccd))

### Features

* improve performance of modal checkout ([#1607](#1607)) ([a48d190](a48d190))
* **modal-checkout:** add filter to cart item data ([#1590](#1590)) ([1e83dc1](1e83dc1))

### Performance Improvements

* **modal-checkout:** process checkout request earlier ([#1612](#1612)) ([5c58f5e](5c58f5e))
@matticbot
Copy link
Contributor

matticbot commented Nov 30, 2023

🎉 This PR is included in version 2.3.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Dec 7, 2023
@semantic-release-bot
chore(release): 2.4.0-alpha.1 [skip ci]
256390e 
# [2.4.0-alpha.1](v2.3.0...v2.4.0-alpha.1) (2023-12-07)

### Bug Fixes

* modal checkout template markup ([#1608](#1608)) ([4d593db](4d593db))
* **modal-checkout:** align Stripe's "save payment" checkbox ([#1623](#1623)) ([69e0e42](69e0e42))
* **modal-checkout:** prevent initial render of details table ([#1601](#1601)) ([06d4ccd](06d4ccd))
* replace FILTER_SANITIZE_STRING ([6f805b0](6f805b0))

### Features

* improve performance of modal checkout ([#1607](#1607)) ([a48d190](a48d190))
* **modal-checkout:** add filter to cart item data ([#1590](#1590)) ([1e83dc1](1e83dc1))

### Performance Improvements

* **modal-checkout:** process checkout request earlier ([#1612](#1612)) ([5c58f5e](5c58f5e))
@matticbot
Copy link
Contributor

matticbot commented Dec 7, 2023

🎉 This PR is included in version 2.4.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Dec 11, 2023
@semantic-release-bot
chore(release): 2.4.0 [skip ci]
494762c 
# [2.4.0](v2.3.0...v2.4.0) (2023-12-11)

### Bug Fixes

* modal checkout template markup ([#1608](#1608)) ([4d593db](4d593db))
* **modal-checkout:** align Stripe's "save payment" checkbox ([#1623](#1623)) ([69e0e42](69e0e42))
* **modal-checkout:** prevent initial render of details table ([#1601](#1601)) ([06d4ccd](06d4ccd))
* replace FILTER_SANITIZE_STRING ([6f805b0](6f805b0))

### Features

* improve performance of modal checkout ([#1607](#1607)) ([a48d190](a48d190))
* **modal-checkout:** add filter to cart item data ([#1590](#1590)) ([1e83dc1](1e83dc1))

### Performance Improvements

* **modal-checkout:** process checkout request earlier ([#1612](#1612)) ([5c58f5e](5c58f5e))
@matticbot
Copy link
Contributor

matticbot commented Dec 11, 2023

🎉 This PR is included in version 2.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miguelpeixe miguelpeixe miguelpeixe approved these changes

Assignees

No one assigned

Labels

released on @alpha released [Status] Approved

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@adekbadek @matticbot @miguelpeixe