-
Notifications
You must be signed in to change notification settings - Fork 15
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
chore: refactoring css and improving build to adhere to workspace conventions #676
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
da82d2b
to
54c9393
Compare
@@ -1,7 +1,7 @@ | |||
import type { Preview } from '@storybook/react'; | |||
|
|||
import '../docs/fonts/fonts.scss'; | |||
import '../src/css/design-tokens.css'; | |||
import '../src/css/index.css'; |
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.
Updating paths in storybook, forgot to rename font.scss
=> font.css
when we removed the sass dependency in #667
@@ -43,7 +43,7 @@ Currently the metamask design tokens repo supports 2 formats, CSS-in-JS and CSS | |||
> _Please note the file path will depend on where in your project you are importing it from._ | |||
|
|||
```css | |||
@import '../../node_modules/@metamask/design-tokens/src/css/design-tokens'; | |||
@import '@metamask/design-tokens/styles'; |
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.
Updating docs to use new vanity import path
@@ -22,19 +22,19 @@ | |||
"require": "./dist/index.js" | |||
}, | |||
"./package.json": "./package.json", | |||
"./design-tokens.css": "./src/css/design-tokens.css" | |||
"./styles.css": "./dist/styles.css" |
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 a breaking change and will constitute a major version update e.g. v3 if we don't want to release it as a major version we could keep the work around in the yarn workspace file and keep both. Thoughts?
], | ||
"scripts": { | ||
"build": "tsup --clean && yarn build:types", | ||
"build": "tsup --clean && yarn build:types && yarn build:css", |
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.
Including the build css script using clean-css-cli
--brand-colors-yellow-yellow400: #ffdf70; | ||
--brand-colors-yellow-yellow500: #ffd33d; | ||
--brand-colors-yellow-yellow600: #ffc70a; | ||
} |
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.
Separating brand colors out into a separate stylesheet. This will give us the oppurtunity to create a brand-colors-deprecated.css
stylesheet in the next release of the design tokens where we can keep track of the 000
token names and any other deprecated tokens
@@ -0,0 +1,65 @@ | |||
/** |
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.
Separating out dark theme variables
@import './light-theme-colors.css'; | ||
@import './dark-theme-colors.css'; | ||
@import './typography.css'; | ||
@import './shadow.css'; |
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.
Creating an index to store all of the separate stylesheets
@@ -0,0 +1,81 @@ | |||
/* |
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.
Light theme colors
@@ -0,0 +1,10 @@ | |||
/** |
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.
Shadows
@@ -275,7 +275,7 @@ module.exports = defineConfig({ | |||
|
|||
// The list of files included in the package must only include files | |||
// generated during the build process. | |||
workspace.set('files', ['dist', 'src/css/design-tokens.css']); // TODO remove src/css/design-tokens.css and add to build script | |||
workspace.set('files', ['dist']); |
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.
We can now align with the metamask-module-template workspace because everything is exported from dist
ab93849
to
804a7a7
Compare
@SocketSecurity ignore npm/[email protected] |
import '../../node_modules/@metamask/design-tokens/dist/styles.css'; | ||
``` | ||
|
||
This new path points to the `dist` directory, ensuring that you're importing the most optimized and production-ready version of the stylesheet. |
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.
Adding migration guide on how to effectively update the design tokens in CSS
Description
This PR undertakes a significant refactor of our CSS architecture within the design tokens system. By breaking up the design token categories into smaller, more manageable stylesheets and exporting them from a central
design-token.css
file, we aim to enhance the maintainability and scalability of our styling infrastructure. Additionally, this refactor includes improvements to our build pipeline, specifically optimizing the process to minimize our CSS output and aligning the export structure with the module template workspace conventions. This change not only streamlines our development workflow but also ensures our design tokens are efficiently integrated and utilized across projects, contributing to a more cohesive and consistent implementation of our design system.Related issues
Fixes: #675
Manual testing steps
To verify the successful refactor and integration of the design tokens:
dist
directory.Screenshots/Recordings
After
Storybook still works as expected
after720.mov
Updating package in portfolio works
after.portfolio720.mov
Updating package in extension works
after720.mov
Pre-merge author checklist
Pre-merge reviewer checklist
dist
, test code being changed).This PR represents a pivotal enhancement in our approach to managing and implementing design tokens, setting a foundation for more efficient, consistent, and scalable use of our design system across projects.