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: adjust theme structure #3084

Merged
merged 8 commits into from
Feb 10, 2022
Merged

feat: adjust theme structure #3084

merged 8 commits into from
Feb 10, 2022

Conversation

Drakeoon
Copy link
Contributor

@Drakeoon Drakeoon commented Feb 8, 2022

Summary

After internal discussion, we decided to simplify our approach and make the new theme closer to the old theme in terms of structure. This PR delivers that, plus it has all the necessary theme.isV3 checks in components

Test plan

Run the project locally:

yarn example start

@callstack-bot
Copy link

callstack-bot commented Feb 8, 2022

Hey @Drakeoon, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

example/src/Examples/ButtonExample.tsx Outdated Show resolved Hide resolved
example/src/ScreenWrapper.tsx Outdated Show resolved Hide resolved
src/components/Badge.tsx Outdated Show resolved Hide resolved
@@ -82,7 +82,10 @@ const DrawerItem = ({
: 'transparent';
const contentColor = active
? colors?.primary
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be theme.colors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In cases where properties overlap i skipped the check, since it would be something like theme.isV3 ? theme.colors.primary : theme.colors.primary. In cases where it does matter, I use the full theme.colors.key syntax

src/components/Drawer/DrawerItem.tsx Show resolved Hide resolved
src/components/Typography/Text.tsx Outdated Show resolved Hide resolved
@Drakeoon Drakeoon force-pushed the feat/adjust-theme-structure branch from 3de8a10 to 46f6e35 Compare February 10, 2022 08:52
@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@lukewalczak lukewalczak merged commit 39071e1 into next Feb 10, 2022
@lukewalczak lukewalczak deleted the feat/adjust-theme-structure branch February 10, 2022 14:45
lukewalczak pushed a commit that referenced this pull request Mar 22, 2022
lukewalczak pushed a commit that referenced this pull request Jun 8, 2022
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.

3 participants