-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Forward modifier from has-*
and not-*
variants to compounded variant
#14935
base: next
Are you sure you want to change the base?
Conversation
69f4139
to
b89c1e0
Compare
packages/tailwindcss/src/variants.ts
Outdated
} else { | ||
return null | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels super weird to me to mutate the variant we're getting. Could this be handled in parseCandidate or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though I guess we'd need some additional metadata for that to work right. I guess as long as this isn't guaranteed to "break" non-compounded versions of the same variant (do we have a test for that?) then it's probably fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handling it during parsing might break things as well, especially when we re-print a candidate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hopefully can fix this w/o too much hassle
bf5de7a
to
8865b00
Compare
// Forward modifier from `not-*` and `has-*` variants to the | ||
// compound variant. | ||
if (modifier !== null && (root === 'not' || root === 'has')) { | ||
value = `${value}/${modifier}` | ||
modifier = null | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more correct, but feels so wrong in this spot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah agree it does feel wrong 🤔
Could in theory add a forwardsModifier()
method to Variants and control it from there? Gonna think on this some
1183f3b
to
0349a86
Compare
0349a86
to
36e1667
Compare
36e1667
to
1dec3d6
Compare
This PR fixes an issue where using a modifier in combination with
has-*
ornot-*
doesn't work as expected.If you have the following HTML:
Then the following CSS is generated:
But if you want to use it with
not-*
, then nothing is being generated:/* <EMPTY> */
But with this PR, we will forward the modifier
parent
from thenot
variant, to thegroup
variant, so the following CSS is generated:This is also implemented for
has-*