MetaMask / MetaMask/metamask-design-system
Clarify styling rules: twClassName (static) vs style with tw.style() (dynamic)
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 37
- Forks
- 14
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 60
Description
Background
Current .cursor/rules/styling.md states:
NEVER store classes in variables - Breaks VSCode Tailwind IntelliSense and linting
However, this rule is ambiguous and the codebase has inconsistent patterns. We need clear guidelines on when to use twClassName vs style with tw.style() in React Native components.
The Problem
What's currently unclear:
- When should components use
twClassNameprop? - When should components use
styleprop withtw.style()? - Are map lookups (const objects) considered "storing in variables"?
- Is storing merged/concatenated class strings acceptable?
Proposed Clarification
✅ twClassName - Pure static strings only
Use twClassName when passing static, unchanging Tailwind classes:
<Box twClassName="rounded-sm border-2 px-4">
<Text>Static styling</Text>
</Box>
✅ style with tw.style() - Dynamic/conditional classes
Use style array pattern with tw.style() when you need:
- Conditionals (pressed, isActive, isDanger, etc.)
- Map lookups (severity → color mapping)
- Merging with user's style prop
// Good: Map lookup (const object is fine)
const borderColorClass = MAP_BANNER_ALERT_SEVERITY_BORDER_COLOR[severity];
return (
<BannerBase
style={[
tw.style(`border-l-4 ${borderColorClass}`),
style, // User's custom style
]}
{...props}
/>
);
❌ Don't store merged/concatenated results
Avoid storing the result of string concatenation/merging:
// ❌ Bad - Stores merged result
const mergedCloseButtonTwClassName = closeButtonTwClassName
? `ml-3 ${closeButtonTwClassName}`
: 'ml-3';
// ✅ Good - Inline in tw.style()
style={[tw.style('ml-3', closeButtonTwClassName), style]}
Components Needing Review
Found these components storing merged/conditional class strings in const variables:
1. BannerBase.tsx (lines 66-68, 77)
// Line 66-68: Stores merged className
const mergedCloseButtonTwClassName = closeButtonTwClassName
? `ml-3 ${closeButtonTwClassName}`
: 'ml-3';
// Line 77: Inline concatenation in prop
twClassName={twClassName ? `rounded-sm ${twClassName}` : 'rounded-sm'}
Suggested fix:
// Use style array pattern
style={[tw.style('ml-3', closeButtonTwClassName), style]}
2. ButtonIcon.tsx (lines 40-46)
// Stores conditional class assignments
const twIconColorClassNames =
variant === ButtonIconVariant.Floating
? 'text-primary-inverse'
: 'text-icon-default';
const borderRadiusClass =
variant === ButtonIconVariant.Default ? 'rounded-lg' : 'rounded-full';
Suggested fix:
// Inline in tw.style()
tw.style(
variant === ButtonIconVariant.Floating
? 'text-primary-inverse'
: 'text-icon-default',
variant === ButtonIconVariant.Default
? 'rounded-lg'
: 'rounded-full',
)
3. HeaderBase.tsx (lines 109-112)
// Stores merged base + user className
const baseStyles = 'flex-row items-center gap-4 h-14';
const resolvedTwClassName = twClassName
? `${baseStyles} ${twClassName}`
: baseStyles;
Suggested fix:
// Use tw.style() in array
style={[
tw.style('flex-row items-center gap-4 h-14', twClassName),
includesTopInset && { marginTop: insets.top },
style,
]}
4. Checkbox.tsx (lines 80-84)
// Stores tw.style() result
const twContainerClassNames = tw.style(
'flex-row items-center',
isDisabled ? 'opacity-50' : 'opacity-100',
twClassName,
);
Note: This stores the computed style object, not a class string. Less clear if problematic, but could be inlined.
Reference Implementation
See BannerAlert component (PR #XXX) for the recommended pattern:
- Uses map lookup for dynamic border color (✅ acceptable)
- Uses
style={[tw.style(...), style]}array pattern - Keeps classes inline in
tw.style()call - Passes
twClassNamethrough{...props}to Box
Proposed Rule Updates
Update .cursor/rules/styling.md React Native section with:
-
Clear distinction:
twClassName= static strings onlystyle+tw.style()= dynamic/conditional/merged
-
Map lookups are fine:
- Const object lookups (MAP_SEVERITY_BORDER) are NOT "storing in variables"
- This is a lookup table pattern, not the anti-pattern
-
Avoid storing merged results:
- Don't store concatenated/merged class strings in const
- Keep classes inline in
tw.style()for IntelliSense
-
Array pattern for style merging:
- Use
style={[tw.style(...), style]}when merging with user style - Matches Box component foundational pattern
- Use
Acceptance Criteria
- Update
.cursor/rules/styling.mdwith clarified guidelines - Refactor BannerBase (lines 66-68, 77)
- Refactor ButtonIcon (lines 40-46)
- Refactor HeaderBase (lines 109-112)
- Review Checkbox (lines 80-84) - decide if acceptable
- Add examples showing correct patterns
- Verify all changes maintain 100% test coverage
References
- Current rule: .cursor/rules/styling.md
- BannerAlert refactor:
banner-alertbranch - Research findings: See plan mode conversation for 5 pattern analysis
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with .cursor/rules/styling.md and compare its current rule with BannerBase.tsx, ButtonIcon.tsx, HeaderBase.tsx, and Checkbox.tsx. Review the listed class and style patterns, then update the rule and refactor the specified components while deciding whether Checkbox's computed style is acceptable. Done means the acceptance criteria are met and the existing test suite retains 100% coverage.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react-native, typescript
- Domain
- documentation, mobile-dev
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100