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

feat(Theming): Adds theming support with documentation. #2636

Open
wants to merge 28 commits into
base: v10
Choose a base branch
from

Conversation

markrickert
Copy link
Member

Please verify the following:

  • yarn test jest tests pass with new tests, if relevant
  • yarn lint eslint checks pass with new code, if relevant
  • yarn format:check prettier checks pass with new code, if relevant
  • README.md (or relevant documentation) has been updated with your changes

Describe 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 not main.

@jamonholmgren
Copy link
Member

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.

Copy link
Member

@jamonholmgren jamonholmgren left a 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.

docs/boilerplate/app/theme/Theming.md Outdated Show resolved Hide resolved
docs/boilerplate/app/theme/Theming.md Outdated Show resolved Hide resolved
docs/boilerplate/app/theme/Theming.md Outdated Show resolved Hide resolved
@markrickert
Copy link
Member Author

@jamonholmgren Absolutely. I'll keep this branch up to date with main and use this as the starting point for my next project.

frankcalise and others added 3 commits March 28, 2024 10:06
* 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>
@markrickert markrickert added this to the Ignite v10 milestone May 13, 2024
@markrickert
Copy link
Member Author

Rebased on the v10 branch and fixed a few issues caused by the Switch/Toggle rewrite

@frankcalise
Copy link
Contributor

Copy link
Contributor

@frankcalise frankcalise left a 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:

  1. DrawerIconButton
  2. ButtonLeftAccessory in DemoPodcastListScreen.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])} />

image

Separate objects <Animated.View style={[themed($middleBar), animatedMiddleBarStyles]} />

image

ButtonLeftAccessory in DemoPodcastListScreen

All in one

<Animated.View  style={themed([$iconContainer, StyleSheet.absoluteFill, animatedLikeButtonStyles])}>
// ...
<Animated.View style={themed([$iconContainer, animatedUnlikeButtonStyles])}>

image

Separate

<Animated.View style={[themed($iconContainer), StyleSheet.absoluteFill, animatedLikeButtonStyles]}>
// ...
<Animated.View style={[themed($iconContainer), animatedUnlikeButtonStyles]}>

image
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:

  1. DrawerIconButton
  2. ButtonLeftAccessory in DemoPodcastListScreen.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])} />

image

Separate objects <Animated.View style={[themed($middleBar), animatedMiddleBarStyles]} />

image

ButtonLeftAccessory in DemoPodcastListScreen

All in one

<Animated.View  style={themed([$iconContainer, StyleSheet.absoluteFill, animatedLikeButtonStyles])}>
// ...
<Animated.View style={themed([$iconContainer, animatedUnlikeButtonStyles])}>

image

Separate

<Animated.View style={[themed($iconContainer), StyleSheet.absoluteFill, animatedLikeButtonStyles]}>
// ...
<Animated.View style={[themed($iconContainer), animatedUnlikeButtonStyles]}>

image

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.

None yet

9 participants