perf: Add tailwind to editor and design system #9032
Conversation
n8n-assistant
bot
added
n8n team
| module.exports = { | ||
| content: ['./index.html', './src/**/*.{vue,js,ts}'], | ||
| darkMode: ['selector', '[data-theme="dark"]'], | ||
| theme: { |
There was a problem hiding this comment.
thank you for adding this. But not sure this is enough.. we need some work here to map our design system to the tailwind tokens..
so that we can have consistency and this could succeed..
There was a problem hiding this comment.
That could be the next step. First, we should make sure none of the tailwind tokens collide with ours, and then we can start using them.
If I understood correctly, it would not add anything to the built bundle if you don't use anything from Tailwind.
I want to leverage its layout helpers in the first place because they are missing from our design system.
For typography, colors, and spacing, we can still use ours, and later, we can integrate.
There was a problem hiding this comment.
How much more effort would be adding that support here? Because colors and spacing would be the biggest win here and feels incomplete without it.
There was a problem hiding this comment.
I don't know how many colors we're using and haven't yet gone deep into customizing Tailwind with CSS variables (I know it's also using them)
1 way would be to overwrite existing tokens with our colors, they have quite a thorough palette and I don't think we use more shades than them (we shouldn't at least).
10-11 variants is more than enough for each color
The other way would be to give all of our own design system colors a meaningful CSS class name and just add them the the tailwind config
https://tailwindcss.com/docs/customizing-colors
Quite the same goes for spacing, luckily we don't have much of them
https://tailwindcss.com/docs/customizing-spacing
I'd say 1-3 days would be enough but verifying and testing is another question (especially without visual regression tests)
There was a problem hiding this comment.
okay, might be too much effort then to ask of you now.
in my head it's simply adding the mapped tokens to our existing css colors.
why do we need regression tests though? I am only asking to add the tokens like this, maybe update a small component to use the tailwind classes to validate..
There was a problem hiding this comment.
we should not touch our current css variables..
There was a problem hiding this comment.
Ok, I see. We just add these and start to use them as CSS classes on the elements rather than the CSS variables in the style tags.
I would be much calmer if we had a regression test to see if nothing was changed visually after introducing tailwind
There was a problem hiding this comment.
Okay. Ya you only need to touch a small component as a POC..
Happy to help with testing in any way. I ran it locally and tested and everything seemed fine.
Do you agree to add colors/spacing tokens within this PR then? Happy to approve if you still think otherwise.
There was a problem hiding this comment.
If nothing is broken I suggest add colors and spacing in a separate PR
|
|
1 flaky test on run #4773 ↗︎
Details:
|
|||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| NDV > should not retrieve remote options when required params throw errors |
Screenshots
Video
|
|
Review all test suite changes for PR #9032 ↗︎
|
|
|
✅ All Cypress E2E specs passed |
Context