Fix interactivity API enabled navigation block #7650
Conversation
thelovekesh
force-pushed
the
fix/navigation-block
branch
from
851e7ef
to
cf16557
Compare
|
Plugin builds for 1739125 are ready 🛎️!
ChecksumsWarning These builds are for testing purposes only and should not be used in production. |
thelovekesh
force-pushed
the
fix/navigation-block
branch
from
bd6d2c6
to
decc397
Compare
| @@ -339,6 +339,8 @@ public function ampify_navigation_block( $block_content, $block ) { | |||
| add_action( 'wp_print_footer_scripts', [ $this, 'dequeue_block_navigation_view_script' ], 0 ); | |||
| } | |||
|
|
|||
| $is_interactive_block = false !== strpos( $block_content, 'data-wp-interactive' ); | |||
There was a problem hiding this comment.
This could be made more robust. What if someone uses data-wp-interactive in the text of the block?
I guess that's true for the other existing code below.
At the very least:
| $is_interactive_block = false !== strpos( $block_content, 'data-wp-interactive' ); | |
| $is_interactive_block = false !== strpos( $block_content, ' data-wp-interactive' ); |
There was a problem hiding this comment.
What about checking this attribute in the <nav> attributes? Something similar to:
$is_interactive_block = str_contains(
preg_match( '/(?<=<nav)\s[^>]+/', $block_content, $matches ) ? $matches[0] : '',
' data-wp-interactive',
);Surprisingly adding space before data-wp-interactive is failing here, because it's added as a newline in the outputted block content HTML.
There was a problem hiding this comment.
Also, since it's a navigation block, should we expect such a string in block text?
There was a problem hiding this comment.
Yeah, that preg_match() seems like it would do the trick.
Also, since it's a navigation block, should we expect such a string in block text?
Maybe if someone likes to blog about different attributes and has a link to a page all about this one 😄
| // Delete `data-wp-*` attributes. | ||
| if ( $is_interactive_block ) { | ||
| $block_content = preg_replace( '/\sdata-wp-[^=]+="[^"]*"/', '', $block_content ); | ||
| $block_content = preg_replace( '/\sdata-wp-[^=]+=\'[^\']*\'/', '', $block_content ); | ||
| } | ||
|
|
There was a problem hiding this comment.
These can remain, right? They just won't do anything.
There was a problem hiding this comment.
Yes, they won't, apart from increasing content length by a few bytes 😅
There was a problem hiding this comment.
Just that their removal could cause problems due to the string replacement not using parsed HTML, and they take additional (inconsequential) server time to remove. IMO, I think we can just leave them.
Co-authored-by: Weston Ruter <westonruter@google.com>
westonruter
added
the
Changelogged
Summary
Fixes #7647
Checklist