tailwindcss
tailwindcss copied to clipboard
tailwindlabs
Add a 'not-' variant for every pseudo variant
This PR adds a not- variant for every pseudo-class variant (plus open:), including the group- and peer- ones (e.g., not-focus-within:, group-not-first:, peer-not-open:).
In addition to containing many new tests for the not- variants in tests/variants.test.html, this PR:
- moves the
backdrop:shadow-mdtest from the pseudo-class area to the pseudo-element area; - fixes a typo (
xl:dark:disabledshadow-md->xl:dark:disabled:shadow-md) and adds a corresponding test; - adds a missing test for the
peer-open:variant.
Hey thanks for this! My only hesitation on merging is around the API — I've always sort of dreamed of coming up with a way to make variants somehow composable but never landed on the right notation.
Random example that I don't think is necessarily great but communicates the idea:
<div class="not(hover):underline">
<div class="group(hover):underline">
<div class="group(not(hover)):underline">
My biggest fear is I merge this and release it and then 30 seconds later I finally have the lightbulb moment for a more composable syntax 😅 Any thoughts? With the JIT engine it's not so bad if we ever come up with a new syntax and have to keep the old stuff around but still hesitant to push further in one direction if there's a chance there's a better path.
@adamwathan wrote:
Hey thanks for this! My only hesitation on merging is around the API — I've always sort of dreamed of coming up with a way to make variants somehow composable but never landed on the right notation.
Random example that I don't think is necessarily great but communicates the idea:
<div class="not(hover):underline"> <div class="group(hover):underline"> <div class="group(not(hover)):underline">My biggest fear is I merge this and release it and then 30 seconds later I finally have the lightbulb moment for a more composable syntax 😅 Any thoughts? With the JIT engine it's not so bad if we ever come up with a new syntax and have to keep the old stuff around but still hesitant to push further in one direction if there's a chance there's a better path.
Something like that would be pretty great, too. Would it actually conflict with the idea in this PR, or is the concern more that you'd end up with some redundancy (and the small performance hit that would bring)?
Also, I haven't looked closely yet, but I bet this PR will need a little tweaking to bring it in line with the changes here: https://github.com/tailwindlabs/tailwindcss/pull/8394/files#diff-cf9185e083748e39c6940d3ad337df23b0ecbbd70b9550f596de7cf4b4668bcfR133-R155
Would it actually conflict with the idea in this PR, or is the concern more that you'd end up with some redundancy (and the small performance hit that would bring)?
Yeah no conflict just don't like having two ways to do things or having to keep around old code for backwards compatibility if we come up with a better approach.
What are some really practical examples of needing this by the way? I haven't found myself reaching for it but of course think it's sensible to want it, but would be good to capture some real world examples where this would lead to simpler code than what you would have to do otherwise.
Would it actually conflict with the idea in this PR, or is the concern more that you'd end up with some redundancy (and the small performance hit that would bring)?
Yeah no conflict just don't like having two ways to do things or having to keep around old code for backwards compatibility if we come up with a better approach.
What are some really practical examples of needing this by the way? I haven't found myself reaching for it but of course think it's sensible to want it, but would be good to capture some real world examples where this would lead to simpler code than what you would have to do otherwise.
Hi @adamwathan one of the examples can be: not-first: or not-last:, but thinking about it again I think it can be useful with html tags or ids or even classes: example not-[p]: not-[#foo]: not-[.btn] 🤔
@adamwathan
Yeah no conflict just don't like having two ways to do things or having to keep around old code for backwards compatibility if we come up with a better approach.
Makes sense.
What are some really practical examples of needing this by the way? I haven't found myself reaching for it but of course think it's sensible to want it, but would be good to capture some real world examples where this would lead to simpler code than what you would have to do otherwise.
I think @bacali95 is right that not-first:/not-last: would be one of the more common use-cases. Not particularly important to me personally, as I'm usually happy using something like Twig's loop variable, but not all templating engines have as convenient a way to work with a loop's index.
Where not- variants would really shine is when handling an element's state in a way that involves stacked variants. The use-case that inspired this PR, for example, was a sticky-nav that should disappear on scroll-up and reappear on scroll-down. So I made a custom concealed: variant, used some JavaScript to add/remove the .concealed class from the nav in response to user-scrolling, and gave the nav a class something like concealed:-transform-y-full. But that wasn't enough, because keyboard-users should still be able to Tab their way to the nav when it's concealed, at which point it should become visible. I could have done this without a not- variant, but the natural solution here is clearly concealed:not-focus-within:. Rolled my own not-focus-within: and then made this PR.
Another use-case for a not- variant came up just last week. This one had to be an entirely custom variant anyway, so it's not directly relevant to the PR, but it demonstrates the same principle (i.e., that while forgoing a not- variant is probably always possible, doing so can be inconvenient and result in less readable code when you're stacking state-related variants). In this scenario I was implementing a scroll-shadow for tables with horizontal-overflow. From the JavaScript, I was controlling state with a .has-x-overflow class and with .is-scrolled-left/.is-scrolled-right classes. I had a left-shadow element and a separate right-shadow element, each of which should have opacity-0 by default but then opacity-100 if the table has overflow (one piece of state) and if the scroll-position inside the table is not all the way to the corresponding side (second piece of state). So I had one custom variant like has-overflow:, and two others like not-scrolled-left: and not-scrolled-right:. That way I could do has-overflow:not-scrolled-left:opacity-100 instead of has-overflow:opacity-100 has-overflow:is-scrolled-left:opacity-0.
What order do you think the variants should be registered in? The registration order determines which styles "win" when multiple are added, for example:
<div class="hover:bg-red-500 not-hover:bg-blue-500 focus:bg-pink-500 not-focus:bg-orange-500">
What color do you think this should be when you hover it or focus it? The way things are implemented in this PR currently, the hover and focus styles won't work I don't think, because not-focus is going to take precedence over hover. Should all of the not ones come first? Or does it need to be more complicated than that?
Another even simpler (but maybe harder, haha) question, what color should this be:
<div class="not-hover:bg-black not-focus:bg-white">...</div>
Should that be considered user error with a lint warning, like we do for this?
<div class="bg-black bg-white">...</div>
Is there a valid situation anyone can think of where it makes sense to add two not- classes that both set the same CSS property at once?
Posted it to Twitter as well: https://twitter.com/adamwathan/status/1530948314107199488
I expect the answers there will be 99% people who haven't thought about it hard enough but sometimes some wisdom appears if you dig enough 😅
😬
So I guess there are two related implementation questions here:
- In what order should the
not-variants be registered in relation to the existing variants? - In what order should the
not-variants be registered among themselves?
Pretty tricky. Strikes me as one of those things that will maybe ~never matter in practice for anyone, but you never know, and clearly decisions must be made if the not- variants are to be incorporated.
I suspect your instinct is right to register all the not- variants before the other variants. My reasoning is simply that the not- variants are less "important" and should therefore always lose specificity-ties to non-not- variants. And if a hover: or focus: class suddenly stops working (because some not- class were introduced), it could easily escape the developer's notice, resulting in a bug that makes it to production.
Possible counterarguments...
Are developers likely to expect not- variants to win specificity-ties? And if so, would thwarting that expectation be as consequential as the "hover-state silently stops working" problem mentioned above?
I'd bet that an awful lot of developers wrongly assume that :not() by itself adds specificity. So yes, I suppose that many developers would expect not-hover: to beat focus:. What happens when they're wrong? Hard to say, but I think probably a developer is likely to reach for the not- stuff after they've already taken care of basic states like hover: and focus:, so maybe their intended not- effect just doesn't work and they have to figure out why... which strikes me as better than the sudden silent failure of a basic hover-state that was already developed and working.
As for the order of the not- variants among themselves, I think maintaining the same order as their non-not- counterparts would be the way to go. I considered the exact opposite order momentarily, but it doesn't seem right to me: if focus-state is regarded as more important than hover-state, then regarding not-hovering as more important than not-focusing implies that the "off" condition somehow doesn't count as "state," if you follow. Not sure if I'm approaching this from the right angle, but that's what I've got for now.
To sum up, I think the not- variants should all be registered before the non-not- variants, and I think among themselves the not- variants should be registered in the same order as their non-not counterparts.
Good thoughts thank you! We had the same thought about "should things be in this perfectly reverse order" but couldn't come up with any real example where you could clearly justify why it mattered.
What about the order of the group and peer buckets? Should it be:
- Base `not` variants
- `group-not` variants
- `peer-not` variants
- Base variants
- `group` variants
- `peer` variants
Or:
- `peer-not` variants
- `group-not` variants
- Base `not` variants
- Base variants
- `group` variants
- `peer` variants
I'm also realizing now maybe the group and peer stuff are actually in a bad order already too — this demo seems bad to me:
https://play.tailwindcss.com/8dsV6plUrt
Is there a valid situation anyone can think of where it makes sense to add two not- classes that both set the same CSS property at once?
Not off-hand, no, but even if there are valid use-cases for that sort of thing, the result you end up with (when both not-conditions are fulfilled) will, in the case of a specificity-tie, necessarily depend on the order in which Tailwind has registered the variants. That makes me think a lint warning might be called for, because what are the chances the user is actually aware of Tailwind's internal implementation here? But if the lint-warning weren't limited to specificity-ties, then I suppose you could end up with false alarms.
I'm also realizing now maybe the
groupandpeerstuff are actually in a bad order already too — this demo seems bad to me:https://play.tailwindcss.com/8dsV6plUrt
Would switching the registration order of hover and group-hover actually make a difference there, though? Isn't that one just a matter of specificity?
the result you end up with (when both not-conditions are fulfilled) will, in the case of a specificity-tie, necessarily depend on the order in which Tailwind has registered the variants
Yeah so this is already the case for this too right:
<input class="hover:border-gray-400 focus:border-blue-400">
If the element is hovered and focused, the border color is determined by the order we have registered the variants, and we register the variants in a very intentionally designed order to make the results in these cases as desirable as possible, which is the same thing I'm trying to do for the not variants here.
Would switching the registration order of hover and group-hover actually make a difference there, though? Isn't that one just a matter of specificity?
Yep 100% true, realized that right after I posted! So we would want to refactor to :where here eventually if we wanted to "fix" this too.
Would switching the registration order of
hoverandgroup-hoveractually make a difference there, though? Isn't that one just a matter of specificity?
And I guess if that's right, then it doesn't really matter whether the base not- variants are declared before or after the group-not- and peer-not- variants (though the order of group- and peer- is conceivably relevant).
Related tangent: in some future release (but please not too soon for the sake of those who still have to support Safari < 14), it might be worth considering using :where() for the group- and peer- modifiers, to bring the specificity of those utilities back down to class-level.
I'd like to mention this should also apply to the :is and :has pseudo functions. Care especially for the, somewhat experimental, :has pseudo which adds different syntax altogether.
The PR now registers the not- variants first (this required reordering things in tests/variants.test.css). It also has not- tests for the new optional: variant and for hover: when hoverOnlyWhenSupported is enabled.
Marked as draft because the PR's current group-not- and peer-not- implementation doesn't support the new nested-modifier syntax (like group-not-hover/sidebar:opacity-75).
@MichaelAllenWarner I opened a PR that brings your PR up to speed with the latest Tailwind core changes, adding support for dynamic group/ and peer/ variants and updating your impl to more closely match the new matchVariant impl currently used in TailwindCSS.
⚠️ NOTE: I don't have the means at the moment to actually test my changes, but I think they should work, so if you can test them first and ensure they work, this might be all we need to successfully merge.
This PR can be found here: https://github.com/MichaelAllenWarner/tailwindcss/pull/6
Hi @adamwathan,
With recent changes to Tailwind (named-modifier syntax for group- and peer- that uses matchVariant() instead of addVariant() internally, the introduction of data- and aria- variants [which are also arguably "state-related"]), my hunch is that the question of adding not- functionality is more complicated than it was previously, and that implementing it well (with due consideration of the variant-ordering we discussed upthread) would be a task better suited for someone more knowledgable about Tailwind's internals than I am.
~~Would you object if I closed this PR?~~
[EDIT: Since @brandonmcconnell has taken the initiative here, I'm now inclined to leave this open.]
@MichaelAllenWarner (cc @adamwathan, @RobinMalfait) I have adjusted my PR to correct the sort order of the selectors, and all tests now pass as expected and follow the intended specificity.
The issue in my tests previously was this:
| Before | After |
|---|---|
|
|
Everything has been updated to match the latest spec using matchVariant like the current core implementation.
I've also added support for…
Non-not variants |
not variants (new) |
|---|---|
supports- |
not-supports- |
aria- |
not-aria- group-not-aria- peer-not-aria- |
data- |
not-data- group-not-data- peer-not-data- |
dark |
light**explanation/rationale below |
screen**explanation/rationale below |
Re the light and screen variants
I'm sure that at some point, there was an intentional reason why light and screen variants were not added, likely because they are the defaults whose styles and can then be overriden by styles specific to dark and print, respectively.
That said, I first implemented both of these as not-dark and not-print, but taking cue from the implementation of motion-reduce and motion-safe, I thought it made the most sense for these two to be implemented with their native CSS names light and screen (rather than not-dark and not-print), though I can certainly rename these back to not-dark and not-print if the TailwindCSS team would prefer that.
All that said, I believe this PR will be 100% up to speed once my stacked PR (https://github.com/MichaelAllenWarner/tailwindcss/pull/6) is merged into it.
@RobinMalfait Could you please review my stacked PR (https://github.com/MichaelAllenWarner/tailwindcss/pull/6) and let me know in that PR if there is anything I'm missing, be it missing variants, ill-crafted tests, or otherwise?
I've also added tests for all of these new variants (and those missing— aria- and data-) via commit 314f566 in my PR (linked in my prior comment above.
Hey folks! I think having first-class :not support in Tailwind would definitely be valuable but this PR has been sitting for almost a year now and we're still not ready to give this feature the attention it would need to confidently pull it in to the framework so I'm going to close this for now.
As you guys have discovered talking it through, there is a lot to think about here in terms of how sorting should work, how things should override each other, what should happen when you stack things, how the APIs should work, etc. and it's not the sort of thing I can give real feedback on without really starting from the beginning and putting a lot of attention into understanding how all of this should work and what the consequences are of any given approach. As much as I wish I could just give drive-by feedback here and have you guys take it over the finish line it's just not really possible with significant features because of how careful we need to be to avoid making a bad decision with an API that leads to us needing to make a breaking change to correct it.
The reality is that any time we get a PR for anything we've already got lots of our own prioritized work going on and we can't drop what we're focused on to prioritize a feature that comes in from a contributor. We can only really justify putting our attention on a complex PR when that feature is at the top of our own priority list, and this one just hasn't made it there yet.
I do imagine we will pick this up eventually and when we do I'd definitely love input from you guys on the best way to make it work, and I'll definitely include you as co-authors on the commit even if we start on it fresh ❤️
Sorry to disappoint at the moment — please don't see closing this as a rejection of the idea, we're simply just not ready to tackle it right now and I think it's better to be direct about that instead of leaving it hanging open forever and leaving people wondering why we're ignoring it or when we're going to get to it.
@adamwathan Makes sense, and I'm certainly not disappointed—I submitted this PR uninvited and with no expectations. Something like this would be a useful feature, but of course if it's implemented then it should be implemented carefully and correctly. Thanks for the note!
@adamwathan cc @MichaelAllenWarner Sounds good! Yeah, my PR did bring this up to speed with the latest changes from v3.2, but there definitely are quite a few things to consider regarding where and how to implement variants such as these to ensure they stay at a lower specificity.
I was thinking that it might even make sense to add a new @tailwind not-variants layer for not variants like these, which would also allow users and plugins to add such variants using methods like addNotVariant and matchNotVariant. However, I understand that adding such changes might have great implications for the future of Tailwind, and now may not be the best time to make such a change.
Thanks again, and I look forward to collaborating on this idea again in the future.
@adamwathan I find myself reaching for not- variants all the time. Think this could make the v3.3 release in a couple of weeks? My latest PR version (built atop @MichaelAllenWarner's great PR here) is 100% ready for the latest version(s) of TailwindCSS and includes tests.
The only missing piece of the puzzle left to figure out is how to expose the not- variants in such a way as they get added before non-not- variants for better specificity and allow users to do the same.
One solution, which I mentioned in my previous message in this would be to introduce a new @tailwind layer for this and not- counterpart functions for addVariant and matchVariant— addNotVariant and matchNotVariant which would use that namespace.
—
Were you still considering an alternative syntax for this, like not(hover):, and if so, would such a syntax be able to consume multiple variants, like not(hover:focus):?
cc @MichaelAllenWarner @RobinMalfait