Skip to content

feat(compass-components): add dark mode flag for using theme in components instead of darkreader COMPASS-5520 #2856

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

Merged
merged 16 commits into from
Mar 3, 2022

Conversation

Anemy
Copy link
Member

@Anemy Anemy commented Mar 2, 2022

COMPASS-5520
This pr adds a COMPASS_LG_DARKMODE feature flag. When enabled this makes it so we theme Compass using our own components, and the darkMode parameter with leafygreen components, instead of using darkreader to globally theme the entire page.

Most of the changes in the pr are around wrapping the leafygreen components in compass-components with a withTheme react high order components. How do y'all feel on how we're structuring these imports/exports now? Any suggestions or thoughts on improving?

It doesn't look great with it on (yet 😉 ), but this give a way for us to test our theming overtime so we can move towards not needing darkreader for themes.

dark.theme.with.env.mp4

// 2. Wrap the components that accept darkMode with Compass' theme.
const Button = withTheme(
LeafyGreenButton as React.ComponentType<typeof LeafyGreenButton>
) as typeof LeafyGreenButton;
Copy link
Member Author

@Anemy Anemy Mar 3, 2022

Choose a reason for hiding this comment

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

Working on a cleaner way to type the high order component that wraps these so we don't have to do this type casting.
We'll want to use this wrapper with some of our own custom components - checking to make sure those types work well too.

@Anemy Anemy merged commit 36025f5 into main Mar 3, 2022
@Anemy Anemy deleted the COMPASS-5520-add-leafygreen-dark-mode-flag branch March 3, 2022 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants