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

chore: Implement Tailwind CSS (WIP) #30170

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

chore: Implement Tailwind CSS (WIP) #30170

wants to merge 13 commits into from

Conversation

georgewrmarshall
Copy link
Contributor

@georgewrmarshall georgewrmarshall commented Feb 6, 2025

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:

  1. Integrating Tailwind CSS configuration and dependencies
  2. Setting up the necessary build pipeline changes
  3. Demonstrating practical usage in component stories
  4. Providing a foundation for the proposed ADR (MetaMask/decisions#44)

The changes include:

  • Adding Tailwind CSS and related dependencies
  • Configuring PostCSS for Tailwind processing
  • Updating build configurations
  • Demonstrating utility class usage in Box component stories
  • Setting up design system token integration

Slack post

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3851

Manual testing steps

  1. Pull branch and install dependencies
  2. Run Storybook (yarn storybook)
  3. Navigate to Box component stories
  4. Verify Tailwind utility classes are working in the demo story
  5. Verify build process completes successfully

Screenshots/Recordings

Before

TBC

After

TBC

Pre-merge author checklist

  • I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards
  • I've completed the PR template to the best of my ability
  • I've included examples demonstrating the implementation
  • I've documented configuration changes and setup requirements
  • I've applied the appropriate labels (POC, enhancement, build)

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 demonstrates a working proof of concept for Tailwind CSS implementation as described in the related ADR and planning issue

@georgewrmarshall georgewrmarshall added the team-design-system All issues relating to design system in Extension label Feb 6, 2025
@georgewrmarshall georgewrmarshall self-assigned this Feb 6, 2025
Copy link
Contributor

github-actions bot commented Feb 6, 2025

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.

Copy link

socket-security bot commented Feb 6, 2025

👍 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: npm/[email protected], npm/@alloc/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/@metamask/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/@jridgewell/[email protected], npm/[email protected]

View full report↗︎

Next steps

Take a deeper look at the dependency

Take 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 package

If 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 risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@georgewrmarshall
Copy link
Contributor Author

@SocketSecurity ignore-all

  • Marking following packages as acceptable risk as we are protected from supply chain attack by lavamoat
Screenshot 2025-02-06 at 2 13 16 PM

@georgewrmarshall georgewrmarshall changed the title Implement Tailwind CSS (WIP) chore: Implement Tailwind CSS (WIP) Feb 6, 2025
@georgewrmarshall georgewrmarshall added the DO-NOT-MERGE Pull requests that should not be merged label Feb 6, 2025
Comment on lines +92 to +99
{
loader: 'postcss-loader',
options: {
postcssOptions: {
plugins: ['tailwindcss', 'autoprefixer'],
},
},
},
Copy link
Contributor Author

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

Comment on lines +89 to +94
postcss([
tailwindcss(tailwindConfig),
autoprefixer(),
rtlcss(),
discardFonts(['woff2']),
]),
Copy link
Contributor Author

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(),
Copy link
Contributor Author

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": {
Copy link
Contributor Author

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]",
Copy link
Contributor Author

@georgewrmarshall georgewrmarshall Feb 6, 2025

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",
Copy link
Contributor Author

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: [],
};
Copy link
Contributor Author

@georgewrmarshall georgewrmarshall Feb 6, 2025

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:

  1. Tells Tailwind to scan UI files and MetaMask's design system files for CSS classes
  2. Uses MetaMask's design system preset for styling
  3. 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}',
Copy link
Contributor Author

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"
Copy link
Contributor Author

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';
Copy link
Contributor Author

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

@@ -1,7 +1,7 @@
/*
MetaMask design system imports
*/
@import './reset.scss';
@import './tailwind';
Copy link
Contributor Author

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;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default Tailwind stylesheet

@metamaskbot
Copy link
Collaborator

Builds ready [b0e335f]
Page Load Metrics (1557 ± 49 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint24918841499304146
domContentLoaded1397181315299144
load14061875155710349
domInteractive22422752
backgroundConnect862312010
firstReactRender1570332311
getState445894
initialActions00000
loadScripts1014134211138038
setupStore75315157
uiStartup15702096177911857
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@@ -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';
Copy link
Contributor Author

@georgewrmarshall georgewrmarshall Feb 7, 2025

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

@metamaskbot
Copy link
Collaborator

Builds ready [37274af]
Page Load Metrics (1750 ± 89 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint41221101671339163
domContentLoaded14242082171618187
load14332105175018589
domInteractive2599392210
backgroundConnect979372110
firstReactRender166833199
getState573242110
initialActions01000
loadScripts9931613125715976
setupStore66417178
uiStartup16072359199219493
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [aa9c4bb]
Page Load Metrics (1773 ± 137 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint35524671701426205
domContentLoaded140222911751279134
load143023811773286137
domInteractive23195533818
backgroundConnect7100282211
firstReactRender1578392412
getState45410115
initialActions01000
loadScripts101017571296233112
setupStore76619209
uiStartup159825942010328157
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [cb199aa]
Page Load Metrics (1736 ± 58 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14721954173711957
domContentLoaded14621934170512058
load14731953173612058
domInteractive26134453015
backgroundConnect996302613
firstReactRender1699412914
getState56514168
initialActions01000
loadScripts10061468123910751
setupStore890322713
uiStartup17912599206018388
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO-NOT-MERGE Pull requests that should not be merged team-design-system All issues relating to design system in Extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants