-
Notifications
You must be signed in to change notification settings - Fork 651
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
fix(tailwind): className manipulation for component props #1556
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 9302024 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
gabrielmfern
force-pushed
the
fix/class-name-manipulation-tailwind
branch
from
July 25, 2024 16:16
0609302
to
ef821fc
Compare
gabrielmfern
force-pushed
the
fix/class-name-manipulation-tailwind
branch
from
July 29, 2024 16:16
76800b3
to
5a3f129
Compare
gabrielmfern
force-pushed
the
fix/class-name-manipulation-tailwind
branch
from
July 29, 2024 16:31
5a3f129
to
a643e48
Compare
vcapretz
approved these changes
Jul 31, 2024
gabrielmfern
force-pushed
the
fix/class-name-manipulation-tailwind
branch
from
July 31, 2024 17:23
a643e48
to
9302024
Compare
gabrielmfern
added a commit
that referenced
this pull request
Aug 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is meant so that it allows users to use
twMerge
with respectivecomponents without it having any issues.
For background, we inline all styles from Tailwind classes and then remove the
classes. This is done on the props of all React elements inside of the Tailwind
component's children. This means that
className
s are removed before the usercan deal with them to merge or manipulate in general.
The reason we can't just direclty not run Tailwind on the components and just
leave the classNames be, is because our components specifically do manipulation
on the styles. Meaning that, if only the classNames come in to be manipulated,
the components won't do what they would normally do.
The way this PR goes around that issue is then, by still running Tailwind on
the component, thus adding the proper
style
properties to the component, butwithout removing the respective classNames. This still allows us to do style
manipulation while allowing users to do className manipulation in the same way.
There is still one issue that might arise from this, though. When using the
<Button>
component, for example, we do some transformations to the paddingbefore actually applying it to the underlying
<a>
element while still addingthe needed classNames to the same element. Since, the current behavior of the
component is to give priority to the new styles from Tailwind classNames over
the already inlined styles, it means that these transformations will be ignored
in our component making it not work the same way as before. The only to fix
this, is to do the opposite of what we already do — we need to give priority to
the already inlined styles on elements.
This last change is still a trade-off though, and might be a bit unintuitive to
users. So, something to lookout for.