-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
POC: Implement partially synced patterns using the block bindings API #55807
Conversation
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.
Thank you so much for putting this working prototype together. I have the impression that it's very close to landing behind the existing experiments with the sub-registry that narrows down places where the store deals with the dynamic content for the Partially Synced Pattern. It definitely requires some testing and polishing, but conceptually, I'm pretty happy with how it all looks. I left some comments that might help to clarify how it finally might work.
@@ -14,7 +14,7 @@ | |||
"content": { | |||
"type": "string", | |||
"source": "html", | |||
"selector": "h1,h2,h3,h4,h5,h6", | |||
"selector": "h2", |
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 assume it was added for testing purposes and it will get reverted.
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.
Yep! The processor API doesn't yet support arbitrary CSS selector.
@@ -41,13 +41,31 @@ function render_block_core_block( $attributes ) { | |||
|
|||
$seen_refs[ $attributes['ref'] ] = true; | |||
|
|||
$filter_block_context = static function( $context ) use ( $attributes ) { | |||
if ( isset( $attributes['dynamicContent'] ) && $attributes['dynamicContent'] ) { | |||
$context['dynamicContent'] = $attributes['dynamicContent']; |
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.
For the block context, it would be nice to name it in a way that created a connection with the pattern, e.g., pattern/dynamicContent
.
@@ -41,13 +41,31 @@ function render_block_core_block( $attributes ) { | |||
|
|||
$seen_refs[ $attributes['ref'] ] = true; | |||
|
|||
$filter_block_context = static function( $context ) use ( $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.
The filtering only makes sense if $attributes['dynamicContent']
is provided, so maybe it should be created conditionally.
Should the filter get applied only around: $content = do_blocks( $content );
? It might help to reason where it takes an effect.
const updates = {}; | ||
for ( const clientId of [].concat( clientIds ) ) { | ||
const attrs = uniqueByBlock ? attributes[ clientId ] : attributes; | ||
const parentPattern = select.getBlock( patternClientId ); |
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.
Have you considered reading the clientId
of the pattern by searching for the first parent of the core/block
type? This way, it's going to be easier to move the logic outside of the subregistry. It can be done later if that is the best way in the current setup.
@@ -8,6 +8,9 @@ | |||
"keywords": [ "reusable" ], | |||
"textdomain": "default", | |||
"attributes": { | |||
"dynamicContent": { |
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 we plan to keep it as an experiment, then it should get injected from outside, similar to uses_context
for block types that have block bindings defined.
); | ||
|
||
function shimAttributeSource( settings ) { | ||
settings.edit = createEditFunctionWithPatternSource()( settings.edit ); |
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.
What's the role of this HOC?
There is a sub-registry injected in the core/block
edit implementation that should play a similar role. It essentially should allow redefining a function that fetches the attributes for the block.
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, it was simply the minimum effort for me to make the POC work at the time. We should revisit the "reading" side of the story.
@@ -55,7 +56,11 @@ const withInspectorControl = createHigherOrderComponent( ( BlockEdit ) => { | |||
|
|||
// Check if the current block is a paragraph or image block. | |||
// Currently, only these two blocks are supported. | |||
if ( ! [ 'core/paragraph', 'core/image' ].includes( props.name ) ) { | |||
if ( | |||
! [ 'core/paragraph', 'core/image', 'core/heading' ].includes( |
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.
We discussed enabling the Button block, too. It's fine to take care of it as a follow-up. I'm leaving it mostly as a note.
@@ -12,4 +12,7 @@ | |||
// if it doesn't, `get_post_meta()` will just return an empty string. | |||
return get_post_meta( $block_instance->context['postId'], $meta_field, true ); | |||
}, | |||
'pattern_attributes' => function ( $block_instance, $meta_field ) { |
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.
It would be great to get confirmation from @michalczaplinski or @SantosGuillamot on how this array should be shaped. There is name
key defined with the meta
value, which implies, we should have another entry with the name pattern
. I might be completely wrong 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.
I think that nothing was ultimately decided here! Just for reference, this is a previous discussion with Riad: #53247 (comment)
We'll need a nested array here OR have a one file per source (like in Riad's original prototype) so meta.php
, patterns.php
, etc.
$gutenberg_experiments = get_option( 'gutenberg-experiments' ); | ||
if ( $gutenberg_experiments && array_key_exists( 'gutenberg-connections', $gutenberg_experiments ) ) { | ||
function gutenberg_register_pattern_support( $block_type ) { | ||
$pattern_support = property_exists( $block_type, 'supports' ) ? _wp_array_get( $block_type->supports, array( '__experimentalConnections' ), false ) : false; |
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.
Noting, the injected context is only necessary when dealing with Partially Synced Patterns. The condition based on the UI elements seems to work fine here, but it isn't a direct relationship.
For anyone following this thread, I created another draft PR (#56235) exploring another approach which I believe could be more backward-compatible. |
Implemented by #56235 |
Props to @kevin940726 and @aaronrobertshaw for their work on this.
What?
See #53705
Implements partially synced patterns using some of the available parts of the block bindings API.
The PR is a technical prototype, with the user experience still requiring lots of development.
Why?
See more of the rationale for block bindings in #54536.
How?
This uses the system implemented in #53241 to declare connections (which will probably be renamed to bindings) on the blocks within a pattern (when the user is designing the pattern).
Users can insert these patterns into a post, and when editing a 'connected' attribute of a block, the values are saved to the pattern block instance in a
dynamicContent
attribute instead of to the block being edited. This allows multiple instances of the same synced pattern to have different content.The PR also opts partially synced patterns into using the HTML API to dynamically replace block values in PHP.
Testing Instructions
Watch the video for a walkthrough.
Prerequisite: Enable the 'Connections' experiment
Testing Instructions for Keyboard
Screenshots or screencast
Kapture.2023-10-20.at.14.18.11.mp4