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

feat: Update CSS variables with new tokens #691

Merged
merged 10 commits into from
May 22, 2024

Conversation

brianacnguyen
Copy link
Contributor

Description

This PR

  • updated the css variables with new tokens
  • updated the migration docs with critical changes

Related issues

Fixes: #687

Manual testing steps

  1. Go to storybook
  2. Checkout colors

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@brianacnguyen brianacnguyen added color Tokens related to color team-design-system All issues relating to design system labels May 22, 2024
@brianacnguyen brianacnguyen self-assigned this May 22, 2024
@brianacnguyen brianacnguyen requested a review from a team as a code owner May 22, 2024 00:51
@brianacnguyen brianacnguyen changed the base branch from main to tokens-v2 May 22, 2024 00:51
@metamaskbot
Copy link
Collaborator

Builds ready [11b2131]

Storybook: Storybook

@metamaskbot
Copy link
Collaborator

Builds ready [c9d5dad]

Storybook: Storybook

@metamaskbot
Copy link
Collaborator

Builds ready [91cc421]

Storybook: Storybook

@metamaskbot
Copy link
Collaborator

Builds ready [3635e45]

Storybook: Storybook

@metamaskbot
Copy link
Collaborator

Builds ready [bf2ab80]

Storybook: Storybook

- black000 renamed to black

### Brand Colors Brand Evolution
- Blue updates
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These brand colors brand evolution updates are compared against the updated brand colors

--brand-colors-blue-blue600: #0260a4;
--brand-colors-blue-blue700: #024272;
--brand-colors-blue-blue800: #01253f;
--brand-colors-blue-blue900: #00080d;
/* Orange */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reordered to match with json file

@metamaskbot
Copy link
Collaborator

Builds ready [babc482]

Storybook: Storybook

Copy link
Collaborator

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

Looks like storybook is still showing the current colors and not the brand evolution ones?

Screenshot 2024-05-22 at 8 44 13 AM

And in theme colors it's the old colors and something strange is going on with themeing it seems light mode is showing current dark mode colors

main branch

Screenshot 2024-05-22 at 8 45 45 AM

this branch

Screenshot 2024-05-22 at 8 46 10 AM

Comment on lines +37 to +44
- Grey updates
- grey000 added
- grey025 added
- grey050 added
- grey030 removed
- grey040 removed
- grey750 removed
- grey1000 added
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the final migration documentation, it would be beneficial to include specific code changes. Added
JS
Added

brandColor.grey000
brandColor.grey025

Removed

brandColor.grey030

CSS
Added

--brand-colors-grey-grey000 
--brand-colors-grey-grey025

Removed

--brand-colors-grey-grey030

This detail will aid engineers who are searching for removed tokens in the codebase. cc @garrettbear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you want me to update this to reflect that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tho the colors will be the same for both changes, the difference is syntax. curious if theres a better way to list this instead of duplicating it

@metamaskbot
Copy link
Collaborator

Builds ready [339f7e4]

Storybook: Storybook

@metamaskbot
Copy link
Collaborator

Builds ready [d52a01f]

Storybook: Storybook

@brianacnguyen brianacnguyen merged commit 762facd into tokens-v2 May 22, 2024
17 checks passed
@brianacnguyen brianacnguyen deleted the tokens-v2-new-tokens branch May 22, 2024 16:44
georgewrmarshall added a commit that referenced this pull request May 22, 2024
* feat: updated css color variables

* feat: updated migration doc with token changes

* fix: updated comments in css variables

* refactor: updated spacing

* feat: added descriptions as comments to themed tokens

* fix: removed newline

* fix: updated css format

* fix: updated lime800 color

* fix: removed root from dark colors

* adding line breaks in migration doc

---------

Co-authored-by: georgewrmarshall <george.marshall@consensys.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
color Tokens related to color team-design-system All issues relating to design system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update CSS variables with newly update brand evolution theme colors
3 participants