Admin UI - Tokens #4302
Admin UI - Tokens #4302
Conversation
packages/admin-ui/tailwind.config.js
Outdated
| @@ -20,48 +33,22 @@ module.exports = { | |||
| } | |||
| }, | |||
| extend: { | |||
There was a problem hiding this comment.
Should we add our tokens and override the default class names?
As we agreed not to use Tailwind's default class names, I would remove them to highlight the missing token definitions.
There was a problem hiding this comment.
Moved everything to overrides (out of the extend section).
| @apply border-border; | ||
| } | ||
|
|
||
| body { |
There was a problem hiding this comment.
Do we need to define the CSS variable in this file and admin-ui/src/styles.scss?
Also, is this the correct place to define the font family import? I would keep this together with the CSS variable definition.
Both of these might come from the POC phase. Shall we discuss it now? I'm happy to talk about it.
There was a problem hiding this comment.
Sounds reasonable.
Why do we need a copy of everything in App.scss when we already have it in admin-ui package? And why don't we abstract the font import from users and have it in single styles.scss?
It's basically because of real Webiny projects and being in sync with the code we'll be shipping.
In real projects, users will need to have the ability to customize the theme, and because of that, this is how their App.scss file will look like. Basically:
- all of the CSS vars are listed in the file
- the default font import statement is here as well, just so they can remove it and import their own font (no point in loading our default font if they will be using their own). That's why we don't have this in our
styles.scss.
Note
I see I forgot to update packages/cwp-template-aws/template/common/apps/admin/src/App.scss, I'll do it during the day.
So, basically, all of this was just to have our App.scss and users' App.scss in sync.
But.... all that being said... maybe we could rethink this a bit. Maybe we could actually not-have a bunch of CSS vars and font import by default in users' projects, but maybe we ship an SCSS file that's imported in App.scss. And then, if users want to customize things, they remove the default theme import, and paste all of the CSS vars and a font import statement (we'd have this documented) and then they proceed to tweak things.
So something like:
// apps/admin/src/App.scss
@import "~@webiny/admin-ui/styles.scss";
// To customize the theme, remove the following line and paste the code from https://docs.webiny.com/xyz
@import "~@webiny/admin-ui/theme.scss";
BTW, now that we've discussed this, I think this way of doing things might also resolve an issue where we have a bunch of TW vars duplicated, which is visible in console:
This just crossed my mind while I was writing this.
There was a problem hiding this comment.
I like the idea of having a separate file for theming 🎉
Regarding the duplicated Tailwind CSS variables: I’m not sure; I would expect that some override would be the default behavior of Tailwind. Also, while checking Tailwind themes, I noticed the same situation.
I would keep this as low priority if you agree,
There was a problem hiding this comment.
This is done as well. I also noted the low priority thingie here.
This PR introduces the new tokens.
Initially we started by manually moving them from Figma designs into code, but very quickly, I realized it's just too cumbersome and error prone to do it that way. So I created a script that automates this.
Basically, users first export all of the variables from Figma, using this tool. Then, they copy the received file it into our project, and run
yarn webiny-admin-import-from-figma.As we can see, the script generates three files, where the most important two are
tailwind.config.jsandstyles.scss.I documented how to use it here.