Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

⚛️ Use a wp-inner-block attribute for each inner block #77

Merged

Conversation

DAreRodz
Copy link
Collaborator

Will fix #76

@DAreRodz
Copy link
Collaborator Author

We cannot use a regexp that detects the class name for inner blocks, as there could be blocks without it (e.g., Paragraph).

However, we can mark all inner blocks of interactive blocks, adding a callback to the render_block_data hook and "polluting" the parsed block data ($parsed_block) with a flag that indicates it is an inner block.

https://github.com/WordPress/block-hydration-experiments/blob/fe4994b561c7d82a558edc24cd630611b9ea03f7/block-hydration-experiments.php#L140-L154

Then, we can read that flag during render_block and append the wp-inner-block attribute to the block wrapper. Again, we cannot use the block class name to detect the wrapper: we should assume it is the first HTML tag or it won't work for those blocks that don't have a class name.

@DAreRodz DAreRodz marked this pull request as ready for review September 27, 2022 12:03
@DAreRodz DAreRodz changed the title ⚛️ Remove the wp-inner-blocks wrapper and add wp-inner-block attribute for each child ⚛️ Use a wp-inner-block attribute for each inner block Sep 27, 2022
@DAreRodz
Copy link
Collaborator Author

There's an issue with inner blocks' hydration. I'm taking a look.

@DAreRodz DAreRodz marked this pull request as draft September 27, 2022 12:17
@DAreRodz DAreRodz marked this pull request as ready for review September 27, 2022 15:03
@DAreRodz
Copy link
Collaborator Author

DAreRodz commented Sep 27, 2022

There's an issue with inner blocks' hydration. I'm taking a look.

Some text nodes with white spaces were making hydration fail (again). I fixed it by preserving those nodes that appeared between inner blocks.

Copy link
Member

@luisherranz luisherranz left a comment

Choose a reason for hiding this comment

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

LGTM!!

I'm going to merge this and resolve conflicts in #74.

Comment on lines +51 to +52
const isInnerBlock = ({ props }) => props && 'wp-inner-block' in props;
if (children.some(isInnerBlock)) {
Copy link
Member

Choose a reason for hiding this comment

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

Note for the future: this is probably not the best approach because, although key in obj is fast, it's doing it multiple times.

If we continue with this approach, we could create a nice set of benchmarks for the toVdom function 🙂

if (children.some(isInnerBlock)) {
const first = children.findIndex(isInnerBlock);
const last = children.findLastIndex(isInnerBlock);
innerBlocksFound = children.slice(first, last + 1);
Copy link
Member

Choose a reason for hiding this comment

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

Note for the future: As we are not doing out-of-order hydration for the moment, we could bypass the toVdom processing of the client component inner nodes.

$class_pattern = '/class="\s*(?:[\w\s-]\s+)*' . $block_classname . '(?:\s+[\w-]+)*\s*"/';
$class_replacement = '$0 ' . $block_wrapper_attributes;
$block_content = preg_replace( $class_pattern, $class_replacement, $block_content, 1 );
$pattern = '/^\s*<[^>]+/';
Copy link
Member

Choose a reason for hiding this comment

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

Ok, so this is now looking for the first tag opener, whatever it is, and it could be replaced with this, right?

$w = new WP_HTML_Tag_Processor($block_content);
$w->next_tag();
foreach ($attributes as $key => $value) {
  $w->set_attribute($key, $value);
}

With a bit more logic for the style and class attributes.

* TODO: use `WP_HTML_Tag_Processor` (see https://github.com/WordPress/gutenberg/pull/42485) once
* the API is released.
*/
function bhe_append_attributes($block_name, $block_content, $attributes)
Copy link
Member

Choose a reason for hiding this comment

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

$block_name doesn't seem to be used anymore.

@luisherranz luisherranz merged commit 4cabeb8 into main-full-vdom-hydration Sep 28, 2022
@luisherranz luisherranz deleted the full-vdom/remove-wp-inner-blocks-wrapper branch September 28, 2022 07:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants