-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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: Implement Tailwind CSS (WIP) #30170
base: main
Are you sure you want to change the base?
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
👍 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 |
@SocketSecurity ignore-all
![]() |
eb28465
to
ae5ffd3
Compare
{ | ||
loader: 'postcss-loader', | ||
options: { | ||
postcssOptions: { | ||
plugins: ['tailwindcss', 'autoprefixer'], | ||
}, | ||
}, | ||
}, |
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.
Loading tailwind styles in storybook
postcss([ | ||
tailwindcss(tailwindConfig), | ||
autoprefixer(), | ||
rtlcss(), | ||
discardFonts(['woff2']), | ||
]), |
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.
Adds tailwind styles to the gulp build configuration process
@@ -289,6 +290,7 @@ const config = { | |||
options: { | |||
postcssOptions: { | |||
plugins: [ | |||
tailwindcss(), |
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 tailwind to the webpack styles build process
@@ -890,15 +890,15 @@ | |||
"gulp-sourcemaps>@gulp-sourcemaps/identity-map": { |
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 the lavamoat build system policy using yarn lavamoat:auto
@@ -300,6 +300,7 @@ | |||
"@metamask/browser-passworder": "^4.3.0", | |||
"@metamask/contract-metadata": "^2.5.0", | |||
"@metamask/controller-utils": "^11.4.0", | |||
"@metamask/design-system-tailwind-preset": "npm:@metamask-previews/[email protected]", |
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.
Importing a temporary preview build of the design system Tailwind preset to standardize all Tailwind configs across the organization and ensure alignment with the design system.
@@ -665,6 +666,7 @@ | |||
"style-loader": "^0.21.0", | |||
"stylelint": "^13.6.1", | |||
"superstruct": "^1.0.3", | |||
"tailwindcss": "^3.3.0", |
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 tailwind as a dependency. v4 has been release but we need to check compatibility with the design system preset and other platforms that use tailwind e.g. portfolio
extend: {}, | ||
}, | ||
plugins: [], | ||
}; |
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 the Tailwind CSS configuration file that:
- Tells Tailwind to scan UI files and MetaMask's design system files for CSS classes
- Uses MetaMask's design system preset for styling
- Removes default Tailwind colors (we only need the colors provided by the design system preset)
module.exports = { | ||
content: [ | ||
'./ui/**/*.{js,jsx,ts,tsx}', | ||
'./node_modules/@metamask/design-system-react/**/*.{cjs,mjs}', |
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 probably remove this if we want as we are no longer importing the design system react library
margin={2} | ||
padding={4} | ||
backgroundColor={BackgroundColor.backgroundAlternative} | ||
borderColor={BorderColor.borderMuted} | ||
className="m-2 p-4 bg-alternative border border-muted" |
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.
Example of replacing Box style utility props for tailwind classnames
@@ -1,7 +1,7 @@ | |||
/* | |||
MetaMask design system imports | |||
*/ | |||
@import './reset.scss'; |
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.
Tailwind CSS includes its own reset, so we can remove this one. In fact, there's a specificity issue where the reset is overriding Tailwind class names so we need to replace it in favor of the tailwind reset
ui/css/index.scss
Outdated
@@ -1,7 +1,7 @@ | |||
/* | |||
MetaMask design system imports | |||
*/ | |||
@import './reset.scss'; | |||
@import './tailwind'; |
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.
Importing the tailwind stylesheet into index.css
@@ -0,0 +1,3 @@ | |||
@tailwind base; | |||
@tailwind components; | |||
@tailwind utilities; |
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.
Default Tailwind stylesheet
Builds ready [b0e335f]
Page Load Metrics (1557 ± 49 ms)
Bundle size diffs
|
@@ -1,6 +1,6 @@ | |||
// This file is for Jest-specific setup only and runs before our Jest tests. | |||
import { jestPreviewConfigure } from 'jest-preview'; | |||
import '../config/assets/index.css'; | |||
// import '../config/assets/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.
Temporary fix for failing integration tests. Still need to figure out the timeout here. After this PR is merged it will be fixed but it may need to be adjusted in future #30214
Builds ready [37274af]
Page Load Metrics (1750 ± 89 ms)
Bundle size diffs
|
Builds ready [aa9c4bb]
Page Load Metrics (1773 ± 137 ms)
Bundle size diffs
|
aa9c4bb
to
cb199aa
Compare
Builds ready [cb199aa]
Page Load Metrics (1736 ± 58 ms)
Bundle size diffs
|
Description
This PR demonstrates a proof of concept implementation of Tailwind CSS in the MetaMask Extension. The implementation aims to modernize our styling approach and improve developer experience by:
The changes include:
Slack post
Related issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3851
Manual testing steps
yarn storybook
)Box
component storiesScreenshots/Recordings
Before
TBC
After
TBC
Pre-merge author checklist
Pre-merge reviewer checklist