-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Customizer now uses React 16 context. #5701
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
Changes from 4 commits
8eb0239
536f08e
fa495ef
4899939
ee072d1
ba27656
e459faf
e2413c0
8ab8ada
19a60a8
799c5c8
0d7f3c8
dbbfc5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "@uifabric/fabric-website-resources", | ||
| "comment": "Updating serve config to respect oufr imports.", | ||
| "type": "patch" | ||
| } | ||
| ], | ||
| "packageName": "@uifabric/fabric-website-resources", | ||
| "email": "dzearing@microsoft.com" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "@uifabric/utilities", | ||
| "comment": "Customizer: moving to use React 16 context.", | ||
| "type": "minor" | ||
| } | ||
| ], | ||
| "packageName": "@uifabric/utilities", | ||
| "email": "dzearing@microsoft.com" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "office-ui-fabric-react", | ||
| "comment": "Check: adjusting shouldComponentUpdate to not ignore theme changes.", | ||
| "type": "patch" | ||
| } | ||
| ], | ||
| "packageName": "office-ui-fabric-react", | ||
| "email": "dzearing@microsoft.com" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,11 @@ export class CheckBase extends BaseComponent<ICheckProps, {}> { | |
| }; | ||
|
|
||
| public shouldComponentUpdate(newProps: ICheckProps): boolean { | ||
| return this.props.checked !== newProps.checked; | ||
| return ( | ||
| this.props.className !== newProps.className || | ||
| this.props.checked !== newProps.checked || | ||
| this.props.theme !== newProps.theme | ||
| ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if styles prop changes? I feel like having shouldComponentUpdate for such a simple component is code smell and we're checking most of the props now. I'm curious why it exists at all |
||
| } | ||
|
|
||
| public render(): JSX.Element { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,7 @@ | ||
| import * as React from 'react'; | ||
| import * as PropTypes from 'prop-types'; | ||
| import { Customizations } from './Customizations'; | ||
| import { hoistStatics } from './hoistStatics'; | ||
|
|
||
| export const CustomizableContextTypes = { | ||
| customizations: PropTypes.object | ||
| }; | ||
| import { CustomizerContext, ICustomizerContext } from './Customizer'; | ||
|
|
||
| export function customizable( | ||
| scope: string, | ||
|
|
@@ -21,11 +17,9 @@ export function customizable( | |
| const resultClass = class ComponentWithInjectedProps extends React.Component<P, {}> { | ||
| public static displayName: string = 'Customized' + scope; | ||
|
|
||
| public static contextTypes = CustomizableContextTypes; | ||
|
|
||
| // tslint:disable-next-line:no-any | ||
| constructor(props: P, context: any) { | ||
| super(props, context); | ||
| constructor(props: P) { | ||
| super(props); | ||
|
|
||
| this._onSettingChanged = this._onSettingChanged.bind(this); | ||
| } | ||
|
|
@@ -39,11 +33,14 @@ export function customizable( | |
| } | ||
|
|
||
| public render(): JSX.Element { | ||
| const defaultProps = Customizations.getSettings(fields, scope, this.context.customizations); | ||
|
|
||
| return ( | ||
| // tslint:disable-next-line:no-any | ||
| <ComposedComponent {...defaultProps} {...this.props as any} /> | ||
| <CustomizerContext.Consumer> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 I can steal this for createComponent 😄 |
||
| {(context: ICustomizerContext) => { | ||
| const defaultProps = Customizations.getSettings(fields, scope, context.customizations); | ||
| // tslint:disable-next-line:no-any | ||
| return <ComposedComponent {...defaultProps} {...this.props as any} />; | ||
| }} | ||
| </CustomizerContext.Consumer> | ||
| ); | ||
| } | ||
|
|
||
|
|
||
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.
?