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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
236b4e6
Move bindings logic to Block Bindings class
artemiomorales Jan 12, 2024
6e1eb1c
Remove erroneous echo statement that was breaking UI
artemiomorales Jan 10, 2024
37e8dbd
Fix error in registering block bindings sources
artemiomorales Jan 11, 2024
27cfb49
Add missing return statement
artemiomorales Jan 12, 2024
892aa17
Add docblocks
artemiomorales Jan 12, 2024
1319d1e
Remove obsolete file
artemiomorales Jan 12, 2024
c0e4407
Fix docblock
artemiomorales Jan 12, 2024
d596b23
Sync register_source docblock
artemiomorales Jan 12, 2024
e607d95
Remove erroneous subpackage declaration
artemiomorales Jan 12, 2024
0a317f1
Update package name
artemiomorales Jan 12, 2024
aafcc0f
Fix gutenberg package declarations
artemiomorales Jan 12, 2024
7252c01
Remove extraneous comments
artemiomorales Jan 12, 2024
d7e1c93
Move allowed_blocks property to filter function
artemiomorales Jan 15, 2024
aa566cc
Update description of Block Bindings class
artemiomorales Jan 15, 2024
9c8bf55
Address PHPCS spacing issue
artemiomorales Jan 15, 2024
e6473a7
Rename function call and call using string in filter
artemiomorales Jan 16, 2024
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
43 changes: 43 additions & 0 deletions lib/experimental/block-bindings/block-bindings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
/**
* Block Bindings API
*
* @since 6.5.0
*
* @package WordPress
* @subpackage Script Modules
*/

if ( ! function_exists( 'wp_block_bindings' ) ) {
function wp_block_bindings() {
static $instance = null;
if ( is_null( $instance ) ) {
$instance = new WP_Block_Bindings();
}
return $instance;
}
}

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 );
}
}

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

if ( ! function_exists( 'wp_block_bindings_get_sources' ) ) {
function wp_block_bindings_get_sources() {
return wp_block_bindings()->get_sources();
}
}

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 );
}
}
60 changes: 53 additions & 7 deletions ...mental/block-bindings/html-processing.php → ...lock-bindings/class-wp-block-bindings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,49 @@
<?php
/**
* Define the mechanism to replace the HTML depending on the block attributes.
*
* @package gutenberg
*/

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

class WP_Block_Bindings {

/**
* Holds the registered block bindings sources, keyed by source identifier.
*
* @since 6.5.0
* @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 +52,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 +145,12 @@ function block_bindings_replace_html( $block_content, $block_name, $block_attr,
}
return;
}

public function get_sources() {
return $this->sources;
}

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:

gutenberg/lib/experimental/block-bindings/block-bindings.php

Lines 50 to 54 in 7252c01

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 •
edited
Loading

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
View file
Open in desktop
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
5 changes: 2 additions & 3 deletions lib/experimental/block-bindings/sources/pattern.php
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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