-
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
feat: new withFluentProvider decorator for converged Provider #16869
Conversation
@@ -2,5 +2,5 @@ import { merge } from '@fluentui/utilities'; | |||
import { PartialTheme, Theme } from '../types'; | |||
|
|||
export function mergeThemes(a: Theme, b: PartialTheme | Theme): Theme { | |||
return merge(a, b) as Theme; | |||
return merge({}, a, b) as Theme; |
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.
A small bugfix, merge()
mutates the first object 😨
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.
can this be covered in unit tests?
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.
Oops, overlooked your comment, yes it can be
return <FluentProvider theme={theme}>{props.children}</FluentProvider>; | ||
}; | ||
|
||
export const withFluentProvider = makeDecorator({ |
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.
will you add this to react-examples
in a separate PR ?
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.
It's done per package, for react-avatar
it's done in #16793.
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.
I see the author has to explicitly enable this decorator
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 a66ee06:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: b6e11cbb2ee4037c927927560e0e78b1e1f8e377 (build) |
Perf AnalysisNo significant results to display. All results
Perf Analysis (Fluent)Perf comparison
Perf tests with no regressions
|
🎉 Handy links: |
🎉 Handy links: |
🎉 Handy links: |
🎉 Handy links: |
🎉 Handy links: |
🎉 Handy links: |
Description of changes
This PR adds a new
withFluentProvider()
to be used with converged components anduseTheme()
to switch themes.