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

Block Bindings API: Refactor logic into Block Bindings class and singleton pattern #57742

Merged
merged 16 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions lib/experimental/block-bindings/block-bindings.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php
/**
* Block Bindings API
*
* This file contains functions for managing block bindings in WordPress.
*
* @since 17.6.0
* @package gutenberg
*/

/**
* Retrieves the singleton instance of WP_Block_Bindings.
*
* @return WP_Block_Bindings The WP_Block_Bindings instance.
*/
if ( ! function_exists( 'wp_block_bindings' ) ) {
function wp_block_bindings() {
static $instance = null;
if ( is_null( $instance ) ) {
$instance = new WP_Block_Bindings();
}
return $instance;
}
}

/**
* Registers a new source for block bindings.
*
* @param string $source_name The name of the source.
* @param string $label The label of the source.
* @param callable $apply The callback executed when the source is processed during block rendering. The callable should have the following signature:
* function (object $source_attrs, object $block_instance, string $attribute_name): string
* - object $source_attrs: Object containing source ID used to look up the override value, i.e. {"value": "{ID}"}.
* - object $block_instance: The block instance.
* - string $attribute_name: The name of an attribute used to retrieve an override value from the block context.
* The callable should return a string that will be used to override the block's original value.
* @return void
*/
if ( ! function_exists( 'wp_block_bindings_register_source' ) ) {
function wp_block_bindings_register_source( $source_name, $label, $apply ) {
wp_block_bindings()->register_source( $source_name, $label, $apply );
}
}

/**
* Retrieves the list of allowed blocks.
*
* @return array The list of allowed blocks.
*/
if ( ! function_exists( 'wp_block_bindings_get_allowed_blocks' ) ) {
function wp_block_bindings_get_allowed_blocks() {
return wp_block_bindings()->get_allowed_blocks();
}
}

/**
* Retrieves the list of registered block sources.
*
* @return array The list of registered block sources.
*/
if ( ! function_exists( 'wp_block_bindings_get_sources' ) ) {
function wp_block_bindings_get_sources() {
return wp_block_bindings()->get_sources();
}
}

/**
* Replaces the HTML content of a block based on the provided source value.
*
* @param string $block_content Block Content.
* @param string $block_name The name of the block to process.
* @param string $block_attr The attribute of the block we want to process.
* @param string $source_value The value used to replace the HTML.
* @return string The modified block content.
*/
if ( ! function_exists( 'wp_block_bindings_replace_html' ) ) {
function wp_block_bindings_replace_html( $block_content, $block_name, $block_attr, $source_value ) {
return wp_block_bindings()->replace_html( $block_content, $block_name, $block_attr, $source_value );
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,59 @@
<?php
/**
* Define the mechanism to replace the HTML depending on the block attributes.
* Block Bindings API: WP_Block_Bindings class.
*
* Support for custom fields in blocks.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We should update this since Block Bindings are not only about custom fields
(e.g. Partially Synced Patterns).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I updated the description here: aa566cc

How does that look?

*
* @since 17.6.0
* @package gutenberg
*/

if ( ! function_exists( 'block_bindings_replace_html' ) ) {
if ( class_exists( 'WP_Block_Bindings' ) ) {
return;
}

/**
* Core class used to define supported blocks, register sources, and populate HTML with content from those sources.
*/
class WP_Block_Bindings {

/**
* Holds the registered block bindings sources, keyed by source identifier.
*
* @var array
*/
private $sources = array();

// Allowed blocks that support block bindings.
// TODO: Look for a mechanism to opt-in for this. Maybe adding a property to block attributes?
private $allowed_blocks = array(
'core/paragraph' => array( 'content' ),
'core/heading' => array( 'content' ),
'core/image' => array( 'url', 'title', 'alt' ),
'core/button' => array( 'url', 'text' ),
);

/**
* Function to register a new source.
*
* @param string $source_name The name of the source.
* @param string $label The label of the source.
* @param callable $apply The callback executed when the source is processed during block rendering. The callable should have the following signature:
* function (object $source_attrs, object $block_instance, string $attribute_name): string
* - object $source_attrs: Object containing source ID used to look up the override value, i.e. {"value": "{ID}"}.
* - object $block_instance: The block instance.
* - string $attribute_name: The name of an attribute used to retrieve an override value from the block context.
* The callable should return a string that will be used to override the block's original value.
*
* @return void
*/
public function register_source( $source_name, $label, $apply ) {
$this->sources[ $source_name ] = array(
'label' => $label,
'apply' => $apply,
);
}

/**
* Depending on the block attributes, replace the proper HTML based on the value returned by the source.
*
Expand All @@ -14,7 +62,7 @@
* @param string $block_attr The attribute of the block we want to process.
* @param string $source_value The value used to replace the HTML.
*/
function block_bindings_replace_html( $block_content, $block_name, $block_attr, $source_value ) {
public function replace_html( $block_content, $block_name, $block_attr, $source_value ) {
$block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block_name );
if ( null === $block_type ) {
return;
Expand Down Expand Up @@ -107,4 +155,22 @@ function block_bindings_replace_html( $block_content, $block_name, $block_attr,
}
return;
}

/**
* Retrieves the list of registered block sources.
*
* @return array The array of registered sources.
*/
public function get_sources() {
return $this->sources;
}

/**
* Retrieves the list of allowed blocks that support block bindings.
*
* @return array The array of allowed blocks.
*/
public function get_allowed_blocks() {
return $this->allowed_blocks;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thinking about a future where this mechanism may change (it becomes an opt-in or opt-out, or we support so many core blocks that it becomes easier to list the ones that aren't supported), and ways that the method could be future proofed.

Maybe changing it to something like get_is_block_type_supported( $block_type ) could be a way to future proof it. That would prevent leaking the allowed_blocks array into public code and allow us to change the way it's implemented while keeping the interface the same.

Copy link
Member

Choose a reason for hiding this comment

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

I definitely agree with the rationale. What do you think about going even further and making it an implementation detail of the function that takes care of computing the value of the attributes that have a corresponding custom source binding? If there are no attribute changes then there is nothing much to process in the block content's HTML.

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 what Dan suggests makes total sense. The allowed_blocks property is already private so if we use a function like get_is_block_type_supported( $block_type ) instead it should be enough to "future proof" it.

I'm not sure I follow your suggestion @gziolo though. Could you elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we go with this, we should also update the wrapper function in:

if ( ! function_exists( 'wp_block_bindings_get_allowed_blocks' ) ) {
function wp_block_bindings_get_allowed_blocks() {
return wp_block_bindings()->get_allowed_blocks();
}
}

Copy link
Member

@gziolo gziolo Jan 15, 2024

Choose a reason for hiding this comment

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

What I meant is that with more refactoring, you can make the list of allowed blocks and implementation details of process_block_bindings. The only other place where the same list is currently needed is:

foreach ( $block_bindings_allowed_blocks as $block_name => $attributes ) {
	add_filter( 'render_block_' . $block_name, 'process_block_bindings', 20, 3 );
}

However, I don't see any reason why it couldn't become:

add_filter( 'render_block', 'process_block_bindings', 20, 3 );

The same check would happen in the filter instead before it starts any processing:

function process_block_bindings( $block_content, $attributes, $block_instance ) {
	$allowed_blocks = array(
		'core/paragraph' => array( 'content' ),
		'core/heading'   => array( 'content' ),
		'core/image'     => array( 'url', 'title', 'alt' ),
		'core/button'    => array( 'url', 'text' ),
	);
 	if ( empty( $attributes['metadata']['bindings'] ) ||  ! isset(  $allowed_blocks[ $block_instance->name ] ) ) {
 		// stop processing
	}
 	// continue processing
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks Greg! Yeah, that makes sense to me. 👍

There's no need to expose wp_block_bindings_get_allowed_blocks() as public API so we can remove it as well as remove the internal $this->get_allowed_blocks() method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I moved the $allowed_blocks property to the filter function and removed the loop 👍
Perhaps the motivation of the original code was to prevent running the filter function on every block on the page?

In any case, I think what I've revised here addresses the issue. Let me know what you think!

}
5 changes: 2 additions & 3 deletions lib/experimental/block-bindings/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
* @package gutenberg
*/

require_once __DIR__ . '/sources/index.php';
require_once __DIR__ . '/html-processing.php';

require_once __DIR__ . '/class-wp-block-bindings.php';
require_once __DIR__ . '/block-bindings.php';
// Register the sources.
$gutenberg_experiments = get_option( 'gutenberg-experiments' );
if ( $gutenberg_experiments ) {
Expand Down
32 changes: 0 additions & 32 deletions lib/experimental/block-bindings/sources/index.php

This file was deleted.

5 changes: 2 additions & 3 deletions lib/experimental/block-bindings/sources/pattern.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@
*
* @package gutenberg
*/

if ( function_exists( 'register_block_bindings_source' ) ) {
if ( function_exists( 'wp_block_bindings_register_source' ) ) {
$pattern_source_callback = function ( $source_attrs, $block_instance, $attribute_name ) {
if ( ! _wp_array_get( $block_instance->attributes, array( 'metadata', 'id' ), false ) ) {
return null;
}
$block_id = $block_instance->attributes['metadata']['id'];
return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $block_id, $attribute_name ), null );
};
register_block_bindings_source(
wp_block_bindings_register_source(
'pattern_attributes',
__( 'Pattern Attributes', 'gutenberg' ),
$pattern_source_callback
Expand Down
5 changes: 2 additions & 3 deletions lib/experimental/block-bindings/sources/post-meta.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
*
* @package gutenberg
*/

if ( function_exists( 'register_block_bindings_source' ) ) {
if ( function_exists( 'wp_block_bindings_register_source' ) ) {
$post_meta_source_callback = function ( $source_attrs ) {
// Use the postId attribute if available
if ( isset( $source_attrs['postId'] ) ) {
Expand All @@ -17,7 +16,7 @@

return get_post_meta( $post_id, $source_attrs['value'], true );
};
register_block_bindings_source(
wp_block_bindings_register_source(
'post_meta',
__( 'Post Meta', 'gutenberg' ),
$post_meta_source_callback
Expand Down
20 changes: 6 additions & 14 deletions lib/experimental/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,7 @@ function wp_enqueue_block_view_script( $block_name, $args ) {
) ) {

require_once __DIR__ . '/block-bindings/index.php';
// Allowed blocks that support block bindings.
// TODO: Look for a mechanism to opt-in for this. Maybe adding a property to block attributes?
global $block_bindings_allowed_blocks;
$block_bindings_allowed_blocks = array(
'core/paragraph' => array( 'content' ),
'core/heading' => array( 'content' ),
'core/image' => array( 'url', 'title', 'alt' ),
'core/button' => array( 'url', 'text' ),
);

if ( ! function_exists( 'process_block_bindings' ) ) {
/**
* Process the block bindings attribute.
Expand Down Expand Up @@ -128,9 +120,9 @@ function process_block_bindings( $block_content, $block, $block_instance ) {
// }
// }
//
global $block_bindings_allowed_blocks;
global $block_bindings_sources;
$modified_block_content = $block_content;
$block_bindings_allowed_blocks = wp_block_bindings_get_allowed_blocks();
$block_bindings_sources = wp_block_bindings_get_sources();
$modified_block_content = $block_content;
foreach ( $block['attrs']['metadata']['bindings'] as $binding_attribute => $binding_source ) {
// If the block is not in the list, stop processing.
if ( ! isset( $block_bindings_allowed_blocks[ $block['blockName'] ] ) ) {
Expand Down Expand Up @@ -159,13 +151,13 @@ function process_block_bindings( $block_content, $block, $block_instance ) {
}

// Process the HTML based on the block and the attribute.
$modified_block_content = block_bindings_replace_html( $modified_block_content, $block['blockName'], $binding_attribute, $source_value );
$modified_block_content = wp_block_bindings_replace_html( $modified_block_content, $block['blockName'], $binding_attribute, $source_value );
}
return $modified_block_content;
}

// Add filter only to the blocks in the list.
foreach ( $block_bindings_allowed_blocks as $block_name => $attributes ) {
foreach ( wp_block_bindings_get_allowed_blocks() as $block_name => $attributes ) {
add_filter( 'render_block_' . $block_name, 'process_block_bindings', 20, 3 );
}
}
Expand Down
Loading