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

[vtadmin-web] Upgrade to tailwindcss v3 #9648

Merged
merged 8 commits into from
Feb 7, 2022

Conversation

doeg
Copy link
Contributor

@doeg doeg commented Feb 7, 2022

Description

This updates vtadmin-web to use tailwindcss v3, following the Upgrade Guide.

Here's the announcement, for the curious: https://tailwindcss.com/blog/tailwindcss-v3

I verified that everything still looks + works ok by poking around in the UI. The most likely oversight (since it's difficult to grep for) is any instance of "dynamic classnames", which are not supported; that is, class names that use string templates like this:

// Not supported
<div class="text-{{ error ? 'red' : 'green' }}-600"></div>

// OK!
<div class="{{ error ? 'text-red-600' : 'text-green-600' }}"></div>

Happy to fix forward anything I may have missed. I've long dreamed of something like Percy for visual regression testing... but that's a whole thing that comes with its own trade-offs. So! Manual verification for now.

Beyond having the latest + greatest, the other motivation for doing this now is that the tailwindcss@npm:@tailwindcss/postcss7-compat dependency, a requirement from tailwindcss v2, breaks npm outdated:

🍕~/workspace/vitess/web/vtadmin $ npm outdated
npm ERR! Only tag, version, and range are supported

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/sarabee/.npm/_logs/2022-02-07T19_25_43_567Z-debug.log

Related Issue(s)

This goes along with the react-scripts + postcss upgrade in #9493, and the subsequent postcss upgrade in #9612.

Checklist

  • Should this PR be backported? No
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

N/A

…t; move to dev dependencies

Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
@@ -232,3 +230,121 @@ table tbody td[rowSpan] {
.Toastify__toast-theme--dark {
background: none !important;
}

/* See https://tailwindcss.com/docs/extracting-components */
@layer components {
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 pulled this CSS from src/style/components.css, and then deleted that file.

This is because tailwindcss v3 tweaked its base button styles in a way that conflicted with our .btn component, as described in tailwindlabs/tailwindcss#6602. Trying to fix it resulted in a real nasty snarl of import ordering issues, so I've opted to simply pull everything into this one file for now.

@@ -56,3 +56,201 @@
.tabContainer {
margin: 2.4rem 0;
}

.danger {
Copy link
Contributor Author

@doeg doeg Feb 7, 2022

Choose a reason for hiding this comment

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

This unpleasant part of the diff adds a bunch of CSS classes for rendering the colour chips on the debug page. The new JIT compiler in tailwindcss v3 does not work with "dynamic classes" e.g., a JavaScript string template like background-${foo}.

There are more "elegant" ways to fix this with postcss loops but it looked like it would require adding a new postcss dependency to support loops and... 🤷

@doeg doeg requested a review from notfelineit February 7, 2022 18:55
@doeg doeg marked this pull request as ready for review February 7, 2022 18:55
@doeg doeg requested a review from ajm188 as a code owner February 7, 2022 18:55
@doeg doeg added Component: VTAdmin VTadmin interface release notes none Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Feb 7, 2022
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Copy link
Contributor

@notfelineit notfelineit left a comment

Choose a reason for hiding this comment

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

Thank you for handling this!

Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
@doeg doeg merged commit 55060d7 into vitessio:main Feb 7, 2022
@doeg doeg deleted the sarabee-tailwindcss-v3 branch February 7, 2022 20:35
@doeg doeg removed the Backport me! label Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: VTAdmin VTadmin interface Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants