Skip to content

Conversation

@adhami3310
Copy link
Member

No description provided.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 6, 2025

CodSpeed Performance Report

Merging #4766 will not alter performance

Comparing upgrade-to-tailwind-4 (1e42ee0) with main (b3b79a6)

Summary

✅ 8 untouched benchmarks

@adhami3310
Copy link
Member Author

@carlosabadia i couldn't figure out how to fix reflex-web here. feel free to try

@masenf
Copy link
Collaborator

masenf commented Feb 13, 2025

i tried poking around with these changes

diff --git a/assets/tailwind-theme.css b/assets/tailwind-theme.css
index 92cfba50..a0f54364 100644
--- a/assets/tailwind-theme.css
+++ b/assets/tailwind-theme.css
@@ -1,15 +1,29 @@
 @tailwind base;
 @tailwind components;
 @tailwind utilities;
-
-:root {
-    --font-instrument-sans: 'Instrument Sans', sans-serif;
-    --font-source-code-pro: 'Source Code Pro', monospace;
-    --font-jetbrains: "JetBrains Mono", monospace;
-    color-scheme: light dark;
+@reference '../styles/tailwind.css'
+
+:root {}
+
+/* in tw4, need to define apply-able classes as @utility */
+@utility font-small {
+    font-family: var(--font-instrument-sans);
+    font-size: 0.9rem;
+    font-style: normal;
+    font-weight: 500;
+    line-height: 1.25rem;
+    /* 142.857% */
+    letter-spacing: -0.01094rem;
 }
 
 @layer components {
+    :root {
+        --font-instrument-sans: 'Instrument Sans', sans-serif;
+        --font-source-code-pro: 'Source Code Pro', monospace;
+        --font-jetbrains: "JetBrains Mono", monospace;
+        color-scheme: light dark;
+    }
+
     .font-instrument-sans {
         font-family: var(--font-instrument-sans);
     }
@@ -203,13 +217,7 @@
     }
 
     .font-small {
-        font-family: var(--font-instrument-sans);
-        font-size: 0.9rem;
-        font-style: normal;
-        font-weight: 500;
-        line-height: 1.25rem;
-        /* 142.857% */
-        letter-spacing: -0.01094rem;
+        @apply font-small;
     }
 
     .font-md {

and while that got things to compile, the side didn't look the same.

desktop-only/mobile-only was busted

font sizes were different

i think to make it work, we need to port our relatively complex tailwind config over to the v4 config format.

@carlosabadia
Copy link
Contributor

I'll create a ticket to update it. However, will previous Tailwind versions still be available for use after this is merged?

@adhami3310
Copy link
Member Author

should we add an option to choose what version of tailwind to use ? i'm conflicted

@carlosabadia
Copy link
Contributor

should we add an option to choose what version of tailwind to use ? i'm conflicted

It can be changed in rx.config -> frontend_packages=["tailwindcss@3.4.13"]

The v4 wont work though, without the changes in this PR

@carlosabadia
Copy link
Contributor

@adhami3310 Started upgrading reflex-web and noticed that Tailwind styles no longer take priority over Radix themes. We might need to adjust the layer where Tailwind is generated by default or something

@itsmeadarsh2008
Copy link
Contributor

Here's the migration tool for reference:
https://tailwindcss.com/docs/upgrade-guide#changes-from-v3
npx @tailwindcss/upgrade

@adhami3310 adhami3310 closed this May 22, 2025
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