SSR: Add wp-show attribute directive processor

[ockham]

Implement the data-wp-show directive, which is supposed to transform code like

<div data-wp-show="falsyValue" class="my-class">
  <p>Children</p>
</div>

into

<template data-wp-show="falsyValue">
  <div class="my-class">
    <p>Children</p>
  </div>
</template>

per this discussion. (For truthy values of data-wp-show, it does not modify the HTML at all.)

Note that the behavior to move the data-wp-show attribute from the wrapped tag to the wrapping one was implemented in da195d7. If we decide that we'd rather keep the attribute on the wrapped tag, we can simply revert that commit.

[gziolo]

All those directives are neat!

[ockham]

Per #152 (comment), I'll update this PR to turn the wp-show tag directive into an attribute directive.

[ockham]

I'm considering removing the WP_Directive_Processor I've added here, and rebasing this PR on #158 instead to use that PR's WP_Directive_Processor (yep, same name). This is currently blocked (see) but should stop being a problem next week.

We could then rewrite the directive like so:

diff --git a/src/directives/attributes/wp-show.php b/src/directives/attributes/wp-show.php
index 89ed612..82b28cc 100644
--- a/src/directives/attributes/wp-show.php
+++ b/src/directives/attributes/wp-show.php
@@ -3,25 +3,19 @@
 require_once __DIR__ . '/../utils.php';
 
 function process_wp_show( $tags, $context ) {
-	if ( ! $tags->is_tag_closer() ) { // TODO: Exclude void and self-closing!
-		set_bookmark_for_directive( $tags, 'wp-show' );
+	if ( $tags->is_tag_closer() ) {
 		return;
 	}
 
-	$end = 'wp-show-closer';
-	$tags->set_bookmark( 'wp-show-closer' );
-	$start = seek_bookmark_for_directive( $tags, 'wp-show' );
-
 	$value = $tags->get_attribute( 'wp-show' );
-	if ( null !== $value ) {
-		$show = evaluate( $value, $context->get_context() );
+	if ( null === $value ) {
+		return;
+	}
 
-		if ( ! $show ) {
-			$content = $tags->get_content_inside_bookmarks( $start, $end );
-			$tags->set_content_inside_bookmarks( $start, $end, '<template>' . $content . '</template>' );
-		}
+	$show = evaluate( $value, $context->get_context() );
+	if ( ! $show ) {
+		$content = $tags->get_inner_html();
+		echo $content;
+		$tags->set_inner_html( '<template>' . $content . '</template>' );
 	}
-	$tags->seek( $end );
-	$tags->release_bookmark( $start );
-	$tags->release_bookmark( $end );
 }

[dmsnell]

is there any foreseeable need for set_outer_html()? from what @adamziel discovered it seems like that one creates problems with the tag processor, but set_inner_html() doesn't. if we don't have to solve that problem I'd be in favor of letting it sit unsolved.

[luisherranz]

Not having set_outter_html would limit our capability of wrapping the whole tag with <template> in wp-show:

From this:

<div data-wp-show="false" class="my-class">
  <p>Children</p>
</div>

Doing this:

$p->set_outter_html( '<template>' . $p->get_outter_html() . '</template>' );

<template>
  <div data-wp-show="false" class="my-class">
    <p>Children</p>
  </div>
</template>

Instead of this:

$p->set_inner_html( '<template>' . $p->get_inner_html() . '</template>' );

<div data-wp-show="false" class="my-class">
  <template>
    <p>Children</p>
  </template>
</div>

We haven't decided about it yet, but yesterday I shared an example of something that (I think) would not be possible with the set_inner_html option.

• What should be the HTML structure returned when wp-show is false? #159

This case could work with a "wrap tag" API as well, if that would make things easier.

---

It would also leave some leftovers when using wp-fill.

For example, this code:

<table wp-slot="data"></table>

<template wp-fill="data">
  <tr>
    <td wp-text="id"></td>
    <td wp-text="firstName"></td>
    <td wp-text="lastName"></td>
  </tr>
</template>

Would end up as this using $p->set_inner_html('') to remove the fill once it has been moved to the correct place:

<table wp-slot="data">
  <tr>
    <td wp-text="id"></td>
    <td wp-text="firstName"></td>
    <td wp-text="lastName"></td>
  </tr>
</table>

<template wp-fill="data"></template>

Leaving the empty template hanging around, but it doesn't affect the page.

This case could work with a "remove tag" API as well, if that would make things easier.

---

The rest of the core directives we have in mind right now don't seem to need anything similar to set_outter_html.

So at this moment, it depends on what we decide for wp-show. We need to keep investigating.

[SantosGuillamot]

This case could work with a "wrap tag" API as well, if that would make things easier.

Not sure if you meant this, but I suggested a new approach to handle wp-show that wouldn't require set_outer_html if I am not mistaken: Restrict the wp-show directive to be used inside <template>. The same way you are using wp-fill in your examples. As mentioned there, I am not convinced about it, but I think you'll know better than me if it makes sense or not.

[luisherranz]

It makes sense indeed. I've answered you on the other issue:

• What should be the HTML structure returned when wp-show is false? #159 (comment)

[ockham]

Rebased on #169.

[ockham]

We've decided that we'd like wp-show to work as follows:

When wp-show is false:

• Wrap it with <template> in the server (maybe also move the wp-show to the template).
• Remove it from the DOM in the client.

In the client, having the DOM elements wrapped by <template> is not useful. I mean, we already have the static virtual nodes that represent that HTML in memory, we don't need them in the DOM anymore.

I.e. we'd like the server-side rendered wp-show directive to return something like this:

<template>
  <div data-wp-show="false" class="my-class">
    <p>Children</p>
  </div>
</template>

(with data-wp-show="false" possibly moved to <template>)

Our directive processor (#169) currently doesn't support set_outer_html, and it's been found that the latter could be somewhat tricky to implement.

As a consequence, I'd like to try implementing a wrap_in_tag (or so) method first, which should hopefully be easier, since it's not as potentially "destructive" to the current tag as set_outer_html.

[luisherranz]

I'm going to close this issue as part of the migration to the Gutenberg repository.

This task was added to the Roadmap and we'll open a new issue/discussion once it's time to start working on this again.