-
Notifications
You must be signed in to change notification settings - Fork 612
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
Admin UI - Tokens #4302
Open
adrians5j
wants to merge
67
commits into
feat/new-admin-ui
Choose a base branch
from
feat/tokens
base: feat/new-admin-ui
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Admin UI - Tokens #4302
Changes from 60 commits
Commits
Show all changes
67 commits
Select commit
Hold shift + click to select a range
33b23d6
wip: create Avatar component
adrians5j 348b74f
Merge remote-tracking branch 'refs/remotes/origin/feat/new-admin-ui' …
adrians5j 936f83e
wip: create Avatar component
adrians5j 0abec5f
fix: remove needless `cn` usage
adrians5j 1a550cb
wip: create Avatar component
adrians5j a3ff702
wip: create Avatar component
adrians5j d89b81e
wip: create Avatar component
adrians5j 83537b8
Merge branch 'feat/new-admin-ui' into feat/avatar
adrians5j 04e7e94
wip: create Avatar component
adrians5j 3a6898a
wip: create Avatar component
adrians5j 83ad097
wip: create Avatar component
adrians5j 5dd474b
Merge remote-tracking branch 'refs/remotes/origin/feat/new-admin-ui' …
adrians5j a95a914
fix: update dependencies
adrians5j 5f002a4
wip: create Avatar component
adrians5j d8f6de5
chore: run prettier
adrians5j 9a432c8
wip: remove avatar-related changes
adrians5j aaffef7
wip: remove avatar-related changes
adrians5j 63873db
wip: remove avatar-related changes
adrians5j 2c993d6
wip: remove avatar-related changes
adrians5j ed4638d
wip: remove avatar-related changes
adrians5j 17e8f8d
wip: remove avatar-related changes
adrians5j e675a7a
wip: remove avatar-related changes
adrians5j 9fb83be
wip
adrians5j 24ce7d1
wip - colors placeholders
adrians5j a63753f
wip
adrians5j cd42084
wip
adrians5j 82b5481
wip
adrians5j c9352e8
wip
adrians5j 2b284e5
wip
adrians5j 5b802ae
wip
adrians5j 2fe7a33
wip
adrians5j c89186f
wip
adrians5j b7a3462
wip
adrians5j 0523b82
wip
adrians5j 61bce7b
wip
adrians5j 0320857
wip
adrians5j 451188a
wip
adrians5j 0bc0441
wip
adrians5j 366308e
wip
adrians5j 2cc9434
wip
adrians5j 6fa6ed4
wip
adrians5j 5261222
wip
adrians5j c5e6486
wip
adrians5j 08d10d0
wip
adrians5j f5795d9
wip
adrians5j ba2ff84
wip
adrians5j 4051407
Merge branch 'feat/new-admin-ui' into feat/tokens
adrians5j b53d791
wip
adrians5j 8b04ace
wip
adrians5j 86a1579
wip
adrians5j 50a6ffb
wip
adrians5j 15dd5f0
Merge remote-tracking branch 'origin/feat/tokens' into feat/tokens
adrians5j e400c74
wip
adrians5j 3afc7b3
wip
adrians5j 369671e
wip
adrians5j bcd1d5a
wip
adrians5j dd15b89
wip
adrians5j 3d6973f
wip
adrians5j 99d6dab
wip
adrians5j e3d6535
wip
adrians5j 044c9e7
wip
adrians5j 1e88e44
wip
adrians5j 2576141
wip
adrians5j ea3b6ed
wip
adrians5j 2e844f4
wip
adrians5j 5626e88
wip
adrians5j b34dca6
wip
adrians5j File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
@import url("https://fonts.googleapis.com/css2?family=Inter:wght@400;600&display=swap"); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# Overview | ||
This document covers different aspects of the development process for the `@webiny/admin-ui` package. | ||
|
||
## Tailwind CSS | ||
Webiny's Admin app uses Tailwind CSS for styling. | ||
|
||
### Tailwind Configuration | ||
Tailwind CSS is configured as usual, via the [`tailwind.config.js`](./tailwind.config.js) file. Among other things, this file defines the default Webiny theme, which is based on Webiny's design system. More on this in the next section. | ||
|
||
### Default Theme | ||
One of the main things we define via the [`tailwind.config.js`](./tailwind.config.js) file is the default theme, which is based on Webiny's design system, and which can be found in this [Figma file](https://www.figma.com/file/f0QUDWX37Kt5X53eltTRiT/Webiny-Design-System?type=design&node-id=127-26352&mode=design&t=nhoOU7NamjWvImoW-0). | ||
|
||
But, do note that all the default theme-values are not actually defined in the `tailwind.config.js` file, but rather in the `tailwind.config.theme.js` file, which is a file that is generated from a Figma export (more on this in the next section). | ||
|
||
### Figma To Code | ||
Since manually transferring values from the mentioned Figma file into code has shown to be a very cumbersome process, we've created a script that basically takes a Figma export and generates all the necessary Tailwind CSS configuration. Note that when we say "Figma export", we basically mean an export of Alias tokens, created by this [Export/Import Variables](https://www.figma.com/community/plugin/1256972111705530093/export-import-variables) plugin. | ||
|
||
Once the export is downloaded, we place it in `packages/admin-ui/scripts/importFromFigma/exports/Alias tokens.json`, and then we run the following command from project root: | ||
|
||
```bash | ||
yarn webiny-admin-import-from-figma | ||
``` | ||
|
||
This will generate a new `tailwind.config.theme.js` file, which will contain all the necessary Tailwind CSS configuration. On top of that, it will also generate a `src/styles.scss` file, which contains actual values for CSS variables that are referenced in the `tailwind.config.theme.js` file. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,10 @@ | ||
# @webiny/admin-ui | ||
[![](https://img.shields.io/npm/dw/@webiny/admin-ui.svg)](https://www.npmjs.com/package/@webiny/admin-ui) | ||
[![](https://img.shields.io/npm/v/@webiny/admin-ui.svg)](https://www.npmjs.com/package/@webiny/admin-ui) | ||
[![code style: prettier](https://img.shields.io/badge/code_style-prettier-ff69b4.svg?style=flat-square)](https://github.com/prettier/prettier) | ||
[![PRs Welcome](https://img.shields.io/badge/PRs-welcome-brightgreen.svg?style=flat-square)](http://makeapullrequest.com) | ||
|
||
## Install | ||
``` | ||
npm install --save @webiny/admin-ui | ||
``` | ||
|
||
Or if you prefer yarn: | ||
``` | ||
yarn add @webiny/admin-ui | ||
``` | ||
# `@webiny/admin-ui` | ||
|
||
This package contains React components and styles for Webiny's Admin app. | ||
|
||
> [!NOTE] | ||
> This package is included in every Webiny project by default, and it's not meant to be used as a standalone package. | ||
|
||
|
||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable.
Why do we need a copy of everything in
App.scss
when we already have it inadmin-ui
package? And why don't we abstract the font import from users and have it in singlestyles.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: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:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is done as well. I also noted the low priority thingie here.