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

[#69] Tailwind 4 upgrade #81

Merged
merged 6 commits into from
Feb 25, 2025
Merged

[#69] Tailwind 4 upgrade #81

merged 6 commits into from
Feb 25, 2025

Conversation

joshuapease
Copy link
Contributor

@joshuapease joshuapease commented Feb 20, 2025

Overview

Upgrades Tailwind to v4.

Really wasn't that bad, the npx @tailwindcss/upgrade@next command worked well.

Here are the manual changes I made (more details in code comments).

  • When using only the .border class, TW uses currentColor instead of the default gray-200. I manually added border-gray-200 back to all instances of border that lacked a color class.
  • I Ported the JS config to app.css fortunately we didn't have a lot of unique changes other than our custom sizes and using the selector mode for our dark theme.
  • Migrated to Tailwind's max- media queries.
  • I resolved some issues related to nesting dark:xyz variants within a ::after selector.
    • Additional details are provided in the comments.
  • Resolved some issues with @keyframes in the dialog component.

JavaScript components / utilities.

Making components and utilities in JavaScript files seems to be "second class" to just writing them in CSS files. I couldn't even find details in the docs.

They still can be imported in the CSS-based Tailwind Config.

I'm curious if we should migrate these to CSS files since that seems to be the way TW is hidden. We do lose intellisense when we stop using the JS-based components. But I find that a very minor annoyance.

<button> tags use the default cursor

I noticed this minor change in TW 4's upgrade guide.

https://tailwindcss.com/docs/upgrade-guide#buttons-use-the-default-cursor

Buttons now use cursor: default instead of cursor: pointer to match the default browser behavior.

If you'd like to continue using cursor: pointer by default, add these base styles to your CSS:

@layer base {
  button:not(:disabled),
  [role="button"]:not(:disabled) {
    cursor: pointer;
  }
}

Our button components manually have cursors applied. But I'm curious if this a default we want to restore.

Other notes & unanswered questions

Comment on lines +26 to +42
},
// Animation Keyframes
'@keyframes dialog-in': {
'0%': { transform: 'translateY(2rem)', opacity: '0' },
'100%': { transform: 'translateY(0px)', opacity: '1' },
},
'@keyframes dialog-out': {
'0%': { transform: 'scale(1)', opacity: '1' },
'100%': { transform: 'scale(0.9)', opacity: '0' },
},
'@keyframes dialog-backdrop-in': {
'0%': { opacity: '0' },
'100%': { opacity: '1' },
},
'@keyframes dialog-backdrop-out': {
'0%': { opacity: '1' },
'100%': { opacity: '0' },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CleanShot 2025-02-20 at 06 59 39@2x

These keyframes weren't ending up in the final CSS and I was also getting some Typescript errors.

I don't think nesting keyframes within a class is technically valid. Seems like TW4 must be a bit more strict.

The issue was resolved by moving them outside of .dialog and making the opacity values strings.

Comment on lines +68 to +75
'&[data-required] label .field-label': {
'&::after': {
'@apply content-["*"] text-red-600 align-super text-xs ml-2 font-medium':
{},
},
'.dark &::after': {
'@apply text-red-300': {},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Nesting dark:text-red-300 inside of the ::after selector was throwing the above error.

TW 4 docs recommend this setting to restore the TW 3 "selector" strategy for dark mode.

@custom-variant dark (&:where(.dark, .dark *));

However, I don't think TW3 used :where().

where() seems to confuse TW's compiler when it's nested inside of ::after

export default {
plugins: {
tailwindcss: {},
autoprefixer: {},
'@tailwindcss/postcss': {},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could have opted for TW's vite plugin, but for now I kept things the way they were.

https://tailwindcss.com/docs/installation/using-vite

Wasn't sure if it would bypass all the other postcss features we use.

@@ -65,7 +65,7 @@
size: 'sm',
title: 'Close',
variant: 'subtle',
class: 'absolute right-4 top-4 size-24 !min-h-0 [&_svg]:size-12',
class: 'absolute right-4 top-4 size-24 min-h-0! [&_svg]:size-12',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -33,7 +33,7 @@
{% tag el with attrs|merge({
href: href,
type: clickable and el == 'button' ? 'button' : null,
class: classNames('inline-flex items-center justify-center gap-4 rounded-sm text-center font-medium focus:outline-none focus-visible:ring-4', {
class: classNames('inline-flex items-center justify-center gap-4 rounded-xs text-center font-medium focus:outline-hidden focus-visible:ring-4', {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TW-4 renamed a few utilities to make them more consistent. The automated migration handled most of them.

@@ -15,7 +15,7 @@

<header class="{{ cx(
'border-b border-neutral-200 bg-neutral-100',
'mdd:sticky mdd:top-0 mdd:z-50 mdd:bg-white mdd:shadow-md',
'max-md:sticky max-md:top-0 max-md:z-50 max-md:bg-white max-md:shadow-md',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had added some custom screens config to create max media queries.

But this is now part of TW 4 (or might have been since 3 actually)

@joshuapease joshuapease marked this pull request as ready for review February 20, 2025 17:46
@joshuapease joshuapease merged commit 9785f3c into 5.x Feb 25, 2025
@joshuapease joshuapease deleted the jp/69-tailwind-4-upgrade branch February 25, 2025 23:29
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.

5 participants