-
Notifications
You must be signed in to change notification settings - Fork 11
What should be the HTML structure returned when wp-show
is false?
#159
Comments
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 |
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:
|
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 data-wp-show="false" style="display: none;">
<img src="...">
</div> would download 99 unnecessary images on page load. But as everything, it's open for discussion, of course 🙂 |
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 |
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 |
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 |
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. |
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 |
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? |
Well, wrapping them in the const Comp = ({ children }) => {
const [show, setShow] = useState(false);
return show ? children : null;
} |
That makes sense 🙂 Thanks for clarifying! |
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 |
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? |
@luisherranz Seems to work fine. Checked the example here to verify. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden Thanks. |
So if both methods are supported, and if Speaking of a11y, note also @mrwweb's excellent point about how the accessibility in the |
+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 |
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):
|
Sure. If there is a deductible and unambiguous behavior, we can automate it with directives.
That's a shame. Would something like |
The CSS rule |
While working on creating the
wp-show
directive attribute, I wasn't sure the HTML that should be returned whenwp-show
evaluates 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-each
to keep consistency.Alpinejs is using the second approach. For directives like
x-if
orx-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.
The text was updated successfully, but these errors were encountered: