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

var() is incorrectly wrapped around dashed idents with CSS scroll animations #12205

Closed
mskelton opened this issue Oct 14, 2023 · 6 comments · Fixed by #12236
Closed

var() is incorrectly wrapped around dashed idents with CSS scroll animations #12205

mskelton opened this issue Oct 14, 2023 · 6 comments · Fixed by #12236

Comments

@mskelton
Copy link

What version of Tailwind CSS are you using?

For example: v3.3.3

What build tool (or framework if it abstracts the build tool) are you using?

Next.js 13.4.16

What version of Node.js are you using?

v20.6.1

What browser are you using?

Chrome

What operating system are you using?

macOS

Reproduction URL

https://play.tailwindcss.com/mFflelRLYg

Describe your issue

I've been exploring using Tailwind with CSS scroll animations and I've noticed that Tailwind is incorrectly adding var() to specific keywords. For example, with this code:

<div class="[timeline-scope:--tl]">
  <div class="[scroll-timeline:--tl]">Scroller</div>
  <div class="[animation-timeline:--tl]">Element</div>
</div>

It is incorrectly setting the css properties like so:

timeline-scope: var(--tl);
scroll-timeline: var(--tl);
animation-timeline: var(--tl);

When the intended and correct output should be:

timeline-scope: --tl;
scroll-timeline: --tl;
animation-timeline: --tl;

MDN docs

@adamwathan
Copy link
Member

Thanks for reporting, definitely a bug and will be interesting to figure out how we effectively solve for this because of the sounded-like-a-good-idea-but-clearly-wasn't automatic var insertion stuff we do 🤦🏻‍♂️

We'll definitely figure it out but as a temporary workaround you can add an underscore before the dashed ident and Tailwind will be too dumb to convert it to a var call:

https://play.tailwindcss.com/k2bdKI8TlB

<div class="[timeline-scope:_--tl]">
  <div class="[scroll-timeline:_--tl]">Scroller</div>
  <div class="[animation-timeline:_--tl]">Element</div>
</div>

@mskelton
Copy link
Author

Thanks for the underscore tip, I was trying to figure out a way to trick the compiler, but that was on thing I didn't try 😄

@macarie
Copy link

macarie commented Oct 15, 2023

I've encountered the same issue today while trying to animate CSS vars.

@adamwathan, while I see the benefits of having automatic var insertion, I don't expect it to happen with custom properties (e.g., [transition-property:--sc-t,--sc-b]).

Maybe a good middle ground would be to keep it only for custom values (e.g., transition-[--sc-t,--sc-b], which gets converted to var(--sc-t,--sc-b), which is wrong tho 😅) while keeping the custom properties verbatim. What do you think?

@adamwathan
Copy link
Member

@macarie Yeah I think that could be pretty sensible, agree it's a bit strange that if you pass a complete CSS declaration that we insert var into part of it instead of accepting it verbatim. Issue is it's a breaking change but we'll have to figure it out regardless.

@RobinMalfait
Copy link
Contributor

One solution I've implemented in #12236 is to not automatically inject the var() when using custom properties, where the custom property itself can receive a <dashed-ident>. If you do want it (even though I think it is invalid right now), you have to be explicit about it: [timeline-scope:var(--foo)]

Only problem with this is that properties that expect a <custom-ident> or an <ident> can technically also receive just --foo without the var().

We can add these properties to the list as well, but this will result in a breaking changes 🤔

@RobinMalfait
Copy link
Contributor

This should be fixed by #12236, and will be available in the next release.

You can already try it by using the insiders build npm install tailwindcss@insiders.

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 a pull request may close this issue.

4 participants