feat(dashboard): design system tokens #7300
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| --purple-300: 256 88% 64% / 60%; | ||
| --purple-200: 256 88% 64% / 40%; | ||
| --purple-100: 256 88% 64% / 20%; | ||
| --purple-50: 256 88% 64% / 10%; |
There was a problem hiding this comment.
Isn't it a bit weird that some colors have opacity and some not ?
There was a problem hiding this comment.
I tend to agree, @twentyone24 do you think we can make all of the opacity based? Or is it too much effort?
There was a problem hiding this comment.
If we can make it, then we don't need to define all the tokens in this CSS file. Instead, we will just have "base" colors and generate all the "semantic" ones in the tailwind config. Otherwise still we can simplify the opacity variables by defining them in the tw.
|
I'm beginning to think that we should not add all of these right now. We started with the mentality that we will be adding on demand. All of these colors are unused. Maybe let's just do the semantic colors? |
|
@desiprisg what do you mean by semantic colors? you mean removing the sky, and pink, teal and the ones that are unused? @twentyone24 what do you think? IMO, if we have a color on the design system on figma we should also have it here. Otherwise let's remove it from Figma. |
| @@ -18,7 +35,27 @@ | |||
| --foreground-900: 226 21% 12%; | |||
| --foreground-950: 222 32% 8%; | |||
|
|
|||
| --primary: 346 73% 50%; | |||
| --novu-800: 346 72% 42%; | |||
There was a problem hiding this comment.
Is Novu color the primary color?
There was a problem hiding this comment.
Align doesa separation between the color pallete, and their semantic usage. Novu is the color palette, while primary is the semantic usage of the novu color.
| --warning-light: var(--orange-200); | ||
| --warning-lighter: var(--orange-50); | ||
|
|
||
| --error-dark: var(--red-950); |
There was a problem hiding this comment.
Does Align UI suggest this practice? I am not well-versed in how modern design systems prefer to do this, but I'd avoid overloading CSS with too many semantics. As a pattern, it makes sense when the color needs to be reused in two semantic tokens, for example, error and destructive in our case, but I'd resolve this in the component layer.
That is, on the CSS level, we can define a red var, and then we will define a message that is an error and uses red or a destructive button that also uses red.
Let me know if this sounds reasonable.
There was a problem hiding this comment.
Yes it is how align works, and destructive actually doesn't exists in align. They just call it error which uses the red color palette. I kept here the current destructive naming during the migration period. It will be removed in favor of the align tokens for errors
| lighter: 'hsl(var(--away-lighter))', | ||
| }, | ||
| error: { | ||
| DEFAULT: 'hsl(var(--error))', |
There was a problem hiding this comment.
Please refer to my previous comment about adding too many tokens and variables on the CSS level.
There was a problem hiding this comment.
I would highly suggest keeping the align UI based approach, as it will ensure on modification of a color token all relevant components will be updated, in-case we would want to change things like color-feature for example. it is mapped perfectly to what naveen is having on Figma and it will simplify our maintenance for new components assell. As copy-paste from align UI snippets will work out of the box.
Yes, I mean add what makes sense for us. Align ui provides 100x what we need, to I don't understand why we need to use the colors in the boilerplate here. It was supposed to be a starting point and we would create our own design system. This never happened. |
Makes sense, I agree. |
|
Team, I'm merging this PR as is. To maintain consistency and speed up front-end implementation, let's avoid creating custom patterns when using the Align UI design system. Following their definitions will help us achieve a more uniform UI with less work. I'm currently updating some of our components to align their prop terminology with the design system, ensuring that our Figma screens and code components use similar language. Once I have a proof of concept ready, I'll schedule an engineering session to review it and establish guidelines for modifying design system components and going forward. |
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer