-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
ThemeProvider: add instructions to replace Fabric, Customizer, loadTheme in README #15194
ThemeProvider: add instructions to replace Fabric, Customizer, loadTheme in README #15194
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ee36178:
|
Perf AnalysisNo significant results to display. All results
Perf Analysis (Fluent)Perf comparison
Perf tests with no regressions
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 9c8c7aebfb1f94da29e886b5886d647407ea3aef (build) |
1e2e9bf
to
c0a8753
Compare
### loadTheme | ||
|
||
`loadTheme` remains to work as is. However, you are recommended to replace `loadTheme` with `ThemeProvider`. That way, your application consistently has one way of providing theme. | ||
|
||
To do that, instead of calling `loadTheme(your_theme)`, you will simply wrap the root component of your React application once with `ThemeProvider`: | ||
|
||
``` | ||
<ThemeProvider theme={your_theme}> | ||
<App /> | ||
</ThemeProvider> | ||
``` | ||
|
||
One caveat here is that if you app has styles which relies on `@microsoft/load-themed-styles`, `ThemeProvider` won't be able to replace `loadTheme` in this case. |
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.
loadTheme
does not exist in this package; this guidance probably should just live in release-notes, possibly the styling
readme where it currently lives.
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.
agree this doesn't seem to be the most ideal place. These are mainly purposed as our release notes.
I am going to do more wiki/doc cleanup next week (tracked in #14740). I am going to start here and re-evaluate as I go.
c0a8753
to
ee36178
Compare
🎉 Handy links: |
🎉 Handy links: |
Pull request checklist
$ yarn change
Description of changes
Add instructions for how to replace
Fabric
,Customizer
, andloadTheme
withThemeProvider
.Related issue: #14740
Focus areas to test
(optional)