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 'inherit' to default color palette #2706

Closed
wants to merge 1 commit into from

Conversation

iksaku
Copy link
Contributor

@iksaku iksaku commented Oct 30, 2020

Basically, what I called on #2622, so PR wasn't that big deal.

What this allows is for component-type implementations to provide a "default styling" while still giving the end developer the ability to "modify" the colors they want to use.

This can get kind of hard sometimes, since classes like border-gray-500 have a greater priority to border-gray-300 in the default config file.

Headless components sure are a big help for this, since you can call for whatever styling you want, but for pre-styled components, well, I think the point is clear.

Here is an example Codepen demonstrating a potential use case while working with pre-styled components.

@adamwathan
Copy link
Member

Still struggling to understand the big benefit of this as a default, because I completely deleted the text-inherit class from your demo and it behaves exactly the same way:

https://codepen.io/adamwathan/pen/YzWabdN

@iksaku
Copy link
Contributor Author

iksaku commented Oct 31, 2020

Oh, I made an error with the .current class name, sorry 😅 , here's another pen, in which I try to better demonstrate the difference between using currentColor and inherit for color properties.

Say you want to allow your component to have a "set of default coloring", like a bg-black with a text-white, but also want to allow the end developer to be able to change those sets of colors. CSS proprotiy based on which rule is present first will obviously get in the way, so you nest your components in a "non-rendering" div (using display: contents).

Now, if we use the currentColor variable, whenever we change the color of the text, it will also change the color of the background... This is due to .text-* classes having a higher priority in Tailwind by default. So, using bg-current will only get in our way, as seen in the new Pen.

If we, instead of using currentColor use the inherit value, then the color property will only inherit its value from the same parent property, so, when mixing text and background colors, background will only inherit from parent background definitions, while text will keep working as it has from time to time.

I'm not sure there's a way to prevent generating a .text-inherit class, since it may not be needed at all (text color will be inherited everywhere if there's no other rule), but for border and background colors, this will indeed help.

@hacknug
Copy link
Contributor

hacknug commented Nov 13, 2020

Still struggling to understand the big benefit of this as a default

Imagine a fixed-height card with a sticky header. Card has .bg-white and header uses .bg-inherit to always stay in sync. No need for custom properties here and you support ancient browsers.

Here's a different example using this pattern to get bleeding sections that match the color of their parent container (sections can use any color of the palette): https://koahealth.com/

adamwathan added a commit that referenced this pull request Sep 25, 2021
Re-implementation of #2706.

Co-Authored-By: Jorge González <[email protected]>
@adamwathan
Copy link
Member

Created a new PR for this (with @iksaku as a co-author) here since this has gotten a bit out of date: #5597

Going to close this one in favor of that one, thanks.

@adamwathan adamwathan closed this Sep 25, 2021
RobinMalfait pushed a commit that referenced this pull request Sep 26, 2021
Re-implementation of #2706.

Co-Authored-By: Jorge González <[email protected]>
adamwathan added a commit that referenced this pull request Sep 26, 2021
Re-implementation of #2706.

Co-Authored-By: Jorge González <[email protected]>

Co-authored-by: Jorge González <[email protected]>
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.

None yet

3 participants