Skip to content
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

[WNMGDS-2669] Make token changes informed by Figma migration #2927

Merged
merged 14 commits into from
Feb 13, 2024

Conversation

pwolfert
Copy link
Collaborator

@pwolfert pwolfert commented Feb 9, 2024

Summary

  • Reorganized and renamed some tokens (and therefore their CSS vars)
  • Updated some token values and CSS so no visual changes occur

Renames

From To
--table-striped__background-color --table__background-color--striped
--table-striped-header__background-color --table-header__background-color--striped
--form-hint__color --hint__color
--form-hint__color--inverse --hint__color--inverse
--form-error__color --inline-error__color
--form-error__color--inverse --inline-error__color--inverse

Removals

  • --dialog-icon__size
  • --icon__color--error
  • --icon__color--inverse
  • --icon__color--primary
  • --icon__color--success
  • --icon__color--warn
  • --label__color--inverse

Changed values

  • Value of --alert__icon-size now uses rems instead of px and accurately reflects the final size of those icons

How to test

  1. Instructions on how to test the changes in this PR.

Checklist

  • Prefixed the PR title with the Jira ticket number as [WNMGDS-####] Title or [NO-TICKET] if this is unticketed work.
  • Selected appropriate Type (only one) label for this PR, if it is a breaking change, label should only be Type: Breaking
  • Selected appropriate Impacts, multiple can be selected.
  • Selected appropriate release milestone

@pwolfert pwolfert added Type: Breaking This item causes a breaking change. Impacts: Core Impacts the core DS primarily, changes may occur in other themes as well. labels Feb 9, 2024
@pwolfert pwolfert added this to the 10.0.0-beta.1 milestone Feb 9, 2024
Copy link
Collaborator

@zarahzachz zarahzachz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran yarn test:browser:all and got 1 failure

~[webkit] › stories.test.ts:72:13 › Components/Button/All Anchor Buttons › with core theme › matches snapshot ~

it doesn't make a whole lotta sense given you didn't change any button tokens though

scratch that - all tests pass a 2nd time through

@@ -280,7 +280,6 @@ export const components: AnyTokenValues = {
'__background-color': t.color['white'],
'__padding': t.spacer['4'],
'-overlay__background-color': t.color['background-dialog-mask'],
'-icon__size': '0.8125rem',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤯

@pwolfert
Copy link
Collaborator Author

@zarahzachz, what was the visual error you got? What did it look like? I ran the full VRTs on my own computer and didn't get anything.

These didn't actually change content-wise, but storybook wants to render them slightly differently. I do believe the code this is based on is deterministic, but it's just not something we can practically determine on our own 😆
@pwolfert pwolfert merged commit aa7c928 into main Feb 13, 2024
1 check passed
@pwolfert pwolfert deleted the pwolfert/clean-up-tokens branch February 13, 2024 01:49
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impacts: Core Impacts the core DS primarily, changes may occur in other themes as well. Type: Breaking This item causes a breaking change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants