chore: adding tailwind css guidelines #144
Conversation
georgewrmarshall
added
the
team-design-system
georgewrmarshall
force-pushed
the
tailwind-css
branch
from
4e19359 to
bac6091
Compare
|
|
||
| // Layout and spacing | ||
| // Interactive states (react supports hover/active) | ||
| <ButtonBase className="h-auto flex-1 flex-col justify-center rounded-lg bg-muted py-4 hover:bg-muted-hover active:bg-muted-pressed"> |
There was a problem hiding this comment.
What is this example showing? How to use className on a component? Or something more specific?
There was a problem hiding this comment.
Great question! This could be improved, I've updated the comments here
// Use the className prop to override existing tailwind classnames when necessary
// Example overriding the default size/shape, layout and interactive states
docs/tailwind-css.md
Outdated
| - **No Direct Styles**: Avoid inline `style` objects | ||
| - **No @apply**: Don't use `@apply` in CSS files | ||
| - **Avoid Style Mixing**: Try to avoid mixing Tailwind with other styling approaches like inline styles in the same component when not necessary. Style mixing may be necessary for custom animations or dynamic values that can't be achieved with Tailwind alone. However, combining component props with Tailwind classes via `className`/`twClassName`/`tw` is acceptable when no equivalent prop exists | ||
| - **No Default Colors**: Never use Tailwind's default color palette |
There was a problem hiding this comment.
Isn't this covered under the "Color and Typography" section above?
There was a problem hiding this comment.
You're absolutely right! There is some duplication here. I might be overly sensitive to static color/font size pitfalls because we have so many of them in across our apps causing small but many inconsistencies 😅 I've removed them here
|
|
||
| **React:** | ||
|
|
||
| ❌ **Avoid** |
There was a problem hiding this comment.
I see that in these examples, "avoid" is given first and then "recommended" follows, but above, the order is "recommended" and then "avoid". It might be helpful to make these consistent. I believe in other documents we tend to do "avoid" -> "recommended".
There was a problem hiding this comment.
Great catch! I've re-ordered so these docs follow the "avoid" -> "recommended" convention here . Thank you
|
|
||
| ### Anti-patterns to Avoid | ||
|
|
||
| - **No Arbitrary Values**: Don't use `[]` syntax for arbitrary values unless absolutely necessary |
There was a problem hiding this comment.
Maybe not worth doing now but it might be nice to provide examples for all of these guidelines.
There was a problem hiding this comment.
Great suggestion! I've added examples here
georgewrmarshall
dismissed stale reviews from cryptodev-2s and brianacnguyen
via
efefecd
Thank you for the thorough and thoughtful review @mcmire! Here is a recording of the changes based on your suggestions 🙏 after720.mov |
georgewrmarshall
requested review from
brianacnguyen,
Copilot,
cryptodev-2s and
mcmire
There was a problem hiding this comment.
Pull Request Overview
This PR establishes comprehensive Tailwind CSS guidelines to standardize styling practices across MetaMask's engineering organization as part of their design system unification effort. The documentation provides clear conventions for using Tailwind CSS effectively with their design system components across React and React Native platforms.
- Component-first approach emphasizing design system components over raw Tailwind classes
- Platform-specific implementation patterns for React (className) and React Native (useTailwind hook)
- Developer tooling configuration and common anti-patterns to avoid
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
## **Description** This PR adds the ESLint Tailwind CSS plugin to enforce the Tailwind CSS best practices and some of the guidelines established in the [MetaMask contributor docs](MetaMask/contributor-docs#144). The plugin provides automated linting to ensure consistent usage of Tailwind CSS classes across the MetaMask Extension codebase. **Key changes:** - Added `eslint-plugin-tailwindcss` and its TypeScript definitions - Configured ESLint rules to enforce Tailwind CSS best practices including: - Class name ordering for consistency - Shorthand enforcement (e.g., `p-4` instead of `px-4 py-4`) - Prevention of contradicting class names - Elimination of unnecessary arbitrary values - Applied automatic fixes to existing code to comply with the new linting rules This change supports the broader design system unification effort by ensuring consistent Tailwind CSS usage across all MetaMask applications, improving code maintainability and reducing styling inconsistencies. [](https://codespaces.new/MetaMask/metamask-extension/pull/34211?quickstart=1) ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: #34210 ## **Manual testing steps** 1. Pull the branch and install dependencies with `yarn install` 2. Run ESLint on files with Tailwind classes: `yarn lint` 3. Verify that Tailwind CSS class violations are flagged as errors 4. Test that the automatic fixes maintain visual consistency by running Storybook: `yarn storybook` 5. Navigate to Design System stories and verify the styling remains unchanged ## **Screenshots/Recordings** ### **Before** - ESLint would not catch Tailwind CSS type errors https://github.com/user-attachments/assets/fe88a393-2865-4db6-adee-50156c11afc2 ### **After** - ESLint now enforces Tailwind CSS best practices automatically - Classes are optimized (e.g., `px-4 py-4` → `p-4`, proper class ordering) - Consistent code style across the codebase https://github.com/user-attachments/assets/01a1f3d8-0f74-4059-a5ba-f65d2b60cd50 ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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.
](https://codespaces.new/MetaMask/metamask-extension/pull/34211?quickstart=1){kind=link}
Description
We are adopting Tailwind CSS as the main styling framework across the MetaMask engineering organization, aligning with the release of our design system libraries and their integration into mobile, extension, portfolio, and developer documentation projects. This documentation will be referenced across all 4 repos.
Context
As part of our design system unification effort, we need comprehensive documentation to guide engineers in using Tailwind CSS effectively and consistently. This change supports the broader initiative to standardize our styling approach across all MetaMask applications, ensuring consistency in implementation and maintainability.
docs720.mov