Make WordPress Core

Opened 7 months ago

Closed 7 months ago

Last modified 5 months ago

#63840 closed enhancement (fixed)

Block Bindings: Support generic block attributes

Reported by: bernhard-reiter's profile Bernhard Reiter Owned by: bernhard-reiter's profile Bernhard Reiter
Milestone: 6.9 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-unit-tests gutenberg-merge
Focuses: Cc:

Description

Currently, Block Bindings support for block attributes such as core/paragraph's content or core/button's text depends on hard-coded logic to locate and replace the respective attributes. Since that logic is not filterable, it means that extending support for additional Core or third-party blocks requires hand-writing similar code in the block's PHP. This is limiting the scalability of Block Bindings.

Thus, the existing block-specific custom logic should be replaced by more generic code that is able to locate and replace an attribute based on the selector definition in its block.json.

Ultimately, this will require a set_inner_html() method from the HTML API (which doesn't exist yet); but we can already provide a decent solution based on what's currently available from the HTML API.

Change History (17)

This ticket was mentioned in PR #9469 on WordPress/wordpress-develop by @Bernhard Reiter.


7 months ago
#1

  • Keywords has-patch has-unit-tests added

Work in progress.

Preparatory work for making it easier to have block bindings support more blocks and block attributes, see https://github.com/WordPress/gutenberg/pull/70642#issuecomment-3178740325.

Trac ticket: https://core.trac.wordpress.org/ticket/63840

@Bernhard Reiter commented on PR #9469:


7 months ago
#2

Here's a little experiment that builds on this PR: https://github.com/ockham/wordpress-develop/pull/5

@gziolo commented on PR #9469:


7 months ago
#3

This is awesome! Can we replicate the same logic in the Gutenberg plugin or does it require any code from HTML API in trunk?

I would be happy to explore how this could be integrated with https://github.com/WordPress/gutenberg/pull/70975.

@Bernhard Reiter commented on PR #9469:


7 months ago
#4

This is awesome! Can we replicate the same logic in the Gutenberg plugin or does it require any code from HTML API in trunk?

I don't think it relies on any new additions to the HTML API that are only present in trunk, so it _should_ be possible to backport this to GB. The main annoyance is probably that the block bindings logic is rather opaquely located in the WP_Block class, i.e. there are no "easy" filters to extend it for other blocks and attributes; so instead, we might have to duplicate a bunch of code in the block's render() method.

I would be happy to explore how this could be integrated with WordPress/gutenberg#70975.

👍

FWIW, non-conditional binding (i.e. replacement of an existing citation) works pretty much OOTB, simply by adding the attribute to BLOCK_BINDINGS_SUPPORTED_ATTRIBUTES:

<details>
<summary>Patch</summary>

  • src/wp-includes/class-wp-block.php

    diff --git a/src/wp-includes/class-wp-block.php b/src/wp-includes/class-wp-block.php
    index 173b0dbe80..902093fe3b 100644
    a b class WP_Block { 
    112112                'core/image'     => array( 'id', 'url', 'title', 'alt' ),
    113113                'core/button'    => array( 'url', 'text', 'linkTarget', 'rel' ),
    114114                'core/post-date' => array( 'datetime' ),
     115                'core/pullquote' => array( 'citation' ),
    115116        );
    116117
    117118        /**
  • tests/phpunit/tests/block-bindings/render.php

    diff --git a/tests/phpunit/tests/block-bindings/render.php b/tests/phpunit/tests/block-bindings/render.php
    index 60b970cf79..d2c38b7951 100644
    a b HTML 
    8383                                ,
    8484                                '<div class="wp-block-button"><a class="wp-block-button__link wp-element-button">test source value</a></div>',
    8585                        ),
     86                        'pullquote block'   => array(
     87                                'citation',
     88                                <<<HTML
     89
     90<figure class="wp-block-pullquote"><blockquote><p>This should not appear</p><cite>Quote McQuoteface</cite></blockquote></figure>
     91
     92HTML
     93                                ,
     94                                '<figure class="wp-block-pullquote"><blockquote><p>This should not appear</p><cite>test source value</cite></blockquote></figure>',
     95                        )
     96
    8697                );
    8798        }

</details>

@Bernhard Reiter commented on PR #9469:


7 months ago
#5

Here's a minor enhancement to the tests that will currently break with the implementation of WP_Block_Bindings_Processor found in this PR:

  • tests/phpunit/tests/block-bindings/wpBlockBindingsProcessor.php

    diff --git a/tests/phpunit/tests/block-bindings/wpBlockBindingsProcessor.php b/tests/phpunit/tests/block-bindings/wpBlockBindingsProcessor.php
    index 5a8cf11095..d7f20099c0 100644
    a b class Tests_Blocks_wpBlockBindingsProcessor extends WP_UnitTestCase { 
    7979               $this->assertTrue( $processor->replace_rich_text( '<strong>New</strong> image caption' ) );
    8080
    8181               $processor->seek( 'image' );
     82               $processor->add_class( 'extra-img-class' );
    8283
    8384               $this->assertEquals(
    8485                       $figure_opener .
    85                        $img .
     86                       '[[Image(breakfast.jpg)]]' .
    8687                       '<figcaption class="wp-element-caption"><strong>New</strong> image caption</figcaption>' .
    8788                       $figure_closer,
    8889                       $processor->build()

The test passes however with the alternative implementation that's based on Jon's suggestion.

@Bernhard Reiter commented on PR #9469:


7 months ago
#6

I've now merged @sirreal's PR https://github.com/ockham/wordpress-develop/pull/7 which makes the helper class anonymous (see discussion), and carried over the changes from https://github.com/ockham/wordpress-develop/pull/6 to use WP_HTML_Text_Replacement (rather than a string buffer) to avoid manipulations of the markup that would break the output (see).

This should be ready for another round of review.

---

Since WP_Block is getting a bit bloated (especially due to block bindings specific code), I'm planning to do a follow-up to move at least replace_html() out of that class (tentatively into block-bindings.php, under the name replace_block_attribute_value()) . (I will keep the block bindings processor class anonymous inside of that function, though.)

@Bernhard Reiter commented on PR #9469:


7 months ago
#7

Unit test failures in CI seem to be unrelated.

#8 @Bernhard Reiter
7 months ago

  • Owner set to Bernhard Reiter
  • Resolution set to fixed
  • Status changed from new to closed

In 60684:

Block Bindings: Allow generically setting rich-text block attributes.

Replace the existing block-specific, hard-coded, logic in the WP_Block class with more generic code that is able to locate and replace a rich-text sourced attribute based on the selector definition in its block.json.

This should make it easier to add block bindings support for more block attributes.

Props bernhard-reiter, jonsurrell, gziolo, cbravobernal, dmsnell.
Fixes #63840.

This ticket was mentioned in PR #9748 on WordPress/wordpress-develop by @Bernhard Reiter.


6 months ago
#9

Backport of changes made to the tests in https://github.com/WordPress/gutenberg/pull/71389.

Independently of that PR, I think these changes make sense, since they make the tests less verbose and less redundant.

The changes are basically as follows:

  • Use the newly introduced block_bindings_supported_attributes_{$block_name} filter to register a test/block's attribute as supported by Block Bindings, rather than using an actual block (Paragraph) for most tests.
  • Merge three test cases that check if get_value_callback works correctly (accepts arguments; correctly includes symbols and numbers; return value is sanitized when rendered) into one, by using a dataProvider.
  • Merge two test cases that check if block context is correctly evaluated, and that access is only given to context included in a source's uses_context property.

No semantic changes have been made, at least not deliberately -- the tests should still cover the same expected behavior.

Trac ticket: https://core.trac.wordpress.org/ticket/63840

#10 @gziolo
6 months ago

In 60720:

Tests: Update rendering test cases for Block Bindings

Refactoring covered:

  • Use the newly introduced block_bindings_supported_attributes_{$block_name} filter to register a test/block's attribute as supported by Block Bindings, rather than using an actual block (Paragraph) for most tests.
  • Merge three test cases that check if get_value_callback works correctly (accepts arguments; correctly includes symbols and numbers; return value is sanitized when rendered) into one, by using a dataProvider.
  • Merge two test cases that check if block context is correctly evaluated, and that access is only given to context included in a source's uses_context property.

Follow-up to [60684].

Props bernhard-reiter, gziolo.
See #63840.

@gziolo commented on PR #9748:


6 months ago
#11

Committed in #60720.

#12 @desrosj
6 months ago

In 60764:

Coding Standards: Apply changes from running composer format.

Follow up to [60684], [60743].

See #63840, #63863.

#14 @wildworks
5 months ago

  • Keywords gutenberg-merge added

@Bernhard Reiter commented on PR #10029:


5 months ago
#15

Thank you! I'll go ahead an land this; we can still iterate and tweak some more if needed.

#16 @Bernhard Reiter
5 months ago

In 60983:

Block Bindings: Polish rich-text attribute replacement code.

In the Block Bindings code introduced to replace rich-text attributes, address feedback to:

  • reuse an existing bookmark (instead of releasing it), and to
  • verify that the expected matching tag closer was found.

Follow-up to [60684].
Props bernhard-reiter, dmsnell.
See #63840.

Note: See TracTickets for help on using tickets.