-
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
Allow heading and button in Pattern Overrides #57789
Allow heading and button in Pattern Overrides #57789
Conversation
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/block-supports/pattern.php |
Size Change: -1.33 kB (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
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.
Looks close, but I think the button url will also need to be supported
global $block_bindings_allowed_blocks; | ||
$pattern_support = array_key_exists( $block_type->name, $block_bindings_allowed_blocks ); |
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.
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.
Now it fails in trunk
Uncaught TypeError: array_key_exists(): Argument #2 ($array) must be of type array, null given in /plugins/gutenberg/lib/block-supports/pattern.php:17
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.
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.
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.
No worries @michalczaplinski! Thanks for approving and merging that PR, good to get it across the line!
packages/patterns/src/constants.js
Outdated
@@ -23,4 +23,6 @@ export const PATTERN_SYNC_TYPES = { | |||
// TODO: This should not be hardcoded. Maybe there should be a config and/or an UI. | |||
export const PARTIAL_SYNCING_SUPPORTED_BLOCKS = { | |||
'core/paragraph': { content: __( 'Content' ) }, | |||
'core/heading': { content: __( 'Content' ) }, | |||
'core/button': { text: __( 'Text' ) }, |
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.
Should it also support the URL? It is editable in the UI, but doesn't work at the moment, possibly because it's missing here.
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.
Same with the various URL connected attributes such as rel, opening in a new tab etc.
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.
Yeah, I was thinking about that too. It's something that block bindings would need to support as well:
https://github.com/WordPress/gutenberg/pull/57742/files#diff-13ba237ac22cd8b66e7ec6447ae508cd6385d2e6e1e4cd1ae8f73c4bdf62136fR33
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.
If I am not mistaken, the URL of the button should work as well. It was included in the first pull request: link. Adding most attributes should be as easy as adding them to the relevant array, so we can try to add anything you consider relevant.
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.
Great, let's try adding them in a follow-up PR.
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.
url
support was added in 585a658!
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.
Works perfectly ⭐
What?
Part of #53705. Allow
core/heading
andcore/button
in Pattern Overrides.Why?
Allow more blocks to be overridable.
How?
Add them to the hardcoded allowed list.
Testing Instructions
Follow the instructions in #53705 (comment), but also add some headings and buttons.
Screenshots or screencast
TBD