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

Apply a filter to each rendered static and dynamic block #11523

Closed
danielbachhuber opened this issue Nov 6, 2018 · 11 comments
Closed

Apply a filter to each rendered static and dynamic block #11523

danielbachhuber opened this issue Nov 6, 2018 · 11 comments
Labels
[Feature] Blocks Overall functionality of blocks [Feature] Extensibility The ability to extend blocks or the editing experience

Comments

@danielbachhuber
Copy link
Member

To permit modification of all blocks, including both existing static and dynamic, it would be helpful to apply a filter to the rendered value of each.

Specifically, something along these lines:

$output = apply_filters( 'do_block', $output, $block );

do_shortcode_tag (ref) is existing prior art.

Proposed Implementation

This filter would be implemented inside of do_blocks(). There's a relatively simple way to do this that exposes a broader issue.

With the simple way, we'd first apply the do_block filter at the point of rendering each dynamic block:

gutenberg/lib/blocks.php

Lines 250 to 251 in a696e50

// Replace dynamic block with server-rendered output.
$rendered_content .= $block_type->render( $attributes, $inner_content );

Then, we'd update the existing preg_replace call that strips HTML comment declarations to instead call preg_replace_callback() and call the do_block filter on the content prior to stripping the content:

gutenberg/lib/blocks.php

Lines 260 to 261 in a696e50

// Strip remaining block comment demarcations.
$rendered_content = preg_replace( '/<!--\s+\/?wp:.*?-->\r?\n?/m', '', $rendered_content );

However, you may have noticed an issue: the do_block filter for dynamic blocks is run before static blocks. This is a minor problem that could be a more annoying implementation detail further down the road. Ideally, the do_block filter should execute in the order that the blocks are present in the post.

Instead of the simple way, we should probably update do_block()'s initial while ( preg_match( statement to also match static blocks, and then switch between rendering behavior (dynamic vs. static).

We'll also need to make sure performance is within reasonable limits.

After Merge

After we settle upon an implementation here, we'll also need to patch core's copy of do_block().

@danielbachhuber danielbachhuber added [Feature] Blocks Overall functionality of blocks [Feature] Extensibility The ability to extend blocks or the editing experience labels Nov 6, 2018
@danielbachhuber danielbachhuber added this to the WordPress 5.0 milestone Nov 6, 2018
@mtias
Copy link
Member

mtias commented Nov 8, 2018

I like this, it simplifies the need of having to call register_block and adding a render callback.

@azaozz
Copy link
Contributor

azaozz commented Nov 10, 2018

To permit modification of all blocks...

What kind of "modifications" do you have in mind? :)

Passing a chunk of HTML as a string would only permit parsing that HTML with a regex, and as we all know, doing that leads to utter madness!! https://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags?page=1&tab=votes#tab-top.

The only way this can be useful is if the block attributes are parsed and passed along in that filter. Then all blocks will become "semi-dynamic" on the front-end allowing modifications based on the attributes set in the editor.

I think this would be very useful and can see some pretty interesting user cases for it. For example #11377 makes the image block "dynamic" just so it is able to add couple attributes to the <img> tag based on block attributes and the theme's width.

@spencerfinnell
Copy link

The only way this can be useful is if the block attributes are parsed and passed along in that filter. Then all blocks will become "semi-dynamic" on the front-end allowing modifications based on the attributes set in the editor.

This would be key for sure. A great use case is per-block content restriction based on additional settings added to core block controls.

@pento
Copy link
Member

pento commented Nov 11, 2018

This filter already exists in Core, and the changes to the parser and do_blocks() merged late last week make it possible in the plugin, too.

@pento
Copy link
Member

pento commented Nov 12, 2018

Actually, I changed my mind about the do_block filter. It's nicer to do it in render_block(), so I renamed the filter in Core to match.

This was referenced Nov 12, 2018
@azaozz
Copy link
Contributor

azaozz commented Nov 12, 2018

Yep, the new 'render_block' filter in wp-includes/blocks.php works quite nicely. It passes the whole $block array as a param, including blockName, attrs, innerBlocks, innerHTML, etc.

Thinking this can be closed as "fixed". Or do we want to backport this to Gutenberg to make it work in WP < 5.0?

@danielbachhuber
Copy link
Member Author

Ideally, it would've been backported to Gutenberg when it was originally implemented.

Given the current release timeline though, I don't think it's necessary to backport. We can mark as fixed.

@ataylorme
Copy link

@pento I tried out the do_block filter (example repo) to add server-side syntax highlighting to the core code block.

The main issue I ran into was that the content of the block is not available, only the innerHTML/block content. I had to disassemble the HTML, add syntax highlighting to the contents, then reassemble things. It worked but seems less than ideal.

@ataylorme
Copy link

It would be great if there was a rawContent or similar attribute that had the block content without the HTML markup.

@azaozz
Copy link
Contributor

azaozz commented Nov 13, 2018

@ataylorme that would depend on the block. If you need more "data" about the content passed to the front-end, the editor would need to store that data in the block attributes.

For the "code" block that may be the text (code) the user entered. For other blocks it may be something else. I'd think it won't be that hard to add this, but it is a change in the way the block works.

@ataylorme
Copy link

@azaozz yes, I think having the contents of all attributes registered saved and made available would be an enhancement.

The code block, for example, registers a content attribute.

When interacting with a block I expect to be able to access data for all its attributes. This is true for contexts outside these filters as well. For example, if a REST API endpoint as proposed in #4763 is added that data could be used in applications that don't output HTML, like a voice-based app, yet only the HTML output is available.

Working with the HTML alone it is not ideal. I had to do some regex processing to strip it and get to the content attribute data of the code block. For blocks with more than one attribute that is not being saved and the data made available that processing will be much more difficult.

If you think this is worth pursuing I'm happy to open a new issue. I just wanted to provide feedback on my experience working with the do_block filter. I was able to do what I set out to accomplish but it took much more code than necessary due to the lack of data for all attributes being available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Feature] Extensibility The ability to extend blocks or the editing experience
Projects
None yet
Development

No branches or pull requests

6 participants