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

use text-decoration-line instead of text-decoration #6378

Merged
merged 2 commits into from
Dec 13, 2021

Conversation

o-t-w
Copy link
Contributor

@o-t-w o-t-w commented Dec 10, 2021

Currently the following code would not lead to the underline being green, which is a bug imo: class="decoration-green-700 hover:underline hover:underline-offset-4"

This is because text-decoration is a shorthand for text-decoration-line, text-decoration-color, text-decoration-style, and text-decoration-thickness so overrides any of those values (if they have been set) to initial.

src/corePlugins.js Outdated Show resolved Hide resolved
src/corePlugins.js Outdated Show resolved Hide resolved
@lukewarlow
Copy link
Contributor

lukewarlow commented Dec 12, 2021

Worth being aware this could technically be classed a breaking change.

@reinink
Copy link
Member

reinink commented Dec 13, 2021

@o-t-w I like this change.

@lukewarlow can you give an example of how this could break things? Seems pretty unlikely, but maybe I'm missing something?

@lukewarlow
Copy link
Contributor

@reinink I agree it's unlikely and someone would have to be relying on what I would agree is buggy behaviour but technically someone could be relying on the fact that text-decoration does override existing values.

I just thought it was worth pointing out. I'm generally of the opinion these kinds of "breaking changes" are perfectly reasonable though.

@reinink reinink merged commit 95dd82b into tailwindlabs:master Dec 13, 2021
@reinink
Copy link
Member

reinink commented Dec 14, 2021

@o-t-w Hey, I forgot to say thanks when I merged this in earlier. Much appreciated! 🙏

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.

3 participants