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

Don't rely on existence of --default-transition-* variables in transition utilities #13219

Merged
merged 4 commits into from
Mar 13, 2024

Conversation

adamwathan
Copy link
Member

This PR fixes an issue where using @theme reference { ... } would cause the transition-* to stop working properly because they were relying on theme variables being present in the generated CSS.

Right now this also inlines the values of those variables rather than emitting a var(...) call. This is fine because we generally inline values of all variables currently, but if we decide to use var(...) for everything we'll want to update this.

Fixes #13215.

@@ -948,4 +948,29 @@ describe('Parsing themes values from CSS', () => {
}"
`)
})

test('utilities that rely on the --default namespace still work when using `@theme reference`', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this test is really needed since this change is reflected in the utilities tests. Maybe better to test this there specifically for the transition utilities with new test cases for providing these two variables both with @theme and with @theme reference.

@Steffan153
Copy link

Steffan153 commented Mar 12, 2024

Shouldn't the default be 150ms, not 0s?

DEFAULT: '150ms',

@adamwathan adamwathan force-pushed the default-vars-with-theme-reference branch from ebf47e9 to 2ac9976 Compare March 12, 2024 18:14
@adamwathan
Copy link
Member Author

adamwathan commented Mar 12, 2024

Shouldn't the default be 150ms, not 0s?

DEFAULT: '150ms',

@Steffan153 So the default value here isn't meant to be the default theme value, it's the value to use if the person has explicitly disabled the default theme by not importing it.

Since the 150ms and cubic-bezier(0.4, 0, 0.2, 1) are opinionated design tokens, to me it makes more sense to default to the browser's default values rather than Tailwind's default values if someone is explicitly opting out of the default theme 👍

The value is still respected even when using @theme reference:

  expect(
    compileCss(
      css`
        @theme reference {
          --default-transition-duration: 150ms;
        }
        @tailwind utilities;
      `,
      ['transition-all'],
    ),
  ).toMatchInlineSnapshot(`
    ".transition-all {
      transition-property: all;
      transition-duration: .15s;
      transition-timing-function: ease;
    }"
  `)

The only time we'll fall back to 0s is if you aren't defining that variable at all, with or without reference 👍

This is more consistent with how things worked in v3 and ensures things will still work if the user suppresses the output of all CSS variables.
@adamwathan adamwathan merged commit 91fca13 into next Mar 13, 2024
1 check passed
@adamwathan adamwathan deleted the default-vars-with-theme-reference branch March 13, 2024 16:14
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