Skip to content
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

Closed
wants to merge 1 commit into from

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Nov 2, 2023

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

  1. Create a Pattern (Edit Site > Patterns > Use the '+' button to create a synced pattern
  2. Add a heading and a paragraph to the pattern, and give them some default text
  3. For both the heading and the paragraph, use the block inspector to change the heading and paragraph connections to a source of 'Pattern attributes' and add a unique value for the 'key' (using preferably kebab case or camel case).
  4. Save the pattern
  5. Switch to editing a post and insert the pattern twice
  6. Update the text in the two instances of the patterns, and note that they can be made distinct, unlike synced patterns that don't have connections.
  7. Preview the post, your changes should be reflected in the frontend.

Testing Instructions for Keyboard

Screenshots or screencast

Kapture.2023-10-20.at.14.18.11.mp4

@talldan talldan added [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Block] Pattern Affects the Patterns Block labels Nov 2, 2023
@talldan talldan changed the title Try using the block connections api POC: Implement partially synced patterns using the block bindings API Nov 2, 2023
@talldan talldan added [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible [Type] Experimental Experimental feature or API. labels Nov 2, 2023
@gziolo gziolo self-requested a review November 2, 2023 13:47
@gziolo gziolo mentioned this pull request Nov 2, 2023
49 tasks
Copy link
Member

@gziolo gziolo left a 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",
Copy link
Member

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.

Copy link
Member

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'];
Copy link
Member

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 ) {
Copy link
Member

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 );
Copy link
Member

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": {
Copy link
Member

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 );
Copy link
Member

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.

Copy link
Member

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(
Copy link
Member

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 ) {
Copy link
Member

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.

Copy link
Contributor

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;
Copy link
Member

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.

@kevin940726
Copy link
Member

For anyone following this thread, I created another draft PR (#56235) exploring another approach which I believe could be more backward-compatible.

@talldan
Copy link
Contributor Author

talldan commented Dec 11, 2023

Implemented by #56235

@talldan talldan closed this Dec 11, 2023
@talldan talldan deleted the try/partial-syncing-block-connection branch December 11, 2023 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Pattern Affects the Patterns Block [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Type] Experimental Experimental feature or API. [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants