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

Add space and divide utilities #1584

Merged
merged 6 commits into from
Apr 17, 2020
Merged

Add space and divide utilities #1584

merged 6 commits into from
Apr 17, 2020

Conversation

adamwathan
Copy link
Member

This PR adds support for new space-{x/y}-{n}, divide-{x/y}-n, and divide-{color} utilities for adding space or borders between child elements.

Examples

Adding 16px of vertical space between items:

<!-- Before -->
<ul>
  <li>One</li>
  <li class="mt-4">Two</li>
  <li class="mt-4">Three</li>
</ul>

<!-- Now, using `space` utilities -->
<ul class="space-y-4">
  <li>One</li>
  <li>Two</li>
  <li>Three</li>
</ul>

Adding 8px of horizontal space between items:

<!-- Before -->
<nav class="flex">
  <a href="#">One</li>
  <a href="#" class="ml-2">Two</li>
  <a href="#" class="ml-2">Three</li>
</ul>

<!-- Now, using `space` utilities -->
<nav class="flex space-x-2">
  <a href="#">One</li>
  <a href="#">Two</li>
  <a href="#">Three</li>
</ul>

Adding a 1px border between items:

<!-- Before -->
<ul>
  <li>One</li>
  <li class="border-t">Two</li>
  <li class="border-t">Three</li>
</ul>

<!-- Now, using `divide` utilities -->
<ul class="divide-y">
  <li>One</li>
  <li>Two</li>
  <li>Three</li>
</ul>

Specifiying a border color:

<!-- Before -->
<ul>
  <li>One</li>
  <li class="border-t border-gray-200">Two</li>
  <li class="border-t border-gray-200">Three</li>
</ul>

<!-- Now, using `divide` utilities -->
<ul class="divide-y divide-gray-200">
  <li>One</li>
  <li>Two</li>
  <li>Three</li>
</ul>

Implementation

You'll often see classes like this implemented using the "lobotomized owl selector":

/* Not our implementation */
.space-x-4 > * + * {
  margin-left: 1rem
}

I had originally implemented these utilities this way as well, but ran into an interoperability problem with Alpine.js because of their use of the <template> tag to store markup snippets in the DOM.

If they were doing something extremely weird I would likely just say "forget it" and use the owl selector instead, but I honestly think the fact that the browser even looks at template tags when evaluating CSS is borderline a bug, and since I personally really enjoy using Alpine, it seemed worth trying to fix if I could find a solution that I was happy with.

After trying a bunch of different things, I landed on this as the best solution:

/* The implementation I decided on */
.space-x-4 > :not(template) ~ :not(template) {
  margin-left: 1rem
}

This effectively ignores any template tags, allowing it to work even with markup as weird as this:

<ul class="divide-y">
  <template>👻</template>
  <li>One</li>
  <li>Two</li>
  <template>👻</template>
  <template>👻</template>
  <li>Three</li>
  <template>👻</template>
  <li>Four</li>
  <li>Five</li>
  <template>👻</template>
</ul>

It unfortunately comes at the cost of increased specificity (0, 1, 2 instead of 0, 1, 0) which means you cannot override the styles applied by these classes with simpler utilities. For example, this will not work:

<ul class="divide-y">
  <li>One</li>
  <li class="border-t-0">Two</li>
  <li>Three</li>
</ul>

After trying to think through real-world scenarios where this would actually matter though, I wasn't able to come up with much. Even if you do run into this, you can always revert to the "old way" and not use these new classes at all in those situations.

This specificity problem isn't really new in Tailwind anyways — it's also present with the group-hover utilities which have a specificity of 0, 2, 0 while regular hover utilities are 0, 1, 1.

I have some ideas for tackling those problems in the future in a more general way if they really end up being problems for people, but so far it's only come up one time that I have ever seen.

These utilities have all been implemented as new core plugins, and all have their own new keys in the theme configuration:

  • space
  • divideWidth
  • divideColor

The space utility pulls from spacing by default, divideWidth pulls from borderWidth, and divideColor pulls from borderColor.

I considered having these plugins reference those keys directly instead of giving them their own keys that inherit from the existing keys (I even considered just adding these classes to the existing margin, borderWidth, and borderColor plugins) but ultimately it felt most flexible and consistent to give them their own keys, especially since for all intents and purposes end-users won't even know the difference between them having their own keys or sharing existing keys, since updates to borderWidth for example will automatically apply to divideWidth anyways.

Only have "responsive" variants generated by default.

Why not a variant?

I considered implementing this as some sort of very generic children-except-first variant that you'd use like this:

<div class="children-except-first:border-t children-except-first:border-gray-200">
  <!-- ... -->
</div>

...but for these two extremely common problems (spacing things out or adding dividers), the syntax felt too obscure. We could still add something like that in the future if we really wanted, but I think from a pure developer experience perspective I prefer the spacing/dividers problem to have a more explicit solution.

@paulcsmith
Copy link

Cool as hell. Can't wait to use it!

@adamwathan adamwathan merged commit 8b2473f into master Apr 17, 2020
@adamwathan adamwathan deleted the stack-divide-utilities branch April 17, 2020 17:26
@vricop
Copy link

vricop commented Apr 17, 2020

You really put a lot of thinking into this. I like that implementation. 👏👏👏👏👏

@benface
Copy link
Contributor

benface commented Apr 17, 2020

@adamwathan Am I correct that the space and divide classes aren't really meant to be used together? It seems the border would appear stuck to the second item instead of exactly between the two items. It looks like if we want to use divide, the space between the items should be handled with padding on each item. I can see that being a source of confusion.

Overall I think this is a nice addition to Tailwind but I'm a bit concerned with the number of potential cases where the abstraction breaks. I already told you about this on Discord some time ago, but here are some of them:

  • When used on elements that contain inline items that wrap (including flex flex-wrap), space-x will add a space at the beginning of every new line.
  • If the first item is hidden, the first visible item will have unexpected extra space or a border before it.
  • space-x doesn't work correctly on RTL languages (margin-right should be used instead of margin-left). Probably not a huge deal in the sense that Tailwind doesn't include anything for RTL out of the box (there exist plugins to add an rtl: variant and for CSS Logical Properties and Values), but for this feature I think it should be considered.

@sir-kain
Copy link

sir-kain commented Apr 17, 2020

Proposal

  1. I suggest that we use gap instead of space as the name of the utility like we have with the grid-gap css property which feel more adequate for this case.

Instead of space-{x/y}-{n}, use gap-{x/y}-{n}

Examples
Adding 16px of vertical space between items:

<ul>
  <li>One</li>
  <li class="mt-4">Two</li>
  <li class="mt-4">Three</li>
</ul>

<!-- Now, using `gap` utilities -->
<ul class="gap-y-4">
  <li>One</li>
  <li>Two</li>
  <li>Three</li>
</ul>
  1. divider !== border-bottom
    A semantic divider should be an independent html element like <hr>, to be able to control the exact spacing with these sibling nodes.
    I think we should also look for a name other than "divide".

@benface
Copy link
Contributor

benface commented Apr 17, 2020

@sir-kain gap-x and gap-y would be confusing because there are already gap, row-gap, and col-gap utilities: https://tailwindcss.com/docs/gap

@adamwathan
Copy link
Member Author

@benface:

Am I correct that the space and divide classes aren't really meant to be used together?

Correct, I can't see how you could ever really use these together or what the expected result would be really (would space-y-4 add 16px of padding on each side of the divider, or would it put 8px on each side, and what would happen at the very start and very end, no space?)

If you want to add space around items that have dividers between them, you'd use vertical padding on each item, just like you would if you were doing the borders manually 👍

When used on elements that contain inline items that wrap (including flex flex-wrap), space-x will add a space at the beginning of every new line.

Yeah this is an annoying one and in my opinion not something we could ever handle internally in Tailwind aside from using gap (when flexbox support is universal).

Example for those curious:

image

To me that's like a 1% scenario though, 99% of the time I just want to space things out in a single row or column (like links in a nav bar), so to block this feature just because of the wrapping thing seems unnecessary, especially because this doesn't break anything related to that situation, it just doesn't solve it just like adding ml-4 to all but the first item in a list of links doesn't handle wrapping either.

For cases where you need the wrapping behavior, you'd have to do the negative margin trick and build it sort of like a flexbox grid (I know you know this but leaving the example for others 👍 )

<div class="flow-root bg-blue-100">
  <div class="-m-2 flex flex-wrap max-w-md">
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">One</span>
    </div>
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">Two</span>
    </div>
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">Three</span>
    </div>
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">Four</span>
    </div>
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">Five</span>
    </div>
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">Six</span>
    </div>
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">Seven</span>
    </div>
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">Eight</span>
    </div>
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">Nine</span>
    </div>
  </div>  
</div>

The annoying thing about this is all the extra wrapper elements you need, which is fine when you actually have this wrapping situation and you have no other choice, but to always do this even when the content will never wrap just because it's the "lowest-common-denominator" solution is really over-engineering in my opinion.

If the first item is hidden, the first visible item will have unexpected extra space or a border before it.

Yeah this is another valid edge-case but again I don't think we have to throw away the idea just because it doesn't solve every possible situation. Long-term, gap will be our holy grail for this, but in the mean-time I'm hoping we can use the space utilities to get gap-like productivity for 95% of the scenarios we'd use gap, and resort to the old complicated workarounds only when it's insufficient. No matter what it's not worse than what we have to do now.

space-x doesn't work correctly on RTL languages

This is a very interesting point — I wonder if it would be weird to use inline-start and block-start for these utilities even though we don't use logical properties for anything else?

This also makes me realize this solution breaks when you do flex-row-reverse which sucks, and switching to inline-start doesn't fix that either. In fact I can't think of any really declarative solution that doesn't either a) require a bunch of wrapper elements, negative margins, and complexity, or b) work for both flex row directions.

I still think the utilities are very valuable but it is unfortunate that there are holes like this and makes me even more desperate for universal flexbox gap support 😩

One more thing I'm worried about — the increased specificity of these selectors didn't seem like a problem to me at first but now I'm wondering if they are a problem when doing responsive stuff, because if you do space-y-4 md:space-y-0 to sort of "undo" the spacing at the md breakpoint, it is still going to stomp over any mt-{n} utilities you try to use on those individual elements. It would definitely be ideal if we could just use > * + * because the specificity is the same as a regular utility.

Can anyone think of a concrete example where they would've been bitten by this?

@benface
Copy link
Contributor

benface commented Apr 18, 2020

Long-term, gap will be our holy grail for this, but in the mean-time I'm hoping we can use the space utilities to get gap-like productivity for 95% of the scenarios we'd use gap, and resort to the old complicated workarounds only when it's insufficient.

Unfortunately, not for flow layout (inline and block). I imagine this will be used a lot on regular, non-flex-or-grid layouts like this:

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

or even inline, like this:

<footer class="space-x-4">
  <a href="#">About Us</a>
  <a href="#">Contact Us</a>
  <a href="#">Privacy Policy</a>
</footer>

The CSS Working Group is not planning on making gap work for such layouts. Thankfully the first example can be converted to flexbox easily with flex flex-col, but the second one cannot if we want to allow wrapping in the middle of an item.

I agree with you that these shortcomings are not bad enough to block this feature. And for divide, I can't think of a better solution. But for space, if only margin-trim was supported, we could simply set the margin of all children without needing the negative-margin-on-container hack, and I think that would be a better solution. Unfortunately, that property is probably years away (the CSS module it was introduced in is still an "Editor's Draft").

This is a very interesting point — I wonder if it would be weird to use inline-start and block-start for these utilities even though we don't use logical properties for anything else?

Not weird, but we'd lose IE11 support. Which I suppose is fine for spacing if it's fine for transform (and I really think it is).

@tobiasthaden
Copy link

@adamwathan

Does margin-right instead of margin-left fix the unwanted space?

.space-x-1 > *:not(:last-child) { margin-right: 4px }

@adamwathan
Copy link
Member Author

The problem I’m trying to solve most is handling flex-row-reverse. I don’t think it can be done without switching to a variant based approach like children-except-first:ml-4, because you need to be able to explicitly use ml-4 or mr-4 based on your circumstances. Logical properties wouldn’t help in this case either unfortunately.

@florianbouvot
Copy link
Contributor

Space is not perfect but works great in a lot of case.
For complex use case, I think a grid component (using flexbox) is better.

What about adding this kind of component (like you do for forms)?

@hdodov
Copy link

hdodov commented Apr 25, 2020

The space-* utilities are clever indeed, but I'd personally prefer the negative margin "hack." I hesitate calling it a hack since negative margins are an actual feature of CSS, along with margin collapsing, so using negative margins could be boiled down to simply "using CSS."

I would argue that this:

.space-x-4 > :not(template) ~ :not(template) {}

...looks a lot more like a hack that a negative margin. Besides, it makes the selector quite large. Given that it's repeated for each space utility (and there are a lot of those), it could add up, even when minified.

Anyway, these are the reasons I think negative margin approach is better:

  • Not bothered by Alpine's <template> stuff
  • Not bothered by RTL (I think)
  • No need for the space-*-reverse utilities
  • Better in terms of specificity
  • Smaller footprint because :not(template) is not needed
  • Better support (not that IE6 matters, but still)

Comparing this:

.space-x-16 {
    margin-left: -1rem;
}

.space-x-16 > * {
    padding-left: 1rem;
}

...to this:

.space-x-16 > :not(template) ~ :not(template) {
    --space-x-reverse: 0;
    margin-right: calc(1rem * var(--space-x-reverse));
    margin-left: calc(1rem * calc(1 - var(--space-x-reverse)));
}

For half the CSS, it handles more use cases. And we have to agree that at least it looks a lot less hacky, without all of that :not(template) and var() logic.

The obvious downside is that you might need another DOM element, but you also might not. Besides, why is that considered bad anyways? Having an element with with just the space-* class could be good for readability/maintainability.


As @benface mentioned, though, margin-trim would be a way better solution, if it was supported. And I do realize that we're still free to not use space-* utilities. I just want to provide some opposition. 😀

@adamwathan
Copy link
Member Author

The negative margin/padding approach has a few problems:

  • It doesn't work automatically with reverse order (padding-left is wrong when the items are reversed, needs to be padding-right), so would still need the extra class and the CSS variable stuff
  • It breaks if the child elements already have padding (like a list of buttons, extremely common)
  • Negative margin affects the layout outside of the parent, which I worry could cause horizontal scrolling issues

To me the need for wrapper elements to avoid the padding problem is a complete deal-breaker, just not worth it when all you are trying to do is put a bit of space between items that already exist.

This is the CSS you'd actually need because of the reverse problem:

.space-x-16 {
    --space-x-reverse: 0;
    margin-right: calc(-1 * 1rem * var(--space-x-reverse));
    margin-left: calc(-1 * 1rem * calc(1 - var(--space-x-reverse)));
}

.space-x-16 > * {
    --space-x-reverse: 0;
    padding-right: calc(1rem * var(--space-x-reverse));
    padding-left: calc(1rem * calc(1 - var(--space-x-reverse)));
}

The size thing isn't a really serious problem, compression algorithms love duplication so the fact that > :not(template) ~ :not(template) is repeated many times doesn't really matter in practice.

Super appreciate the input and feedback but no plans to change the implementation of this, you can instead disable these plugins if you want and replace them with your own implementations using the plugin system.

@MichaelAllenWarner
Copy link
Contributor

<div class="flow-root bg-blue-100">
  <div class="-m-2 flex flex-wrap max-w-md">
    <div class="p-2">
      <span class="rounded-full bg-blue-500 text-white px-3 py-1">One</span>
    </div>
   ...
  </div>  
</div>

@adamwathan

Thanks for this.

I've never seen display: flow-root used in this context—only overflow: hidden, which sometimes causes problems with focus-outlines getting cut off.

I'm wondering: do you know whether flow-root is really as robust a solution as overflow-hidden is in these scenarios? I've searched around a bit and haven't found any discussion of this elsewhere.

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.

9 participants