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 override explicit leading-*, tracking-*, or font-{weight} utilities with font-size utility defaults #14403

Merged
merged 4 commits into from
Sep 18, 2024

Conversation

adamwathan
Copy link
Member

This PR improves how the text-{size} utilities interact with the leading-*, tracking-*, and font-{weight} utilities, ensuring that if the user explicitly uses any of those utilities that those values are not squashed by any defaults baked into the text-{size} utilities.

Prior to this PR, if you wrote something like this:

<div class="text-lg leading-none md:text-2xl">

…the leading-none class would be overridden by the default line-height value baked into the text-2xl utility at the md breakpoint. This has been a point of confusion and frustration for people in the past who are annoyed they have to keep repeating their custom leading-* value like this:

<div class="text-lg leading-none md:text-2xl md:leading-none lg:text-4xl lg:leading-none">

This PR lets you write this HTML instead but get the same behavior as above:

<div class="text-lg leading-none md:text-2xl lg:text-4xl">

It's important to note that this change only applies to line-height values set explicitly with a leading-* utility, and does not apply to the line-height modifier.

In this example, the line-height set by text-sm/6 does not override the default line-height included in the md:text-lg utility:

<div class="text-sm/6 md:text-lg">

That means these two code snippets behave differently:

<div class="text-sm/6 md:text-lg"></div>
<div class="text-sm leading-6 md:text-lg"></div>

In the top one, the line-height md:text-lg overrides the line-height set by text-sm/6, but in the bottom one, the explicit leading-6 utility takes precedence.

This PR applies the same improvements to tracking-* and font-{weight} as well, since all font size utilities can also optionally specify default letter-spacing and font-weight values.

We achieve this using new semi-private CSS variables like we do for things like transforms, shadows, etc., which are set by the leading-*, tracking-*, and font-{weight} utilities respectively. The text-{size} utilities always use these values first if they are defined, and the default values become fallbacks for those variables if they aren't present.

We use @property to make sure these variables are reset to initial on a per element basis so that they are never inherited, like with every other variable we define.

This PR does slightly increase the amount of CSS generated, because now utilities like leading-5 look like this:

  .leading-5 {
+   --tw-leading: 1.25rem;
    line-height: 1.25rem;
  }

…and utilites like text-sm include a var(…) lookup that they didn't include before:

  .text-sm {
    font-size: 0.875rem;
-   line-height: var(--font-size-sm--line-height, 1.25rem);
+   line-height: var(--tw-leading, var(--font-size-sm--line-height, 1.25rem));
  }

If this extra CSS doesn't feel worth it for the small improvement in behavior, we may consider just closing this PR and keeping things as they are.

This PR is also a breaking change for anyone who was depending on the old behavior, and expected the line-height baked into the md:text-lg class to take precedence over the explicit leading-6 class:

<div class="text-sm leading-6 md:text-lg"></div>

Personally I am comfortable with this because of the fact that you can still get the old behavior by preferring a line-height modifier:

<div class="text-sm/6 md:text-lg"></div>

@adamwathan adamwathan force-pushed the feat/preserve-font-overrides branch from 78c0b55 to b9c221a Compare September 12, 2024 13:11
@RobinMalfait RobinMalfait force-pushed the feat/preserve-font-overrides branch from b9c221a to 38047a3 Compare September 12, 2024 14:09
@adamwathan adamwathan force-pushed the feat/preserve-font-overrides branch from 38047a3 to d4dd827 Compare September 12, 2024 15:04
Copy link
Contributor

@thecrypticace thecrypticace left a comment

Choose a reason for hiding this comment

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

One small thing I noticed but otherwise looks right to me

packages/tailwindcss/src/utilities.ts Outdated Show resolved Hide resolved
@adamwathan adamwathan force-pushed the feat/preserve-font-overrides branch from b44f573 to 29caddb Compare September 17, 2024 15:09
@adamwathan adamwathan merged commit a51b63a into next Sep 18, 2024
3 checks passed
@adamwathan adamwathan deleted the feat/preserve-font-overrides branch September 18, 2024 13:20
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.

2 participants