fix: Update mappings to system tokens instead of theme tokens #1314
Conversation
sandrahoang686
changed the title
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
sandrahoang686
changed the title
sandrahoang686
requested review from
hanbyul-here,
dzole0311,
AliceR and
faustoperez
AliceR
changed the title
AliceR
added
the
design system change
| $veda-uswds-color-primary-darker: utils.color('blue-warm-80v'); | ||
| $veda-uswds-color-secondary: utils.color('red-50'); | ||
| $veda-uswds-color-base-dark: utils.color('gray-cool-60'); | ||
| $veda-uswds-color-base-darkest: utils.color('gray-cool-80'); // re-mapped, theme-token 'base-darkest' originally mapped to 'gray-90' |
There was a problem hiding this comment.
'originally' as in 'the uswds default', right?
With the introduction of our $veda-uswds- variables we already decided leave the system, hence I agree, it does not matter what value we map that to.
Whenever we use one of the $veda-uswds- variables it indicates that we deviate from uswds, doesn't it? Do we need to keep track of what exactly the default was in the comments? 🤔
How do we make sure, that when we introduce a new USWDS component, it has the same colors etc. as the existing ones?
Just thinking out loud here.
There was a problem hiding this comment.
Following the USWDS default (to some extent) was an intentional choice for this first template instance, but that could not be the case in the future.
I'm thinking about the earth.gov work or any customization for, let's say, the Air Quality instance.
To me, it would make sense that we override all default values for colors, typographies, etc. (even if they point the same default system values) so we don't have to keep track of the changes and have a single configuration file for each instance.
There was a problem hiding this comment.
@AliceR not necessarily are we deviating from uswds, we are mostly sticking to it with just a very few looking like they are being re-mapped. This file is just us creating references to uswds tokens so we dont have to import uswds-core in each component sass file when we want to use their tokens (also nicely organized so you can see all the tokens we use from uswds in one file). if we need to overwrite a specific style in a uswds component but not across uswds components that use the same token, we can just use these vars to make that change inside that specific component's sass file like here.
When introducing a new uswds component - if they use something we have already re-mapped in the settings file, it should just get that change. Hopefully that helps answer your thoughts.
There was a problem hiding this comment.
Thanks for handling @sandrahoang686
Just a quick heads up: we’ll likely need a dedicated uswds-theme.scss configuration file per instance to customizes the components.
So a possible approach would be:
- Veda-UI ships it's own default styles bundled into one .css file (like we do now)
- Each instance includes its own
_uswds-theme.scssfile to override default styles (SCSS is needed to process the USWDS token names into pure CSS values)
We will try out the theming options we have via #1317.
Description of Changes
I've updated our sass variable references to map directly to system tokens instead of theme tokens because of what was discussed during USWDS sync today to match the mappings in the figma. But i'm indifferent to these changes and have no strong opinions to whether or not they map to the theme or system tokens.
That is because whether our references map to the theme or system tokens, it is the same outcome. BUT we do need to know what theme tokens are RE-MAPPED to different system token values instead of their defaults because we should still make that update in the settings file.
For example, if our reference of theme
base-darkestis re-mapped to a different system token value - any components used out of the box from USWDS that uses theme token colorbase-darkestshould also be re-mapped. And then because we forward@forward 'uswds-theme';in thetheme-varsfile, it would get that update in time. Which is why i'm indifferent to these changes.Any strong opinions / thoughts about this @hanbyul-here @dzole0311 @AliceR ? cc @faustoperez
Notes & Questions About Changes
{Add additonal notes and outstanding questions here related to changes in this pull request}
Validation / Testing
{Update with info on what can be manually validated in the Deploy Preview link for example "Validate style updates to selection modal do NOT affect cards"}