What should be the HTML structure returned when wp-show is false?
#53058
Replies: 23 comments 6 replies
-
|
wrapping in a that also means we can't apply styling to them, not that we would likely want to, but for example, some debug border to show the location of the directives wouldn't be possible. since I'm less aware, possibly forgetful, is the requirement to wrap in a
the main thing is probably that Option 1 simply doesn't participate in layout, as it's not in the page, whereas Option 2 lives in the DOM as an empty
|
Beta Was this translation helpful? Give feedback.
-
|
Imagine these blocks. Each inner block can be hidden or shown when clicked, and they can start hidden or shown depending on an attribute. <!-- wp:superlist/list -->
<ul class="wp-block-superlist-list">
<!-- wp:superlist/list-item -->
<li class="wp-block-superlist-list-item">Item 1</li>
<!-- /wp:superlist/list-item -->
<!-- wp:superlist/list-item -->
<li class="wp-block-superlist-list-item">Item 2</li>
<!-- /wp:superlist/list-item -->
<!-- wp:superlist/list-item -->
<li class="wp-block-superlist-list-item">Item 3</li>
<!-- /wp:superlist/list-item -->
</ul>
<!-- /wp:superlist/list --><?php
// render.php - superlist/list-item
?>
<li
<?php echo get_block_wrapper_attributes(); ?>
data-wp-context='{ "superlist": { "isShown": <?php echo $attributes[ "isShown" ]; ?> } }'
data-wp-show="context.superlist.isShown"
data-wp-on:click="actions.superlist.toggleItem"
>
<?php echo $attributes[ 'text' ]; ?>
</li>If we use Option 2 and the second element is hidden, the resulting HTML would be (I'm omitting the directives for simplicity): <!-- wp:superlist/list -->
<ul class="wp-block-superlist-list">
<!-- wp:superlist/list-item -->
<li class="wp-block-superlist-list-item">Item 1</li>
<!-- /wp:superlist/list-item -->
<!-- wp:superlist/list-item -->
<li class="wp-block-superlist-list-item">
<template>Item 2</template>
</li>
<!-- /wp:superlist/list-item -->
<!-- wp:superlist/list-item -->
<li class="wp-block-superlist-list-item">Item 3</li>
<!-- /wp:superlist/list-item -->
</ul>
<!-- /wp:superlist/list -->That is not correct because it's rendered like this:
This specific case seems to be resolved by adding <?php
// render.php - superlist/list-item
wp_store( array(
'selectors' => array(
'superlist' => array(
'listItemDisplay' => $attributes[ 'isShow' ] ? 'list-item' : 'contents'
),
),
) );
?>
<li
...
data-wp-style:display="selectors.superlist.listItemDisplay"
>
...
</li>// view.js
store({
selectors: {
superlist: {
listItemDisplay: ({ context }) =>
context.superlist.isShown ? "list-item" : "contents",
},
},
});There are also some accessibility problems related to display: contents. So:
|
Beta Was this translation helpful? Give feedback.
-
I believe it's the correct one because the browser doesn't load the assets. For example, a 100-image slideshow using this for each image: <div style="display: none;">
<img src="...">
</div>would download 99 unnecessary images on page load. But as everything, it's open for discussion, of course 🙂 |
Beta Was this translation helpful? Give feedback.
-
interesting. have we confirmed this with any tests? seems like browsers today might optimize that away. if not, I can run a test and check |
Beta Was this translation helpful? Give feedback.
-
I did a quick test in Stackblitz and it seems like Chrome is indeed honoring that behavior. Although I wouldn't mind if it wouldn't in certain circumstances, like a desktop computer with a fiber connection (similar to what it does with Screen.Capture.on.2023-03-03.at.11-48-04.mp4 |
Beta Was this translation helpful? Give feedback.
-
|
Option 2 combined with tables would result in An extra table row also sounds problematic for all sorts of reasons: borders, paddings, rowspans and colspans. Perhaps if the outer tag was rendered with an inline |
Beta Was this translation helpful? Give feedback.
-
|
Maybe there's a third option I didn't share in the opening post: Restrict the <template data-wp-show="false">
<div class="my-class">
<p>Children</p>
</div>
</template>I'm not convinced of this approach, I don't see clear benefits of this one over option 1, but sharing it here in case it makes sense somehow. I'm still not familiar with how this would work internally, especially in the SSR. |
Beta Was this translation helpful? Give feedback.
-
Yeah, my neither... but it's the best option if we finally can't use
The problem is that if we use
I think it was me who told you this, but I was wrong. Alpine is using I think the best option in terms of developer experience is 1. Actually, I would even use a slightly simplified variation of 1. When
In the client, having the DOM elements wrapped by |
Beta Was this translation helpful? Give feedback.
-
Okay, you're right! Got it 🙂
I agree with this. I guess I would just go with option 3 if we can't use
I like it 🙂 I have one question, though: Would this mean that, if you inspect the HTML in the browser, you won't be able to see the hidden elements? Could this cause some issues with debugging? |
Beta Was this translation helpful? Give feedback.
-
Well, wrapping them in the const Comp = ({ children }) => {
const [show, setShow] = useState(false);
return show ? children : null;
} |
Beta Was this translation helpful? Give feedback.
-
|
That makes sense 🙂 Thanks for clarifying! |
Beta Was this translation helpful? Give feedback.
-
|
For an alternative to wrapping hidden elements in It is currently supported by Chrome and Edge. It is spec'ed in the HTML5 living standard. Therefore, if the primary concern is about the browser downloading images that are in hidden containers, I suggest the solution be to ensure that |
Beta Was this translation helpful? Give feedback.
-
Oh, I didn't know It doesn't cover scripts, does it? For example, in this case, the Twitter JavaScript will be loaded before the tweet is shown on the screen: <blockquote class="twitter-tweet">Tweet content...</blockquote>
<script async src="https://platform.twitter.com/widgets.js"></script>I guess sometimes that could be a problem, although sometimes that could be the desired behavior. So maybe the best option is to do the same as Alpine and ship two ways to do conditionals, one for From what I read (and unlike Oh, and we don't need a special directive for <div data-wp-bind.hidden="context.someValue" class="my-class">
<p>Children</p>
</div>@alexstine, @joedolson: what do you think? |
Beta Was this translation helpful? Give feedback.
-
|
@luisherranz Seems to work fine. Checked the example here to verify. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden Thanks. |
Beta Was this translation helpful? Give feedback.
-
|
So if both methods are supported, and if Speaking of a11y, note also @mrwweb's excellent point about how the accessibility in the |
Beta Was this translation helpful? Give feedback.
-
|
+1 for relying on the That said, seeing the example of Or maybe a custom directive is not the right way here, but we may be able to automate the |
Beta Was this translation helpful? Give feedback.
-
I've taken that approach in scripts I write and generally like it. One advantage is that it fails if your IDs aren't unique. Generally speaking, anything like that to enforce accessible (otherwise it's broken) seems like a good idea.
I suspect this won't work out very well. From Monica Dinculescu (h/t CSS-Tricks):
|
Beta Was this translation helpful? Give feedback.
-
Sure. If there is a deductible and unambiguous behavior, we can automate it with directives.
That's a shame. Would something like |
Beta Was this translation helpful? Give feedback.
-
|
The CSS rule |
Beta Was this translation helpful? Give feedback.
-
|
By the way, now that the <div data-wp-context='{ "isShown": "none" }'>
<button data-wp-on--click="actions.toggle">Toggle</button>
<div data-wp-style--display="context.isShown">
<p>Children</p>
</div>
</div>store({
actions: {
toggle: ({ context }) => {
context.isShown = context.isShown === "none" ? "inherit" : "none";
}
}
}); |
Beta Was this translation helpful? Give feedback.
-
|
Hey, sticking with Option 1, what about something like this? <!-- False -->
<template
data-wp-show="false"
data-wp-show--tag="div"
data-wp-show--attributes='{"class":"my-class"}'
>
<p>Children</p>
</template>
<!-- True -->
<div data-wp-show="true" class="my-class">
<p>Children</p>
</div>I think it could work. We would keep all the information to restore the elements whenever the condition turns The only thing is that we need to change the name for both the initial and the closing tags during SSR. I don't know how feasible it would be with the WP HTML Processor. |
Beta Was this translation helpful? Give feedback.
-
|
Still figuring out this proposal, haha. It may not make any difference compared with others already exposed before. 🤷 |
Beta Was this translation helpful? Give feedback.
-
|
Circling back to this, perhaps there should be separate directives for Show: this should be related to css styles (display:none, for example). Since there is already a syntax for <template data-wp-if="context.open">
<p>Some arbitrary content</p>
</template>This is inline with how Alpinejs handles these two cases, with the 2 directives. |
Beta Was this translation helpful? Give feedback.
-
|
Alpine does not support server-side rendering, but for the Interactivity API, there is no need to use a template. We can allow people to place the directive directly on the element. If the directive reference evaluates to The dev would write: <p data-wp-if="context.open">Some arbitrary content</p>The Interactivity API in the server would leave it as it is if <template data-wp-if="context.open">
<p>Some arbitrary content</p>
</template>Does that make sense? 🙂
So far, people have been doing this using With regard to the name, both |
Beta Was this translation helpful? Give feedback.
-
|
Great points! Yes it does make sense. I personally think |
Beta Was this translation helpful? Give feedback.
-
|
Perhaps what we need to consider is that if we ever manage to add A tricky question for sure! 😄 |
Beta Was this translation helpful? Give feedback.
-
|
Another great point. |
Beta Was this translation helpful? Give feedback.
-
|
If you're willing to try implementing it, drop by the Slack channel #core-interactivity-api and we can discuss how it should work. We have some previous attempts around that could be useful 🙂 |
Beta Was this translation helpful? Give feedback.
-
|
Hi! Is there any news on the implementation of the directive? |
Beta Was this translation helpful? Give feedback.
-
While working on creating the
wp-showdirective attribute, I wasn't sure the HTML that should be returned whenwp-showevaluates asfalse. I'm opening this issue to discuss the implications of the different alternatives and decide on an approach.Given the following HTML:
I considered two alternatives:
Option 1 - Wrapped everything inside the
<template>tagOption 2 - Wrapped children inside the
<template>tagAs I mentioned, I don't know the pros and cons of each alternative, so I'd love to hear your thoughts. Whatever we decide, should probably be applied to similar directives like
wp-eachto keep consistency.Alpinejs is using the second approach. For directives like
x-iforx-for, they used them directly in the<template>tags, and it requires that<template>MUST contain only one root element.On the other hand, if not explained properly, I feel that Option 2 could cause unexpected layout issues.
Beta Was this translation helpful? Give feedback.
All reactions