Class merge issue with tailwind colour extensions
GorillaCoffee opened this issue · 9 comments
Describe the bug
The classes/1 method (or matter of fact, merge/2) eagerly overrides classes sharing the same prefix when the array passed in argument contains a class referencing a custom colour.
To Reproduce
Have a colour extension set up in the tailwind.config.js of your project, such as:
module.exports = {
theme: {
extend: {
colors: {
primary: require('tailwindcss/colors').gray,
}
}
},
}Then call, for example :
> Tails.classes(["text-md", "text-primary"])
"text-primary"The "text-md" class is no longer present.
The issue seems to present itself in other cases, such as:
> Tails.classes(["border-2", "border-primary"])
"border-primary"Expected behavior
In the same example above, the "text-md" class should be present in the returned string :
> Tails.classes(["text-md", "text-primary"])
"text-md text-primary"Runtime
- Elixir version: 1.14.4-otp-25
- Erlang version: 25.3
- OS: MacOS Monterey 12.6
- Tails version: 0.1.8
Additional context
Let me know where I can help. Thank-you!
🤔 this is pretty interesting. I'm going to have to look into what we could do here, but the basic issue is that we expect colors to be done using the method illustrated here: https://github.com/zachdaniel/tails#colors
That way, we know what your custom colors will be and can include them in the merge logic. Unfortunately, you couldn't do require('tailwindcss/colors').gray in that case. We'd probably need to support some kind of separate config if you want to do it that way, i.e config :tails, custom_colors: ["primary"]
I have the same issue. I did read about the colors config file, but I don't really want to do that. tailwind.config.js works perfectly fine and I feel like introducing a json file to declare my colors would be a step backwards.
What I don't understand, is that it works perfectly for bg and border.
The basic conflict comes down to the fact that, for merging logic, we have to know when the suffix refers to a color (i.e border-primary) or something else (i.e border-2). bg doesn't have a catch-all numeric size variant that could conflict with a custom color.
in the case of
border-[#243c5a]
border-[10px]
we have to know which one we're talking about.
We don't currently have an "is size or is color" parser to distinguish these, and technically a perfect one can't be written. Because you could make a color called 2 and then say border-2. I'm not sure what would actually happen if you did that, but the point is that it's not a super easy fix. We can fix the non custom value version by detecting numbers (and a special carve out for divide-x-reverse and divide-y-reverse.
However, detecting custom values like that would require a size parser and/or a color parser (whichever is easier, probably size as it may just be "ends with px/em" or something).
Okay, I was wrong, it's easier to match on custom colors, as that can be reasonably done with #, rgb(, rgba(, hsla(, currentcolor and one of the predefined CSS colors like black.
....and we already have logic in to do exactly that for custom colors :) So it is perhaps sufficient for 99% of cases to just add matching for directional values starting with an integer and then we can wash our hands of it. As long as custom colors don't start with a number, I think this covers the cases.
Okay, so I've made some changes that alleviate some of the issues, but not all. Without knowing that "some class we don't recognize" is a "custom color", we can't properly merge them. i.e
classes(["bg-specialblue", "bg-specialred"]
would produce "bg-specialblue bg-specialred". What I will add is a configuration option that defaults to false (for backwards compatibility purposes at a minimum) to assume that any remaining non-parsed classes that start w/ a prefix that contains colors is a color.
sigh, so an example of an issue that arises here is us not being able to tell the difference between a custom font size utilities and custom text colors.
For example, with text-blorange and text-really-big. Without knowing a constant set of values for one of those, it's pretty much dead in the water. And since colors are globally used, it makes more sense to configure those statically IMO. the colors file is just one way of doing that.
I've made some changes that will make it behave better with custom colors that aren't statically defined, but I've also added some docs to show examples of what we can't figure out without a static set of custom colors provided to us. If we want to do this without the colors file, then we'd need to ship this with a js runtime or call out to node and eval the file or something, which is obviously not going to happen :)
I'm going to close this for now, because I can't think of anything we can do beyond requiring a colors.json file. If someone can think of a way past the example I documented without requiring a colors.js file then we can talk about that. It is possible that we could hard-code the default set of things like font size utilities, and make those configurable, but we'll always need to know one or the other statically.
relevant documentation that was added: https://github.com/zachdaniel/tails/blob/main/README.md#colors