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

Set up twrnc theming for React Native component library #111

Merged
merged 32 commits into from
Nov 15, 2024

Conversation

brianacnguyen
Copy link
Contributor

@brianacnguyen brianacnguyen commented Nov 13, 2024

Description

This PR introduces Themed twrnc to metamask-design-system. It includes updates to @metamask/design-system-twrnc-preset to provide twrnc settings, provider, and hooks to be used to achieve the themed tailwind-like capabilities. @metamask/design-system-twrnc-preset is also updated to be consumed by @metamask/design-system-react-native and @metamask/storybook-react-native with a simple example.

Related issues

Fixes: #19

Manual testing steps

  1. Run yarn install in root
  2. Run yarn storybook:ios in root
  3. Press Cmd + Shift + A or navigate to Features > Toggle Appearance for theme switching

Screenshots/Recordings

Before

After

Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-11-13.at.16.08.29.mp4

Pre-merge author checklist

  • I've followed MetaMask Contributor Docs
  • I've completed the PR template to the best of my ability
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). 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.

@brianacnguyen brianacnguyen requested a review from a team as a code owner November 13, 2024 23:55
@brianacnguyen brianacnguyen marked this pull request as draft November 13, 2024 23:55
"web": {
"favicon": "./assets/favicon.png"
}
"userInterfaceStyle": "automatic"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For expo's theme switching capability

},
"web": {
"favicon": "./assets/favicon.png"
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove unneeded things

"expo": "~49.0.23",
"expo-status-bar": "~1.6.0",
"expo-system-ui": "~2.4.0",
"react-native-reanimated": "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.

This is needed for dark mode

return (
<View>
<View style={tw`bg-primary-default p-3`}>
<Button text="Sample Button" />
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 this here to show the theme switching colors

{
"path": "../design-system-twrnc-preset/tsconfig.build.json"
}
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mcmire suggested to add this

export const useTailwind = () => {
const { tw } = useContext(ThemeContext);
return tw;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hook to be used instead of importing tw to get Themed tw

@brianacnguyen brianacnguyen self-assigned this Nov 14, 2024
@brianacnguyen brianacnguyen marked this pull request as ready for review November 14, 2024 15:03
@brianacnguyen brianacnguyen changed the title Twrnc package - draft Set up twrnc theming for React Native component library Nov 14, 2024
"build:docs": "typedoc",
"changelog:update": "../../scripts/update-changelog.sh @metamask/design-system-twrnc-preset",
"changelog:validate": "../../scripts/validate-changelog.sh @metamask/design-system-twrnc-preset",
"publish:preview": "yarn npm publish --tag preview",
"since-latest-release": "../../scripts/since-latest-release.sh",
"test": "NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter",
"test": "NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter -passWithNoTests",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

temporary addition since tests will be added later #90

Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

Theming and Expo are working well! 🚀 I’ve left a few questions. This PR is quite large, and while I appreciate the comments, it would be helpful to provide even more. A self-review with additional context, especially on React Native specifics, would be valuable to break things down for future PRs since I'm less familiar with those areas.

I still think including the ThemeProvider within design-system-react-native could have simplified things by reducing complexity, dependencies, and overall overhead. That said, I’m on board with this approach to keep us aligned and moving forward.

  • Pulled the branch and confirmed theming in Expo and Storybook ✅

package.json Show resolved Hide resolved
.depcheckrc.yml Show resolved Hide resolved
apps/storybook-react-native/.eslintrc.js Outdated Show resolved Hide resolved
packages/design-system-react-native/jest.config.js Outdated Show resolved Hide resolved
packages/design-system-react-native/package.json Outdated Show resolved Hide resolved
Copy link

socket-security bot commented Nov 15, 2024

👍 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/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/@types/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/@types/[email protected], npm/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/@webassemblyjs/[email protected], npm/[email protected], npm/[email protected], npm/@vue/[email protected], npm/@vue/[email protected], npm/@vue/[email protected], npm/@vue/[email protected], npm/@vue/[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

@brianacnguyen
Copy link
Contributor Author

@metamaskbot publish-preview

@brianacnguyen
Copy link
Contributor Author

@SocketSecurity ignore-all

Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

LGTM

  • checked expo storybook themeing locally ✅
  • checked ignored socket security report is supply chain threats which is protected by lavamoat ✅
Screenshot 2024-11-15 at 2 27 00 PM

@brianacnguyen brianacnguyen merged commit f120ff2 into main Nov 15, 2024
26 checks passed
@brianacnguyen brianacnguyen deleted the twrnc-package branch November 15, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

twrnc Setup: Set up twrnc theming for React Native component library
2 participants