-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[ETK][GB 13.x] Fix loading of A8C-specific patterns in WPCOM #62745
Conversation
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
736786e
to
ba2c624
Compare
apps/editing-toolkit/editing-toolkit-plugin/full-site-editing-plugin.php
Show resolved
Hide resolved
This PR modifies the release build for editing-toolkit To test your changes on WordPress.com, run To deploy your changes after merging, see the documentation: PCYsg-mMA-p2 |
apps/editing-toolkit/editing-toolkit-plugin/full-site-editing-plugin.php
Show resolved
Hide resolved
ba2c624
to
493e692
Compare
apps/editing-toolkit/editing-toolkit-plugin/full-site-editing-plugin.php
Outdated
Show resolved
Hide resolved
apps/editing-toolkit/editing-toolkit-plugin/block-patterns/class-block-patterns-from-api.php
Outdated
Show resolved
Hide resolved
apps/editing-toolkit/editing-toolkit-plugin/full-site-editing-plugin.php
Outdated
Show resolved
Hide resolved
3f16eb2
to
06f8501
Compare
I moved the unit tests to the existing |
… all of them before the new API rest requests
ab1e4cc
to
835aa48
Compare
I did an extensive amount of smoke testing on the following types of sites:
And I think this is good to go! The main thing I added today was backwards compatibility with older Gutenberg versions. This is important because Atomic users are able to disable the Gutenberg plugin, so we cannot rely on the new approach being available until it's in core WordPress. (This is not really a concern for simple sites, but the same code runs on both Atomic and Simple, so we have to add it anyways.) I did that by checking |
'rest_dispatch_request', | ||
register_patterns_on_api_request( | ||
function () { | ||
require_once __DIR__ . '/block-patterns/class-block-patterns-from-api.php'; |
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.
👍🏻
Changes proposed in this Pull Request
For full context, see: p1649463981925889-slack-C7YPUHBB2.
GB 13.0.0 loads block patterns asynchronously through a new endpoint introduced in #. This is not compatible with the
current_screen
hook for two reasons: execution order (which means we can't use that hook anymore) and the fact that even if we use another hook/filter, the data about the current screen is not available (though we are discussing ways to provide that data, here). The workaround for now (for shipping GB 13.0.0 to WPCOM) is to just load all patterns regardless of the screen (site editor or regular editor).Testing instructions
gutenberg-edge
sticker to itpublic-api.wordpress.com
yarn dev --sync
apps/editing-toolkit
) in Calypso and run tests:yarn run test:php
. All tests should pass.Peek.2022-04-13.21-48.mp4
TODO