-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(Theming): Adds theming support with documentation. #2636
base: v10
Are you sure you want to change the base?
Conversation
Before we merge this, I want to make sure we get some real-world experience. @markrickert you're starting a new project soon, so this will be a good opportunity for that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really looking good. Just want to wait for real-world experience.
@jamonholmgren Absolutely. I'll keep this branch up to date with |
4480ff2
to
b883d42
Compare
e4f096b
to
37b26b3
Compare
37b26b3
to
328528c
Compare
* feat: cng ftw * fix(babel-config): simplify plugins with SDK 50 * fix(boilerplate): remove unused eslint pkgs * fix(cli): reword workflow question * fix(cli): remove unnecessary warning
* feat(boilerplate): mmkv in over async storage * fix(cli): remove async storage new arch specifics * test(boilerplate): mmkv clean up * fix(boilerplate): storage.load with just a string * test(boilerplate): storage test types
… Switch (#2667 by @frankcalise) * refactor(toggle): split to Checkbox, Radio and Switch components * fix(boilerplate): update demos to use specific toggle components * updating deprecated reanimated Extrapolate * TS fixes * docs: explode Toggle docs to Checkbox, Radio, Switch * docs(components): added Checkbox, Radio, Switch to main listing * fix(boilerplate): remove reanimated from Checkbox * fix(boilerplate): remove reanimated from Radio * fix(boilerplate): checkbox, radio match old duration times from default reanimated values * fix(boilerplate): remove reanimated dep from Switch * docs(toggle): tsx formatting * refactor(boilerplate):drop prefix from component specific props * some missed props * fix(boilerplate): toggle components accessibilityRole --------- Co-authored-by: Mazen Chami <mazenchami@gmail.com>
Co-authored-by: Jamon Holmgren <jamonholmgren@gmail.com>
Co-authored-by: Jamon Holmgren <jamonholmgren@gmail.com>
Co-authored-by: Jamon Holmgren <jamonholmgren@gmail.com>
[skip ci]
328528c
to
06beb2c
Compare
Rebased on the |
TODO: need to check over the showroom issue: https://infiniteredcommunity.slack.com/archives/C41PK1LAY/p1716828905367989?thread_ts=1716662953.807429&cid=C41PK1LAY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really great work, it's fun to see this in action!
One thing I noticed with this implementation, is animated styles do something funny when incorporated into the entire themed(...)
function.
The easy workaround is via documentation and making a note about animated styles. Still have to dive deeper into themed
I'm guessing for a proper fix
For example, there are two instances of this that I found:
DrawerIconButton
ButtonLeftAccessory
inDemoPodcastListScreen.tsx
When the style and animated style are both passed into the themed
function, it doesn't style correctly (I'm thinking because of shared values, etc don't get properly executed since it's a non-worklet (although I'm not entirely sure that's true, since the heart does animate correctly, it's just the style is misaligned)?
If you instead create an array of styles, pass the themed
bare style object as one and also the animated style, it works.
DrawerIconButton
All in one <Animated.View style={themed([$middleBar, animatedMiddleBarStyles])} />
Separate objects <Animated.View style={[themed($middleBar), animatedMiddleBarStyles]} />
ButtonLeftAccessory in DemoPodcastListScreen
All in one
<Animated.View style={themed([$iconContainer, StyleSheet.absoluteFill, animatedLikeButtonStyles])}>
// ...
<Animated.View style={themed([$iconContainer, animatedUnlikeButtonStyles])}>
Separate
<Animated.View style={[themed($iconContainer), StyleSheet.absoluteFill, animatedLikeButtonStyles]}>
// ...
<Animated.View style={[themed($iconContainer), animatedUnlikeButtonStyles]}>
One thing I noticed with this implementation, is animated styles do something funny when incorporated into the entire themed(...)
function.
For example, there are two instances of this that I found:
DrawerIconButton
ButtonLeftAccessory
inDemoPodcastListScreen.tsx
When the style and animated style are both passed into the themed
function, it doesn't style correctly (I'm thinking because of shared values, etc don't get properly executed since it's a non-worklet?
If you instead create an array of styles, pass the themed
bare style object as one and also the animated style, it works.
DrawerIconButton
All in one <Animated.View style={themed([$middleBar, animatedMiddleBarStyles])} />
Separate objects <Animated.View style={[themed($middleBar), animatedMiddleBarStyles]} />
ButtonLeftAccessory in DemoPodcastListScreen
All in one
<Animated.View style={themed([$iconContainer, StyleSheet.absoluteFill, animatedLikeButtonStyles])}>
// ...
<Animated.View style={themed([$iconContainer, animatedUnlikeButtonStyles])}>
Separate
<Animated.View style={[themed($iconContainer), StyleSheet.absoluteFill, animatedLikeButtonStyles]}>
// ...
<Animated.View style={[themed($iconContainer), animatedUnlikeButtonStyles]}>
Please verify the following:
yarn test
jest tests pass with new tests, if relevantyarn lint
eslint checks pass with new code, if relevantyarn format:check
prettier checks pass with new code, if relevantREADME.md
(or relevant documentation) has been updated with your changesDescribe your PR
This PR adds theming support to the ignite boilerplate as well as relevant documentation on how to use the theming system.
Note that this PR is onto the
v10
branch and notmain
.