Skip to content

Commit

Permalink
Dropdown: Correctly updating set-size when number of options updates …
Browse files Browse the repository at this point in the history
…after first render (#14415)

#### Pull request checklist

- [x] Addresses an existing issue: Fixes #14375 
- [x] Include a change request file using `$ yarn change`

#### Description of changes

If options were added after the initial render of the `Dropdown` the `aria-setsize` attribute's value was not changing because the check to update the cache was being done in `UNSAFE_componentWillReceiveProps` where `newProps.options` was always the same as `this.props.options`. This PR fixes the issue by creating a cache of the options and comparing against this cache on subsequent renders.

#### Focus areas to test

(optional)
  • Loading branch information
khmakoto authored Aug 11, 2020
1 parent 12b2fde commit 44befb4
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "prerelease",
"comment": "Dropdown: Correctly updating set-size when number of options updates after first render.",
"packageName": "@fluentui/react-next",
"email": "[email protected]",
"dependentChangeType": "patch",
"date": "2020-08-07T23:17:16.796Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "patch",
"comment": "Dropdown: Correctly updating set-size when number of options updates after first render.",
"packageName": "office-ui-fabric-react",
"email": "[email protected]",
"dependentChangeType": "patch",
"date": "2020-08-07T23:17:13.524Z"
}
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,6 @@ export class DropdownBase extends React.Component<IDropdownInternalProps, IDropd
selectedIndices: this._getSelectedIndexes(newProps.options, newProps[selectedKeyProp]),
});
}

if (
newProps.options !== this.props.options // preexisting code assumes purity of the options...
) {
this._sizePosCache.updateOptions(newProps.options);
}
}

public componentDidUpdate(prevProps: IDropdownProps, prevState: IDropdownState) {
Expand Down Expand Up @@ -249,6 +243,11 @@ export class DropdownBase extends React.Component<IDropdownInternalProps, IDropd
// eslint-disable-next-line deprecation/deprecation
const onRenderPlaceholder = props.onRenderPlaceholder || props.onRenderPlaceHolder || this._onRenderPlaceholder;

// If our cached options are out of date update our cache
if (options !== this._sizePosCache.cachedOptions) {
this._sizePosCache.updateOptions(options);
}

const selectedOptions = getAllSelectedOptions(options, selectedIndices);
const divProps = getNativeProps(props, divProperties);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { IDropdownOption, DropdownMenuItemType } from '../Dropdown.types';
* cache should provide a little bit of relief.
*/
export class DropdownSizePosCache {
private _cachedOptions: IDropdownOption[];
private _displayOnlyOptionsCache: number[];
private _size = 0;

Expand All @@ -30,6 +31,7 @@ export class DropdownSizePosCache {

this._size = size;
this._displayOnlyOptionsCache = displayOnlyOptionsCache;
this._cachedOptions = [...options];
}

/**
Expand All @@ -39,6 +41,13 @@ export class DropdownSizePosCache {
return this._size;
}

/**
* The chached options array.
*/
public get cachedOptions(): IDropdownOption[] {
return this._cachedOptions;
}

/**
* Returns the position of this option element relative to the full set of selectable option elements.
* Note: the first selectable element is position 1 in the set.
Expand Down
11 changes: 5 additions & 6 deletions packages/react-next/src/components/Dropdown/Dropdown.base.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,6 @@ export class DropdownBase extends React.Component<IDropdownInternalProps, IDropd
selectedIndices: this._getSelectedIndexes(newProps.options, newProps[selectedKeyProp]),
});
}

if (
newProps.options !== this.props.options // preexisting code assumes purity of the options...
) {
this._sizePosCache.updateOptions(newProps.options);
}
}

public componentDidUpdate(prevProps: IDropdownProps, prevState: IDropdownState) {
Expand Down Expand Up @@ -248,6 +242,11 @@ export class DropdownBase extends React.Component<IDropdownInternalProps, IDropd
// eslint-disable-next-line deprecation/deprecation
const onRenderPlaceholder = props.onRenderPlaceholder || props.onRenderPlaceHolder || this._onRenderPlaceholder;

// If our cached options are out of date update our cache
if (options !== this._sizePosCache.cachedOptions) {
this._sizePosCache.updateOptions(options);
}

const selectedOptions = getAllSelectedOptions(options, selectedIndices);
const divProps = getNativeProps(props, divProperties);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { IDropdownOption, DropdownMenuItemType } from '../Dropdown.types';
* cache should provide a little bit of relief.
*/
export class DropdownSizePosCache {
private _cachedOptions: IDropdownOption[];
private _displayOnlyOptionsCache: number[];
private _size = 0;

Expand All @@ -30,6 +31,7 @@ export class DropdownSizePosCache {

this._size = size;
this._displayOnlyOptionsCache = displayOnlyOptionsCache;
this._cachedOptions = [...options];
}

/**
Expand All @@ -39,6 +41,13 @@ export class DropdownSizePosCache {
return this._size;
}

/**
* The chached options array.
*/
public get cachedOptions(): IDropdownOption[] {
return this._cachedOptions;
}

/**
* Returns the position of this option element relative to the full set of selectable option elements.
* Note: the first selectable element is position 1 in the set.
Expand Down

0 comments on commit 44befb4

Please sign in to comment.