Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't accommodate hidden elements in space/divide #13459

Merged
merged 3 commits into from
Apr 9, 2024
Merged

Conversation

adamwathan
Copy link
Member

This PR changes the implementation of the space-* and divide-* utilities to use a different selector, and apply the "between" styling to the first/left element instead of the last/right element:

- .space-y-4 > :not([hidden]) ~ :not([hidden]) {
+ .space-y-4 > :not(:last-child) {
-   margin-top: 1rem;
+   margin-bottom: 1rem;
  }

This is a small breaking change, specifically when the very last child of an element has the hidden attribute:

<ul class="space-y-4">
  <li>One</li>
  <li>Two</li>
  <li>Three</li>
  <li hidden>Hidden</li>
</ul>

Prior to this PR, the "Three" element would have no space below it, but with this new implementation it does. I suspect this change will impact almost nobody though, because in all of the real-world scenarios I've explored, hidden elements are never the very last element. They are almost always the very first element, or first few elements, and in those situations this new implementation has the same behavior as the old implementation.

Motivation

The "between" styling that we do with space-* and divide-* is most commonly achieved in CSS with the "lobotomized owl selector", like this:

.space-y-4 > * + * {
  margin-top: 1rem;
}

Our implementation in Tailwind though has always used selectors that look like this:

.space-y-4 > :not([hidden]) ~ :not([hidden]) {
  margin-top: 1rem;
}

Pretty much the only reason we did this was so that you could use these utilities with x-for in Alpine.js

<ul class="space-y-4" x-data="{ colors: ['Red', 'Orange', 'Yellow'] }">
  <template x-for="color in colors">
    <li x-text="color"></li>
  </template>
</ul>

Alpine leaves that <template> tag in the DOM, which means the first <li> is actually the second child of the <ul>, and receives the "between" styling even when it shouldn't:

https://play.tailwindcss.com/7q6MiKZnRg

By using the implementation we've used up until now, Alpine users could throw the hidden attribute on their <template> element and now the "between" styling would only show up where it's supposed to:

https://play.tailwindcss.com/p6ZGpSw1zt

The problem is on pages with a lot of DOM nodes, the selector we use is fucking slow 😄

https://twitter.com/cramforce/status/1774847095171862943

image

It's not just "technically slow but you can't actually tell because it's still fast" either — if you have a really big list or table or something it's very, very noticeable:

#13445

The new implementation introduced in this PR is almost 2000x faster, getting things back down to "feels as fast as any other normal selector" speeds.

One nice benefit of the new implementation is that you don't need to use the hidden attribute on template tags anymore — the "between" styling just works as expected without it because the template tag is receiving a margin-bottom for example but the entire element is hidden so that margin isn't actually rendered in the browser anyways.

Copy link
Member

@RobinMalfait RobinMalfait left a comment

Choose a reason for hiding this comment

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

Looks good, just needs a changelog entry

@adamwathan adamwathan merged commit 818692f into next Apr 9, 2024
1 check passed
@adamwathan adamwathan deleted the fast-space-divide branch April 9, 2024 16:59
@Schepp
Copy link

Schepp commented May 2, 2024

I know I'm late to the party, but I'd like to suggest adding margin-trim to the space-* containers in order to trim away any leftover margin at their far edges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants