-
Notifications
You must be signed in to change notification settings - Fork 843
Boost: Invalidate LCP Data on Environment Changes #43151
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
Conversation
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Boost plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 3 files.
1 file is newly checked for coverage.
Full summary · PHP report · JS report Coverage check overridden by
Coverage tests to be added later
|
42270de to
9194bd7
Compare
dilirity
left a comment
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.
I think this is ready for a merge, but considering I wrote the majority of the code, I'd like for someone else to review it as well.
| /** | ||
| * Handle when a critical CSS environment change is detected. | ||
| */ | ||
| public function handle_environment_change( $is_major_change, $change_type ) { |
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.
I noticed this was added in https://github.com/Automattic/jetpack/pull/41516/files but was unused even there.
LiamSarsfield
left a comment
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.
|
I tested everything but the cloud css test at the end and they all worked. I'll take a look at the cloud css part tomorrow morning. |
Resolves HOG-84:Invalidate LCP when theme is switched
Proposed changes:
LCP_Invalidatorclass to handle the invalidation (clearing) of stored LCP (Largest Contentful Paint) analysis data.after_switch_theme).jetpack_boost_deactivateaction).jetpack_boost_lcp_invalidatedwhich fires after LCP data has been cleared by the invalidator.Lcpclass to hook intojetpack_boost_lcp_invalidatedto restart the LCP analysis process after invalidation.jetpack_boost_critical_css_environment_changedaction in favor of the more generaljetpack_boost_environment_changed. The old action will still work but trigger a deprecated notice.Cloud_CSSclass to use the newjetpack_boost_environment_changedaction.Other information:
Jetpack product discussion
pc9hqz-3tr-p2
Does this pull request change what data or activity we track or use?
N/A
Testing instructions:
Prerequisites:
Jetpack Boost plugin installed and activated.
LCP Optimization feature enabled.
Click here for SQL to check LCP State Option and Storage Posts
Testing Theme Switch (Major Change):
Note the current LCP state option value and any existing LCP storage posts. Pay attention to the data, and note the
post_modified_gmttimestamps of the existing posts.Click here for SQL to check initial LCP State and Storage
Go to
Appearance->Themes.Activate a different theme. Note the approximate time of activation.
Wait a few seconds for the analysis to complete (it should be quick).
Re-check the
jetpack_boost_ds_lcp_stateoption in thewp_optionstable. Verify its value has been updated (it won't have a timestamp, but the content should reflect recent analysis).Re-check the
wp_poststable forjb_store_lcpposts. Verify that new posts now exist and theirpost_date_gmtorpost_modified_gmttimestamps are after the time you activated the theme.Click here for SQL to check updated LCP State and new Storage posts
Testing Plugin Deactivation:
Ensure there is some LCP data stored (check state option and storage posts).
Click here for SQL to check initial LCP State and Storage
Go to
Plugins->Installed Plugins.Deactivate the Jetpack Boost plugin.
Verify that the
jetpack_boost_ds_lcp_stateoption has been cleared or reset, and that alljb_store_lcpposts have been deleted/trashed.Click here for SQL to verify LCP State is cleared and Storage posts are deleted
Testing Deprecated Action:
Add the following PHP snippet to your site (e.g., in a custom plugin or your theme's
functions.php).Click here for PHP Snippet to test deprecated action
Ensure
WP_DEBUGandWP_DEBUG_LOGare enabled in yourwp-config.phpfile so that deprecation notices and the custom log message are written to/wp-content/debug.log.Trigger an environment change (e.g., switch themes via
Appearance->Themes, or save a published post/page).Check your
/wp-content/debug.logfile. Verify that:jetpack_boost_critical_css_environment_changed, indicating it was called and suggestingjetpack_boost_environment_changedinstead.Testing
Cloud_CSSUpdate:is_major_changeflag passed tojetpack_boost_environment_changed).