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

Adjust shadow, radius, and blur scales to ensure all utilities have a value suffix #14849

Merged
merged 5 commits into from
Nov 4, 2024

Conversation

adamwathan
Copy link
Member

@adamwathan adamwathan commented Oct 31, 2024

This PR reworks the default --shadow-* and --inset-shadow-* scales to remove the bare shadow and inset-shadow utilities, and ensure every shadow has an explicit size as part of the utility name.

Here's a complete list of changes:

v3 v4 Alpha Proposed
N/A shadow-xs shadow-2xs
shadow-sm shadow-sm shadow-xs
shadow shadow shadow-sm
shadow-md shadow-md shadow-md
shadow-lg shadow-lg shadow-lg
shadow-xl shadow-xl shadow-xl
N/A inset-shadow-xs inset-shadow-2xs
N/A inset-shadow-sm inset-shadow-xs
shadow-inner inset-shadow inset-shadow-sm

The motivation for this change is just to make the scale more predictable — it's never been intuitive to me that shadow sits in between shadow-sm and shadow-md.

This PR doesn't remove the ability to create classes like shadow and inset-shadow by adding bare --shadow and --inset-shadow theme variables, but does remove them from the default theme.

Impact

We'll include a codemod for this in our upgrade tool to automate this change for people upgrading from v3 to v4, but this is still sort of an annoying breaking change admittedly and will make lots of educational resources, example components, and LLM tools out of date for v4 😕 At the same time I don't want to feel like we can never correct regrettable legacy decisions just to preserve backward compatibility.

We made a similar change like this when we went from the v0.x color palette to the v1.x color palette changing names like bg-red to bg-red-500 and that proved to definitely be the right decision long term, so want to rip the band-aid off here too if we can.

Planning to make the same change for rounded, drop-shadow, and blur as well — maybe in separate PRs but maybe just all in this one as well since I don't think we want to do one and not all.

Update: I've also made the same changes to the --radius-*, --drop-shadow-*, and --blur-* scales now, effectively removed the rounded, drop-shadow, and blur classes by default, and changing the meaning rounded-sm, drop-shadow-sm, and blur-sm.

We'll put together a codemod to handle this stuff in a separate PR.

@adamwathan adamwathan requested a review from a team as a code owner October 31, 2024 19:18
@adamwathan adamwathan changed the title Rename shadow to shadow-sm and inset-shadow to inset-shadow-sm Adjust shadow scales to remove shadow and inset-shadow utilities Oct 31, 2024
@saadeghi
Copy link

saadeghi commented Oct 31, 2024

I like the new range 👍

Planning to make the same change for rounded, drop-shadow, and blur as well

I think a challenge of using the default theme of Tailwind is, sometimes class names are not predictable (to guess). For example, It's not easy to guess if each token starts with xs or sm or… (also some of them have a default)

I think it would be better if they all start at the same name (2xs or 3xs or...)

Here are the other sizings from Tailwind 4 alpha, and since they all don't start from the same name, it's sometimes hard guess, I think it's a common experience to try typing the class name it in editor and if there's an intellisense and I don't see a suggestion, then I know "oh, this one starts at sm not xs"

token - - - - - - - - - - - - - - - -
breakpoint - - - - sm md lg xl 2xl - - - - - - -
blur [default] - - - sm md lg xl 2xl 3xl - - - - - -
radius [default] - - - sm md lg xl 2xl 3xl 4xl - - - - -
dropshadow [default] - - - sm md lg xl 2xl - - - - - - -
width - 3xs 2xs xs sm md lg xl 2xl 3xl 4xl 5xl 6xl 7xl - -
font-size - - - xs sm md lg xl 2xl 3xl 4xl 5xl 6xl 7xl 8xl 9xl

@steveoh
Copy link

steveoh commented Oct 31, 2024

My first thought would be to follow the font size classes and use a -base suffix but if that doesn't exist in v4 then what you suggest seems great.

@kandros
Copy link

kandros commented Oct 31, 2024

I’m slightly against this proposal, the best feature of tailwind is that is basically a standard library at this point, predictable results unless theres project-specific overrides. The idea that things change meaning across versions creates both a mental overhead (less trust of the result) and technical challenges for things with different versions, eg design systems + app code, federated modules ecc.

considering this is more of “I wish it was different” than a solution to a problem makes me think is not worth it

@xewl
Copy link

xewl commented Nov 1, 2024

I'm all for the band-aid ripping. A major version shouldn't be held back by choices made in a by-then legacy version. Break all the things 🔥

@michaelsnook
Copy link

This sounds good to me -- I've been using TW for years and I never understood that shadow wasn't the same as shadow-md.

So now, will anything happen if I use the class shadow? Will there be a default size / can I configure it? (or will that class name be left open for folks to write their own default shadow rules?)

@jan-dh
Copy link

jan-dh commented Nov 1, 2024

The new scale makes sense, would make shadow and alias to shadow-md for predictably though.

@adamwathan adamwathan changed the title Adjust shadow scales to remove shadow and inset-shadow utilities Adjust shadow, radius, and blur scales to ensure all utilities have a value suffix Nov 4, 2024
@adamwathan adamwathan force-pushed the change/no-default-in-shadow-scale branch from 6047dc6 to d1724fd Compare November 4, 2024 17:19
Copy link
Member

@philipp-spiess philipp-spiess left a comment

Choose a reason for hiding this comment

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

LGTM! I wonder if something can be simplified in the utilities code now that we no longer need to handle an shadow (without an argument)?

In other words, do we still need this branch now or should this just return null?

https://github.com/tailwindlabs/tailwindcss/blob/next/packages/tailwindcss/src/utilities.ts#L4279-L4291

@adamwathan
Copy link
Member Author

LGTM! I wonder if something can be simplified in the utilities code now that we no longer need to handle an shadow (without an argument)?

In other words, do we still need this branch now or should this just return null?

https://github.com/tailwindlabs/tailwindcss/blob/next/packages/tailwindcss/src/utilities.ts#L4279-L4291

@philipp-spiess I think we should still support that for backwards compatibility personally, just in case someone is porting over their existing config that still has a shadow class 👍

@philipp-spiess
Copy link
Member

philipp-spiess commented Nov 4, 2024

@adamwathan makes sense... does this mean it should be in the compat layer though? (depends on if we want people to use this in a fresh v4 project I guess?)

@adamwathan
Copy link
Member Author

@adamwathan makes sense... does this mean it should be in the compat layer though? (depends on if we want people to use this in a fresh v4 project I guess?)

@philipp-spiess Yeah maybe? Just a question of do we consider the whole idea of being able to have a shadow utility to be legacy, or just shipping one by default to be legacy.

I'm not sure how we'd make it work in the compat layer either without unregistering the default shadow handler and registering a replacement 🤔

@philipp-spiess
Copy link
Member

philipp-spiess commented Nov 4, 2024

Just a question of do we consider the whole idea of being able to have a shadow utility to be legacy, or just shipping one by default to be legacy.

Exactly, that's what I was thinking. I don't know what is right either though.

I'm not sure how we'd make it work in the compat layer either without unregistering the default shadow handler and registering a replacement 🤔

We do have a precedence for this where we just wrap the core utility and handle an additional cases: https://github.com/tailwindlabs/tailwindcss/blob/next/packages/tailwindcss/src/compat/theme-variants.ts#L9-L27

@adamwathan adamwathan merged commit ca4e4ae into next Nov 4, 2024
1 check passed
@adamwathan adamwathan deleted the change/no-default-in-shadow-scale branch November 4, 2024 19:04
RobinMalfait added a commit that referenced this pull request Nov 6, 2024
This PR adds a migration for migrating the changes we implemented in
#14849

This is the migration we perform:

| Old               | New                |
| ----------------- | ------------------ |
| `shadow`          | `shadow-sm`        |
| `shadow-sm`       | `shadow-xs`        |
| `shadow-xs`       | `shadow-2xs`       |
| `inset-shadow`    | `inset-shadow-sm`  |
| `inset-shadow-sm` | `inset-shadow-xs`  |
| `inset-shadow-xs` | `inset-shadow-2xs` |
| `drop-shadow`     | `drop-shadow-sm`   |
| `drop-shadow-sm`  | `drop-shadow-xs`   |
| `rounded`         | `rounded-sm`       |
| `rounded-sm`      | `rounded-xs`       |
| `blur`            | `blur-sm`          |
| `blur-sm`         | `blur-xs`          |

Also added an integration test to ensure that `shadow` is properly
migrated to `shadow-sm`, and doesn't get migrated to `shadow-xs`
(because `shadow-sm` is migrated to `shadow-xs`).
adamwathan added a commit that referenced this pull request Nov 9, 2024
This PR reintroduces the `blur`, `shadow`, `drop-shadow`, and `rounded`
utilities that were removed in #14849, just to preserve backward
compatibility as much as possible.

These values are still considered deprecated, and we register them as
`inline reference` with the theme to ensure they don't produce any CSS
variables in output:

```css
/* Deprecated */
@theme default inline reference {
  --blur: 8px;
  --shadow: 0 1px 3px 0 rgb(0 0 0 / 0.1), 0 1px 2px -1px rgb(0 0 0 / 0.1);
  --drop-shadow: 0 1px 2px rgb(0 0 0 / 0.1), 0 1px 1px rgb(0 0 0 / 0.06);
  --radius: 0.25rem;
}
```

These values won't be included in the documentation, and in the future
we'll add an option to explicitly register things as `deprecated` so
that they don't appear as completions in IntelliSense either.

Co-authored-by: Adam Wathan <4323180+adamwathan@users.noreply.github.com>
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.

8 participants