Skip to content

Refactor to remove color set and simplify twrnc preset #719

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

Closed
9 tasks
georgewrmarshall opened this issue Jun 4, 2025 · 0 comments · Fixed by #722
Closed
9 tasks

Refactor to remove color set and simplify twrnc preset #719

georgewrmarshall opened this issue Jun 4, 2025 · 0 comments · Fixed by #722
Assignees

Comments

@georgewrmarshall
Copy link
Contributor

Description

The design system's theme handling in the twrnc preset currently includes an unused ColorSet enum that adds unnecessary complexity to the codebase. This abstraction was originally created to support multiple color sets (Brand, Neutral, etc.), but in practice, we only use the 'Brand' color set. This creates unnecessary nesting and type complexity that should be removed to simplify the codebase.

Technical Details

Current structure has unnecessary nesting:

export enum ColorSet {
  Brand = 'brand',
}

export const colorSetList: ColorSetListProps = {
  [ColorSet.Brand]: {
    [ColorScheme.Light]: flattenColors(lightTheme.colors),
    [ColorScheme.Dark]: flattenColors(darkTheme.colors),
  },
};

Changes needed:

  1. Remove ColorSet enum and related types
  2. Flatten color set list structure to directly map color schemes to colors
  3. Update generateTailwindConfig to work with ColorScheme directly
  4. Update ThemeProvider and ThemeContext to remove ColorSet references
  5. Update exports to remove ColorSet
  6. Convert font sizes to strings to match twrnc requirements

Acceptance Criteria

  • ColorSet enum and related types are removed
  • Color configuration is simplified to directly map ColorScheme to colors
  • Theme switching (light/dark) continues to work correctly
  • All components render with correct colors
  • Font sizes render correctly in all components
  • No breaking changes to public API
  • All tests pass
  • Storybook examples work correctly
  • Documentation is updated to reflect changes

References

  • Related to theme handling in design system
  • Affects @metamask/design-system-twrnc-preset package
  • Improves code maintainability by removing unused abstraction
  • Simplifies type system and reduces complexity
@georgewrmarshall georgewrmarshall changed the title Remove color set from twrnc preset Refactor to remove color set and simplify twrnc preset Jun 6, 2025
@georgewrmarshall georgewrmarshall self-assigned this Jun 6, 2025
brianacnguyen pushed a commit that referenced this issue Jun 7, 2025
## **Description**

This PR **completely refactors** the
`@metamask/design-system-twrnc-preset` package to focus on simplicity
and provide only the essential tooling needed for using `twrnc` with the
design system in React Native applications.

**What changed:**
- **Simplified theme system**: Removed complex `ColorSet`/`ColorScheme`
architecture in favor of a clean `Theme.Light`/`Theme.Dark` enum
- **Better encapsulation**: Moved `twrnc` from peer dependencies to
regular dependencies so consumers don't need to manage it
- **Removed file nesting**: Eliminated the `Theme/` folder structure and
`twrnc-settings/` complexity
- **Clean API surface**: Exports only essentials: `ThemeProvider`,
`Theme`, `useTailwind()`, and new `useTheme()` hook
- **Controlled components**: `ThemeProvider` is now a simple controlled
component instead of managing internal state
- **Removed unused files**: Removed `tailwind.config.js` and
`tailwind.config.d.ts` as they aren't used
* **Standard naming**: Renamed
`[Theme/Theme.utilities.ts](https://github.com/MetaMask/metamask-design-system/pull/722/files#diff-85c440c866d00e849e6beccc82c3cd6925caea6f0b4881edf5d1e2cf8b01b4a8)`
to `tailwind.config.ts` to follow expected conventions, as this file
generates the Tailwind config.
* **Source of Truth for linting**: Updated prettier to point to
`apps/storybook-react-native/tailwind-intellisense.config.js` to align
intellisense and linting settings

**Why this improves the developer experience:**
- **Easier to use**: Single `theme` prop instead of multiple
configuration concepts
- **More predictable**: Consumers control theme logic (system detection
moved to Storybook level)
- **Better separation**: Package handles styling, consumers handle theme
detection
- **Cleaner imports**: Only import what you need without complex nested
structures

## **Related issues**

Fixes: #719
#720

## **Manual testing steps**

1. Check updated theme provider in `storybook-react-native`
```yarn storybook:ios```
2. Navigate to the Wallet Home story which uses both the provider and useTailwind hook
3. Check `ButtonPrimary` stays in light mode in both light and dark mode
4. Check implementation of publish preview from this PR in mobile MetaMask/metamask-mobile#16103

## **Screenshots/Recordings**

### **Before**
- Complex API: `<ThemeProvider colorSet={ColorSet.Brand} theme={Theme.Default}>` (no usage of `ThemeContext`, `ThemeContextProps`, `ThemeProviderProps`, etc)
- Multiple nested folders and exports
- Consumers managed `twrnc` dependency
`useColorScheme` was handled internally in the ThemeProvider based on light, dark, default

https://github.com/user-attachments/assets/eb365979-3eec-4559-9f65-84901e8b78f4

### **After** 
- Simple API: `<ThemeProvider theme={theme}>`
- Flat file structure with essential exports only  
- Package owns `twrnc` dependency internally
- Theme state has been offloaded to storybook

https://github.com/user-attachments/assets/4cdfad8a-8ead-495e-9231-ff60eecb38a9

[Preview package](#722 (comment)) working in mobile MetaMask/metamask-mobile#16103

https://github.com/user-attachments/assets/88d443ff-7c49-44f7-9c00-042323b1a35f

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs)
- [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/develop/.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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant