-
Notifications
You must be signed in to change notification settings - Fork 4.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
Pattern overrides: Update overrides attribute data structure and rename it to content
#58596
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core SVNIf you're a Core Committer, use this list when committing to
GitHub Merge commitsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/compat/wordpress-6.5/block-bindings/sources/pattern.php |
content
content
content
Size Change: +81 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
const newAttributes = { ...block.attributes }; | ||
for ( const attributeKey of attributes ) { | ||
defaultValues[ blockId ] ??= {}; | ||
defaultValues[ blockId ][ attributeKey ] = | ||
defaultValues[ blockId ] ??= { values: {} }; |
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 don't think the default values object has to be the same format, it just has to save the initial default values of these attributes.
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.
This tested well for me:
✅ Pattern instances that have not been migrated show correct override content on the frontend
✅ When loaded in the editor attributes we migrated to correctly to new content.values
property and continued to be overridable in the editor
…attribute to `content`
…te and nested `values` property
…e back compat) and use new values property in the block bindings callback for overrides
(this caused a hard to find bug somewhere) This reverts commit f243630.
Co-authored-by: Kai Hao <kevin830726@gmail.com>
caaa92d
to
9da5905
Compare
🤦 Forgot to add the props to the merge commit message, sorry folks. |
Here's the backport PR - Update pattern overrides handler to retrieve attribute value from new values property |
What?
Part of #53705
Updates the pattern block's
overrides
attribute data structure and renames it tocontent
.This branch intends to maintain full backwards compatible with the previous
overrides
attribute.The old attribute data structure:
The new attribute data structure:
Why?
This new format should prove more flexible for the future.
It allows storing additional metadata for each block, like the block name. (see the discussion in the tracking issue for more information)
It also supports the possibility of moving to different formats in the future, like the patch format that was explored but ultimately reverted - Use a patch format and support linkTarget of core/button for Pattern Overrides.
How?
Testing Instructions
trunk