feat: [ts-starter #6] implement theming infrastructure #10
Conversation
src/pages/test.tsx
Outdated
| </option> | ||
| ))} | ||
| </select> | ||
| <Typography> |
There was a problem hiding this comment.
- That's a weird name and highly confusing given that antd it'self has a component named like that. Name it TypographySample instead.
| ...defaultTheme, | ||
| colorText: "#ffffff", | ||
| colorBgContainer: "#000000", | ||
| colorBgElevated: "#333333", |
There was a problem hiding this comment.
I'd think that all but colorWhite also need adapting here i. e. also colorTextPrimary and colorTextSecondary.
| return ( | ||
| <ThemeContext.Provider | ||
| value={{ | ||
| selectTheme, |
There was a problem hiding this comment.
- memoize value
Passing value unmemoized is a horrible thing to do. Every component that makes use of the theme object will be rerendered everytime App.tsx is rerendered because of this. value absolutely must be memoized for that not to happen! You need to know where not memoizing things will cascade down and cause a lot of unnecessary rerenders. Contexts, especially those high up the react tree are one of those candidates.
There was a problem hiding this comment.
The selectTheme function has been created using useCallback. I thought that should be enough to prevent the reference from changing.
| }; | ||
|
|
||
| useEffect(() => { | ||
| selectTheme(getStoredTheme()); |
There was a problem hiding this comment.
I needed to apply the loaded theme from the local storage and It's supposed to run just once
There was a problem hiding this comment.
I changed the code
No description provided.