Explore not resetting `transition-property` to `none`
benface opened this issue · 12 comments
Right now, the base styles set transition-duration to the default duration and transition-property to none. As a result:
transition-allapplies a transition with the default durationtransition-all transition-500customizes the durationtransition-opacitycustomizes the property but applies the default durationtransition-opacity transition-500customizes both the property and the durationtransition-500alone doesn't do anything (the property is still set tonone)
However, this creates issues with 3rd party libraries that don't expect transition-property and transition-duration to be different from their CSS-defined defaults (all and 0s, respectively).
It could be interesting to not have any base styles, unless the user customized the default transitionProperty (which would be all by default), the default transitionTimingFunction (ease by default), and/or the default transitionDelay (0s by default). So it would then work like this:
transitionapplies a transition with the default durationtransition-500customizes the durationtransition transition-opacitycustomizes the property but applies the default durationtransition-500 transition-opacitycustomizes both the property and the durationtransition-opacityalone doesn't do anything (the duration is still set to0s)
This would obviously be a breaking change, and I'm not convinced it's better. Is it clear that transition sets the transition-duration property, so that if you want to customize the transitionable property, you need to add a transition-opacity class and not just append -opacity to the existing transition class? Curious to hear others' thoughts!
@benface - i have read this a few times, and the more I read it the more conflicted I have become.
However, I do still feel that it would be correct to not set the default transitionProperty unless it is explicitly set by the user, this maintains the original/default value from CSS so it is unlikely to have issues with third parties and other code, by the fact the default value remains unchanged.
However, perhaps the introduction of a default value for transitionProperty is the way to go, as then a user can explicit set this default value if they want it to be none, etc.
At the moment the defaults are available/set for transitionDuration, transitionTimingFunction, transitionDelay so I think it would be correct in extending this to transitionProperty as well.
That's just my opinion on the approach.
@terryupton To me, making transitionProperty support a default key and using that value in the base styles doesn't make sense unless we remove the ability to set a default duration, because then it would be possible — and quite easy — to accidentally set every single element on the page to transition all of its properties (by having both a default transitionProperty that is not none and a default transitionDuration that is not 0s), and there's simply no case where that would be desired.
Also, even though customization is generally good, I feel like this aspect of the plugin should be opinionated so that its usage is roughly the same on different projects using it. Your suggestion would make it possible to completely alter, on a project basis, which type of utility is needed for a transition to be applied. If a default, non-0s duration is set but the default property is set to none, then only a property utility would be needed on an element (e.g. transition-all) to have any kind of effect (this is how it works right now), but if a default, non-none property is set and the default duration is set to 0s, then only a duration utility would be needed on an element (e.g. transition-500). I'm not saying that's bad, but I'd rather the developer experience be consistent from a project to another. Also in the latter case, I feel like we should still be able to set a default duration, otherwise a project that uses transitions extensively would be full of, say, transition-500 classes which looks like a magic number. Sure, the key for the 500ms value could be default, and then the class would be transition-default... but that name could be confusing, what does "default" refer to? Is it a property, a duration, a timing function, or a delay?
That's actually a concern I have in the change I'm proposing: it's not obvious what a class named transition does. It's not an issue until you want to customize one aspect of the transition, like the property or the duration. Then you wonder if you need to add a new class, or change the existing class. That's not an issue with the way it works now, because to apply a transition, we need to explicitly set the property to apply it to, even if it's all (transition-all). Actually, there was in v2.0.0-beta.1 a way to set a default transition property which created a simple transition class (and it defaulted to all of course), but I removed it in v2.0.0-beta.2 for exactly that reason: it had the potential to cause confusion, for very little benefit (-all is just 4 characters).
Hopefully my dilemma is understandable haha...
I also thought about making the base styles less likely to affect 3rd party libraries by only applying them to elements that have a transition- class:
[class^="transition-"], [class*=" transition-"] {
transition-property: none;
transition-duration: 250ms;
}
But that would completely break @apply support. :/
I'm leaning towards not changing anything, and just documenting it better (explain how to deal with library conflicts, etc.)... unless someone changes my mind!
Thinking about this some more... There is a precedent in Tailwind that may justify this change: the border class, which sets the border-width property (and not border-color). If you want to change the color, you have to add a class (just changing border to border-gray doesn't work because the border width stays at 0), and if you want to change the width you have to replace the border class with border-2 for instance. So it's kind of the same principle as the proposed transition class which would set transition-duration to the default duration. It's slightly confusing (perhaps more so than border), but I could live with that. Still waiting to hear more opinions to make a decision though.
(Interestingly, border also requires base styles applied to all elements to work properly.)
A couple of wasted days later, I finally narrowed down a bug/conflict with another third party lib to this. This one was actually quite difficult to debug, prior to the previous, as this library might have been trying to read the transition-property in it's javascript and modifying it's logic based on that - so it wasn't apparent in just looking at the styles to an element that there's any conflict.
Using the wildcard selector seems like it should be an opt-in thing, which one would do if they want to use @apply knowing the caveats that it could conflict with 3rd party libraries - otherwise, your [class^="transition-"], [class*=" transition-"] suggestion seems ideal.
@benface I'm confused as to why, in order for a transition-duration default to be set, a transition-property of none has to be explicitly declared. I'm sure I'm missing something, but I can't fathom what.
If setting the transition property is required in order to use our transition utility classes (which it currently is) - the default property of none is literally doing nothing for us except interfering with 3rd parties. I gotta be missing something.
@chasegiunta I apologize for all the trouble this plugin is giving you, but thank you for the feedback. It's very useful.
UPDATE (September 1, 2019): I changed course and this wall of text is not relevant anymore, read my latest comment below!
I've put a lot of thought into this, and I think I came up with a solution that I like. I just released v2.1.0-beta.1 so I can gather feedback from users (including @terryupton :)). The release notes should explain what changed and the README of the next branch has been updated, but I'll try to explain the rationale.
I realize that having base styles that change the default transition-property and transition-duration for all elements is very opinionated, in the same way that Tailwind's Preflight resets border-width (from medium to 0), border-style (from none to solid), and border-color (from currentcolor to whatever default border color is set in the theme) so its border utilities are simpler to use.
In most of my projects personally, I like to use the same duration for most transitions/animations, so it would be annoying to have to repeat it every time (e.g. transition-opacity transition-250, transition-transform transition-250, etc.). In v1 of the plugin, the transition-[property] utilities also set the default duration, but that broke things like the following (even though it's an edge case): <div class="transition-opacity transition-500 sm:transition-transform"></div> (transition-500 would be unexpectedly overridden by the default duration on sm+ screens). So for v2 I switched to the base styles approach, with transition-[property] utilities that only set the transition-property property as they should. The only thing is that if I set a transition-duration on all elements, I have to set transition-property to none, because its default value (in CSS) is all. I think their reasoning for that was: you should be able to set the transition-duration property and have it do something, instead of requiring two properties for an effect. But so it means that setting transition-duration to the default duration on all elements without setting transition-property: none would make all the elements on the page have transitions on all their properties, without even adding a utility class. That's obviously not what we want... so hopefully that answers your question about that @chasegiunta.
But it made me think: maybe some users don't care about the default duration, and explicitly set a duration every time. I also started thinking how the default default value (250ms) is opinionated; wouldn't it make more sense for it to be 0ms, and let users specify the default they want? And if the default duration was 0ms (which is the same as the default transition-duration in CSS by the way), there would be no need to reset transition-property anymore... which means no need for base styles anymore. 😄 I still want to support having a default transition duration even if it's just for me (and the base styles remain the best implementation I can think of), it just doesn't have to be the default. In fact, with the reports that it caused issues with other libraries, it shouldn't be the default. So that's why I changed the default transitionDuration to 0ms, and why base styles are now only generated if you set it to a non-zero value (or set a default transitionTimingFunction other than ease, or even a default transitionDelay other than zero although I have no idea why you'd do that).
As for the reasoning for adding the ability to set a default transition property despite refusing that exact suggestion from @terryupton earlier in this thread (sorry haha), it comes down to needing a way to communicate the "magic" going on with the transition-property: none base style; that is, we only include it if the default transitionDuration is non-zero (for the reason explained above). So I came up with an auto value for the default transitionProperty (which it is set to by default) that resolves to all or none depending on the default transitionDuration (and when it resolves to all, it actually doesn't generate anything because that's the CSS default). So there's that, but also, what if the user knows better and wants to set the default transition property to none all the time, even though they haven't set a default transition duration? Or what if they want to set it to some specific property like opacity? Well now they can! 😄
I know it's not a perfect solution by any means, but hopefully it's a bit better / more flexible than it was. Let me know what you think!
(Just realized I should probably call it v3.0.0 if/when I release it, because there's a couple breaking changes...)
@benface Had the chance to try out your beta release. For one, it doesn't conflict with these 3rd party libraries 🎉 🎉 🎉 . I will say it took me a few minutes to figure out why nothing was working whenever I set my own transition durations. It's now clear that the transition property also needs to be set if you're going to customize your durations, which is fine, but it needs to be very apparent in the documentation.
This is completely subjective, but I'm also not enthusiastic about the descriptive scale, for the same reasons why I personally don't love that scale type for all tailwind properties (more guess work, harder to wedge in your own values).
Ah, I spoke too soon. I didn't realize that it's still adding a wildcard * { transition-duration: [value] } property if you set a default - which means everything automatically has a transition property of all and you also run into this absolutely crazy webkit CSS bug that's still present from 9 years ago https://bugs.webkit.org/show_bug.cgi?id=46041
(This doesn't actually have any transition classes set, this is just what the default hover interaction looks like after setting a default duration & property)
After a lot of brainstorming with @chasegiunta (thanks Chase!), I decided to change course and revert pretty much all the changes from v2.1.0-beta.1... sorry for anyone who had to read the above. Here is a new beta version that I'm even more confident in: https://github.com/benface/tailwindcss-transitions/releases/tag/v2.1.0-beta.2
Yes, it will be called v2.1.0 after all, because there shouldn't be any breaking change. Even though the default default transition duration of 250ms is slightly opinionated and I kinda like the descriptive scale, I decided that it wasn't worth breaking projects that rely on the default config. And even with no change to the API, I managed to get rid of the base styles that reset transition-property to none and transition-duration/transition-timing-function/transition-delay to the default duration/timing function/delay (if set), all thanks to custom properties! There is even a fallback for IE11 (which doesn't support custom properties) that should work 99% of the time; the only "gotcha" is that if you change the transition property on a breakpoint AND you customized the duration, you have to reset the duration at the same breakpoint otherwise it will be reset to the default, e.g.:
transition-bg transition-100 sm:transition-color
should be:
transition-bg transition-100 sm:transition-color sm:transition-100
But that's only if you need to support IE11, and let's be honest, transitions are usually not critical.
I'd be really thankful if everyone who had issues with base styles conflicting with other dependencies could test this new version, and let me know what you think. Thank you!
Ok so I've spent some time testing and I really thing it's a solid solution, so I went ahead and released v2.1.0 with these changes.
To recap: there are no breaking changes to the API, so you can still set a default transition duration, add a transition-[property] class to an element (e.g. transition-all), and it will apply a transition with the default duration, all without base styles that mess with the default values of CSS properties. In addition, a single transition-[duration] class (e.g. transition-500) now applies a transition as well, as if transition-all was also on the element.
Closing this issue since I am, in the end, no longer "resetting transition-property to none".