-
Notifications
You must be signed in to change notification settings - Fork 22
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
chore: added hook and removed theme provider #2150
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 5 Ignored Deployments
|
|
* overwriteTheme can be used when you want 1 particular component to have a fixed theme | ||
* IMPORTANT: this will also overwrite the lockedTheme for that component | ||
*/ | ||
overwriteTheme?: ITheme; |
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.
As far as I can see, the theme is applied at the application level. How can you set it at the component level?
/** | ||
* lockedTheme will lock the theme for the whole application. | ||
* It will overwrite the localstorage and even the switching of themes in your system will not update the theme | ||
* IMPORTANT: using the useTheme twice in a application can lead to unexpected race conditions |
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.
But you already used the useThem twice! once in the Layout
component and once in Providers
which causes the issue.
I think the better way is splitting the hook to the Provider and the hook, to avoid this kind of issues.
So the setting up the theme will be handled in the Provider (So basically all useEffects)
and useTheme will be used to read/set data from/to context.
This will be re-done using the hook coming from react-ui lib #2137 |
following pattern from docs