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

HTMLTagProcessor Examples #137

Merged
merged 6 commits into from
Jun 30, 2023
Merged

HTMLTagProcessor Examples #137

merged 6 commits into from
Jun 30, 2023

Conversation

pdavies88
Copy link
Contributor

Description of the Change

Add a guide for extending a dynamic block using the HTMLTagProcessor

Closing # #134

@pdavies88 pdavies88 requested a review from fabiankaegy May 23, 2023 18:27
@pdavies88 pdavies88 self-assigned this May 23, 2023
@vercel
Copy link

vercel bot commented May 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
10up-gutenberg-best-practices ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 30, 2023 5:59am

Copy link
Member

@fabiankaegy fabiankaegy 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 for this draft :) I really like the examples you came up with and this reads very well!

I added some inline comments and suggestions. Also there is one thing still missing that is crucial for this to work when we add new settings like your accessibleText attribute. And that is adding the attribute registration on the server. registerBlockExtension only registers the new attribute in JS. But PHP is still unaware of it's existence which can cause many issues especially when using ServerSideRender anywhere since the REST API does some data validation against the registered attributes and throws an error when unknown attributes get passed in.

You can see an example of how you can add an additional attribute here:

/**
 * Register animation attribute to blocks
 */
function register_animation_attribute_for_blocks() {
	$registered_blocks = \WP_Block_Type_Registry::get_instance()->get_all_registered();

	foreach ( $registered_blocks as $name => $block ) {
		$block->attributes['animation'] = array( 'type' => 'object' );
	}
}

add_filter( 'wp_loaded', __NAMESPACE__ . '\\register_animation_attribute_for_blocks', 999 );

Another option would be filtering the block_type_metadata_settings hook to add the attribute there.

Let me know if you have any questions :)

Comment on lines 17 to 18
### Caveats:
* This was only introduced in WP 6.2 earlier versions do not have this capability
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove this because the entire documentation is always meant to be for the latest version of WordPress and any site we are actively working on should always update to the latest version relatively quickly

* This was only introduced in WP 6.2 earlier versions do not have this capability

## Code Example:
This example showcases how to use `WP_HTML_Tag_Processor` with new block attributes. This example extends the `core/button` block and adds in fields for `Accessible Text` and `Button Padding` these new fields can then be used within PHP to create filters with the `WP_HTML_Tag_Processor` to update values of the rendered block content. In this case add in an `aria-label` with the text provided and set the `padding` around the button with the value provided via the block settings.
Copy link
Member

Choose a reason for hiding this comment

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

I really like this example. However I would reduce the complexity and remove the padding setting so we only worry about one singular one.

Also the core button does support padding out of the box so I don't want to confuse anyone into thinking its a good idea add a custom padding controll :)

guides/html-tag-processor.md Outdated Show resolved Hide resolved
guides/html-tag-processor.md Outdated Show resolved Hide resolved
pdavies88 and others added 2 commits May 24, 2023 20:19
}
}

add_filter( 'wp_loaded', __NAMESPACE__ . '\\register_accessibletext_attribute_for_blocks', 999 );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fabiankaegy please check me on this function and the comment as I used the example you provided around animation as a guideline but I am not sure what I did is totally correct. Unlike the animation scenario in this use case we would just get a string back for the attribute of accessibleText

Copy link
Member

Choose a reason for hiding this comment

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

@pdavies88 What you did for the attribute itself is correct :)

The only thing I would change is that both in the commend and in the actual code you are adding the additional attribute to all blocks. In this case we only want to add it to one particular block. We should update that before we merge this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fabiankaegy I updated it to be wrapped in an if statement. Let me know if you think that is the correct approach:

/**
 * Register accessibleText attribute to core/button
 * Assists with ServerSideRender
 * REST API does data validation against the registered attributes
 * Prevents throwing an error when unknown attributes get passed in
 */
function register_accessibletext_attribute_for_blocks() {
	$registered_blocks = \WP_Block_Type_Registry::get_instance()->get_all_registered();
	foreach ( $registered_blocks as $name => $block ) {
		if ( 'core/button' === $block->name ) {
			$block->attributes['accessibleText'] = array( 'type' => 'string' );
		}
	}
}
add_filter( 'wp_loaded', __NAMESPACE__ . '\\register_accessibletext_attribute_for_blocks', 999 );

@pdavies88 pdavies88 requested a review from fabiankaegy May 25, 2023 00:35
Copy link
Member

@fabiankaegy fabiankaegy left a comment

Choose a reason for hiding this comment

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

This is a great addition :)

@fabiankaegy fabiankaegy merged commit ec4abf5 into main Jun 30, 2023
@fabiankaegy fabiankaegy deleted the guide/HTMLTagProcessor branch June 30, 2023 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants