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

Refactor block binding processing and attribute computation #6059

41 changes: 26 additions & 15 deletions src/wp-includes/class-wp-block.php
Original file line number Diff line number Diff line change
Expand Up @@ -192,15 +192,15 @@ public function __get( $name ) {
}

/**
* Processes the block bindings in block's attributes.
* Processes the block bindings and updates the block attributes with the values from the sources.
*
* A block might contain bindings in its attributes. Bindings are mappings
* between an attribute of the block and a source. A "source" is a function
* registered with `register_block_bindings_source()` that defines how to
* retrieve a value from outside the block, e.g. from post meta.
*
* This function will process those bindings and replace the HTML with the value of the binding.
* The value is retrieved from the source of the binding.
* This function will process those bindings and update the block's attributes
* with the values coming from the bindings.
*
* ### Example
*
Expand Down Expand Up @@ -228,13 +228,13 @@ public function __get( $name ) {
*
* @since 6.5.0
*
* @param string $block_content Block content.
* @param array $block The full block, including name and attributes.
* @return string The modified block content.
* @return array The computed block attributes for the provided block bindings.
*/
private function process_block_bindings( $block_content ) {
private function process_block_bindings() {
$parsed_block = $this->parsed_block;

$computed_attributes = array();

// Allowed blocks that support block bindings.
// TODO: Look for a mechanism to opt-in for this. Maybe adding a property to block attributes?
$allowed_blocks = array(
Expand All @@ -251,10 +251,9 @@ private function process_block_bindings( $block_content ) {
empty( $parsed_block['attrs']['metadata']['bindings'] ) ||
! is_array( $parsed_block['attrs']['metadata']['bindings'] )
) {
return $block_content;
return $computed_attributes;
}

$modified_block_content = $block_content;
foreach ( $parsed_block['attrs']['metadata']['bindings'] as $attribute_name => $block_binding ) {
// If the attribute is not in the allowed list, process next attribute.
if ( ! in_array( $attribute_name, $allowed_blocks[ $this->name ], true ) ) {
Expand All @@ -275,11 +274,18 @@ private function process_block_bindings( $block_content ) {

// If the value is not null, process the HTML based on the block and the attribute.
if ( ! is_null( $source_value ) ) {
$modified_block_content = $this->replace_html( $modified_block_content, $attribute_name, $source_value );
$computed_attributes[ $attribute_name ] = $source_value;
}
}

return $modified_block_content;
// If there are computed attributes, update the attributes with the
// computed ones.
if ( ! empty( $computed_attributes ) ) {
// Merge the computed attributes with the original attributes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing end period.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gziolo ☝️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that myself, too. 🙈 I'm working on another commit and I plan to include it. Thank you as always for your attention to detail and checking new commits 🙇🏻

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in acad00e.

$this->attributes = array_merge( $this->attributes, $computed_attributes );
}

return $computed_attributes;
}

/**
Expand Down Expand Up @@ -410,6 +416,9 @@ public function render( $options = array() ) {
)
);

// Process the block bindings and get attributes updated with the values from the sources.
$computed_attributes = $this->process_block_bindings();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm comtemplating on my feedback from yesterday and started thinking whether to move back here the changes to update $this->attributes as it now unclear why this call is here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that it's a little bit better to keep that type of side-effect
directly in the render() and not hide it in the method, but it's not a strong opinion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you. Thank you for trying the alternative approach 🙇


$is_dynamic = $options['dynamic'] && $this->name && null !== $this->block_type && $this->block_type->is_dynamic();
$block_content = '';

Expand Down Expand Up @@ -445,6 +454,12 @@ public function render( $options = array() ) {
}
}

if ( ! empty( $computed_attributes ) && ! empty( $block_content ) ) {
foreach ( $computed_attributes as $attribute_name => $source_value ) {
$block_content = $this->replace_html( $block_content, $attribute_name, $source_value );
}
}

if ( $is_dynamic ) {
$global_post = $post;
$parent = WP_Block_Supports::$block_to_render;
Expand Down Expand Up @@ -488,10 +503,6 @@ public function render( $options = array() ) {
}
}

// Process the block bindings for this block, if any are registered. This
// will replace the block content with the value from a registered binding source.
$block_content = $this->process_block_bindings( $block_content );

/**
* Filters the content of a single block.
*
Expand Down
40 changes: 40 additions & 0 deletions tests/phpunit/tests/block-bindings/render.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public function test_update_block_with_value_from_source() {
$result = $block->render();

$this->assertEquals( $expected, $result, 'The block content should be updated with the value returned by the source.' );
$this->assertSame( 'test source value', $block->attributes['content'] );
}

/**
Expand Down Expand Up @@ -97,5 +98,44 @@ public function test_passing_arguments_to_source() {
$result = $block->render();

$this->assertEquals( $expected, $result, 'The block content should be updated with the value returned by the source.' );
$this->assertSame( "The attribute name is 'content' and its binding has argument 'key' with value '$value'.", $block->attributes['content'] );
}


/**
* Tests if the block content is updated with the value returned by the source
* for the Image block in the placeholder state.
*
* @ticket 60282
*
* @covers ::register_block_bindings_source
*/
public function test_update_block_with_value_from_source_imag_placeholder() {
$get_value_callback = function () {
return 'https://example.com/image.jpg';
};

register_block_bindings_source(
self::SOURCE_NAME,
array(
'label' => self::SOURCE_LABEL,
'get_value_callback' => $get_value_callback,
)
);

$block_content = <<<HTML
<!-- wp:image {"metadata":{"bindings":{"url":{"source":"test/source"}}}} -->
<figure class="wp-block-image"><img alt=""/></figure>
<!-- /wp:image -->
HTML;

$parsed_blocks = parse_blocks( $block_content );
$block = new WP_Block( $parsed_blocks[0] );

$expected = '<figure class="wp-block-image"><img src="https://example.com/image.jpg" alt=""/></figure>';
$result = $block->render();

$this->assertSame( 'https://example.com/image.jpg', $block->attributes['url'] );
$this->assertEquals( $expected, trim( $result ), 'The block content should be updated with the value returned by the source.' );
}
}
Loading