-
Notifications
You must be signed in to change notification settings - Fork 384
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
Bump minimum required WP to 6.3 #7816
Conversation
52f98cb
to
43aa96f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7816 +/- ##
=============================================
- Coverage 77.21% 77.08% -0.14%
+ Complexity 6910 6777 -133
=============================================
Files 210 276 +66
Lines 21518 22303 +785
=============================================
+ Hits 16616 17193 +577
- Misses 4902 5110 +208
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Plugin builds for f508f83 are ready 🛎️!
Checksums
Warning These builds are for testing purposes only and should not be used in production. |
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.
So much goodness! Just a few areas where I have questions.
|
||
/** | ||
* The minimum version of WordPress supported. | ||
* | ||
* @var string | ||
*/ | ||
const WP_MIN_VERSION = '5.6'; | ||
const WP_MIN_VERSION = '6.3'; |
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.
Since the plugin as a whole requires WP 6.3 now, should the DependencySupport
class just be removed entirely now?
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've been thinking about it, and this class helps us manage service initialization effectively, especially if we need to support any dependencies in the future. I believe we should keep it to maintain standardization and be prepared for any future needs.
) { | ||
$this->enable_css_transient_caching(); | ||
} | ||
$this->enable_css_transient_caching(); |
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.
Since previously this was only called when updating from 1.5.0 or 1.5.1 or ir the block_uniq_transformer is relevant, then I wonder if this call is even relevant anymore? Should the entire handle_plugin_update
be removed instead?
// Replace `data-wp-on--click` or `data-wp-on-async--click` with AMP state on submenu open button. | ||
if ( false !== strpos( $new_block_content, 'wp-block-navigation__responsive-container-open' ) ) { | ||
$new_block_content = preg_replace( | ||
'/\sdata-wp-on--click="[^"]+"/', | ||
'/\sdata-wp-on-(?:-click|async--click)="[^"]+"/', | ||
sprintf( ' on="tap:AMP.setState({ %1$s: !%1$s })"', esc_attr( $modal_state_property ) ), | ||
$new_block_content | ||
); | ||
} | ||
|
||
// Replace `data-wp-on--click` with AMP state on submenu close button. | ||
// Replace `data-wp-on--click` or `data-wp-on-async--click` with AMP state on submenu close button. | ||
if ( false !== strpos( $new_block_content, 'wp-block-navigation__responsive-container-close' ) ) { | ||
$new_block_content = preg_replace( | ||
'/\sdata-wp-on--click="[^"]+"/', | ||
'/\sdata-wp-on-(?:-click|async--click)="[^"]+"/', |
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.
New changes in navigation block with interactivity API enabled.
Co-authored-by: Weston Ruter <[email protected]>
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.
Epic!
@westonruter There is a flaky end-to-end test that we can ignore for now; I'll address it separately. Currently, there's an issue where the AMP preview button in the post editor is sometimes not visible when the editor is first opened. It becomes visible only when the editor is in an editable state, such as when someone clicks on it. I can replicate this on WordPress 6.5, but not on the trunk version. amp-wp/assets/src/block-editor/plugins/wrapped-amp-preview-button.js Lines 59 to 61 in 4b94713
|
Verified it with WordPress v6.3,v6.5,v6.6RC2 and it is working fine. Checked the settings and related options those are working fine ✅ |
Summary
AMP_Block_Uniqid_Sanitizer
, andBlockUniqidTransformer
and their test case files.assets/src/settings-page/settings-footer.js
.amp_page_cache_good_response_time_threshold
filter which was deprecated in AMP 2.5.0 in favor of WP'ssite_status_good_response_time_threshold
.@wordpress/primitives
.Checklist