[Experiment] Patterns: Allow the heading block and the button block to be partially synced #56704
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevin940726
added
[Feature] Synced Patterns
|
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/experimental/blocks.php |
kevin940726
force-pushed
the
experiment/support-heading-and-button-for-partial-syncing
branch
from
fce4394
to
5924426
Compare
|
Size Change: -13.5 kB (-1%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
talldan
reviewed
Dec 14, 2023
| 'tag_name' => $block_type->attributes[ $attribute_name ]['selector'], | ||
| ) | ||
| ); | ||
| $selectors = explode( ',', $block_type->attributes[ $attribute_name ]['selector'] ); |
There was a problem hiding this comment.
It'd be good to add a comment above this saying something like:
// Support comma-separated selectors by exploding them into an array.
I guess it's obvious in the context of this PR given it mentions the various heading block tags, but it might not be so obvious if you're just looking at this file on its own wondering why it works this way.
| $selectors = explode( ',', $block_type->attributes[ $attribute_name ]['selector'] ); | ||
| $found = false; | ||
| while ( ! $found && count( $selectors ) > 0 ) { | ||
| $tags = new WP_HTML_Tag_Processor( $block_content ); |
There was a problem hiding this comment.
Similarly, it might be worth a comment saying something like:
// Creating a new tag processor on every loop resets the search to the start.
Comment on lines
+103
to
+104
| // - Heading: content. | ||
| // - Button: text. |
There was a problem hiding this comment.
Not really sure these comments are necessary, seeing how the same thing is listed below in an easy to read format. 😄
| // - Image: url. | ||
| $blocks_attributes_allowlist = array( | ||
| 'core/paragraph' => array( 'content' ), | ||
| 'core/heading' => array( 'content' ), | ||
| 'core/button' => array( 'text' ), |
There was a problem hiding this comment.
Unfortunately, button blocks don't seem to be working correctly. When they have an override they're missing the <div class="wp-block-button"> wrapper.
It's because the code below does this:
$tag_name = $tags->get_tag();
$markup = "<$tag_name>$custom_value</$tag_name>";
$updated_tags = new WP_HTML_Tag_Processor( $markup );
$updated_tags->next_tag();
return $updated_tags->get_updated_html();So it considers the updated html of the block to just be the tag that it's updating and disregards any markup before or after that.
Instead it needs to update the $original_tags and get_updated_html from that, but I don't think it's supported yet 🤔
It is mentioned in the block bindings tracking issue - #54536.
Support to mutate HTML to change the inner content of an element. Until we have a set_inner_markup function, we can manually mutate the HTML for our experiments.
I guess we need to drop button support until then.
Image also doesn't work, as the code only updates tag content, not attribute values, so you end up with <image>$url</image> and it would also strip the image block <figure> wrapper, so I think that should be removed too.
There was a problem hiding this comment.
@SantosGuillamot opened #57249 that updates the PHP implementation for block bindings that should fix the handling for the Heading and Button blocks. It was extracted from the full prototype for Custom Fields: #56867.
|
Ah, I see there's a proposed updated version of the code in #56867 |
Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
dmsnell
reviewed
Dec 23, 2023
| // TODO: In the future, to support connecting more block attributes, | ||
| // we should build the full query from CSS selector. | ||
| $found = $tags->next_tag( trim( array_shift( $selectors ) ) ); | ||
| } |
There was a problem hiding this comment.
we could invert this loop and avoid the confusion @talldan wants to clarify, while improving performance.
- iterate through all the tags until
- we find a tag matching one of the selectors
this assumes that there are likely more tags than selectors, many more, and so the cost of checking each selector at each tag will be cheaper than checking every tag at each selector. additionally, we should make it pretty explicit here in a comment that the only selectors we currently support are tag names.
$selectors = array();
foreach ( explode( ',', $block_type->attributes[ $attribute_name ]['selector'] ) as $tag_name ) {
$selectors[] = trim( strtoupper( $tag_name ) );
}
$tags = new WP_HTML_Tag_Processor( $block_content );
while ( $tags->next_tag() ) {
$tag_name = $tags->get_tag();
if ( ! in_array( $tag_name, $selectors, true ) ) {
continue;
}
$markup = "<$tag_name>$custom_value</$tag_name>";
$updated_tags = new WP_HTML_Tag_Processor( $markup );
$updated_tags->next_tag();
// Get all the attributes from the original block and add them to the new markup.
$names = $tags->get_attribute_names_with_prefix( '' );
foreach ( $names as $name ) {
$updated_tags->set_attribute( $name, $tags->get_attribute( $name ) );
}
return $updated_tags->get_updated_html();
}
return $block_content;
|
This should be superseded by #57249. |
gziolo
deleted the
experiment/support-heading-and-button-for-partial-syncing
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Based on #56235. Allow the
core/headingblock and thecore/buttonblock to be partially synced.Why?
To have more blocks to test the feature.
How?
I had to refactor a part of the block connections code to support
,-separated attributes' selectors. It'd be nice to get a sanity check that it doesn't break or block the work on the block connections side. (c.c. @SantosGuillamot)Note that the solution here is likely just a temporary solution. Ideally, we might want to support arbitrary CSS selectors in the
WP_HTML_Tag_ProcessorAPI.Testing Instructions
Screenshots or screencast
Kapture.2023-12-01.at.11.56.36.mp4