Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "Even if a ComboBoxOptionWrapper component doesn't update (shouldComponentUpdate returns false), children functions are still being executed unnecessarily, which can cause performance issues. This can be avoided by passing in a reference to a function that returns the children elements, instead of returning the elements themselves.",
"type": "patch"
}
],
"packageName": "office-ui-fabric-react",
"email": "[email protected]"
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,24 @@ enum HoverStatus {
interface IComboBoxOptionWrapperProps extends IComboBoxOption {
// True if the option is currently selected
isSelected: boolean;

// A function that returns the children of the OptionWrapper. We pass this in as a function to ensure that
// children methods don't get called unnecessarily if the component doesn't need to be updated. This leads
// to a significant performance increase in ComboBoxes with many options and/or complex onRenderOption functions
render: () => JSX.Element;
}

// Internal class that is used to wrap all ComboBox options
// This is used to customize when we want to rerender components,
// so we don't rerender every option every time render is executed
class ComboBoxOptionWrapper extends React.Component<IComboBoxOptionWrapperProps, {}> {
public render(): React.ReactNode {
return this.props.children;
return this.props.render();
}

public shouldComponentUpdate(newProps: IComboBoxOptionWrapperProps): boolean {
// The children will always be different, so we ignore that prop
return !shallowCompare({ ...this.props, children: undefined }, { ...newProps, children: undefined });
// The render function will always be different, so we ignore that prop
return !shallowCompare({ ...this.props, render: undefined }, { ...newProps, render: undefined });
}
}

Expand Down Expand Up @@ -1053,26 +1058,14 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
const { onRenderOption = this._onRenderOptionContent } = this.props;
const id = this._id;
const isSelected: boolean = this._isOptionSelected(item.index);
const optionStyles = this._getCurrentOptionStyles(item);
const wrapperProps = {
key: item.key,
index: item.index,
styles: optionStyles,
disabled: item.disabled,
isSelected: isSelected,
text: item.text,
};

return (
!this.props.multiSelect ? (
<ComboBoxOptionWrapper
{ ...wrapperProps }
>
const getOptionComponent = () => {
return (
!this.props.multiSelect ? (
<CommandButton
id={ id + '-list' + item.index }
key={ item.key }
data-index={ item.index }
styles={ optionStyles }
styles={ this._getCurrentOptionStyles(item) }
checked={ isSelected }
className={ 'ms-ComboBox-option' }
onClick={ this._onItemClick(item.index) }
Expand All @@ -1088,18 +1081,14 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
</span>
}
</CommandButton>
</ComboBoxOptionWrapper >
) : (
<ComboBoxOptionWrapper
{ ...wrapperProps }
>
) : (
<Checkbox
id={ id + '-list' + item.index }
ref={ 'option' + item.index }
ariaLabel={ this._getPreviewText(item) }
key={ item.key }
data-index={ item.index }
styles={ optionStyles }
styles={ this._getCurrentOptionStyles(item) }
className={ 'ms-ComboBox-option' }
data-is-focusable={ true }
onChange={ this._onItemClick(item.index!) }
Expand All @@ -1110,8 +1099,19 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
>
{ onRenderOption(item, this._onRenderOptionContent) }
</Checkbox>
</ComboBoxOptionWrapper >
)
)
);
};

return (
<ComboBoxOptionWrapper
key={ item.key }
index={ item.index }
disabled={ item.disabled }
isSelected={ isSelected }
text={ item.text }
render={ getOptionComponent }
/>
);
}

Expand Down