Skip to content

Commit

Permalink
be less ambitious
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari committed Nov 25, 2020
1 parent 4a6b896 commit 4a4d3ce
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 24 deletions.
46 changes: 33 additions & 13 deletions packages/material-ui/src/Autocomplete/Autocomplete.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1283,9 +1283,9 @@ describe('<Autocomplete />', () => {

checkHighlightIs(listbox, 'two');

// three option is added and autocomplete re-renders, two should still be highlighted
// three option is added and autocomplete re-renders, reset the highlight
setProps({ options: ['one', 'two', 'three'] });
checkHighlightIs(listbox, 'two');
checkHighlightIs(listbox, null);
});

it('should not select undefined', () => {
Expand Down Expand Up @@ -2014,17 +2014,17 @@ describe('<Autocomplete />', () => {
);
const textbox = screen.getByRole('textbox');

fireEvent.keyDown(textbox, { key: 'ArrowDown' });
expect(handleHighlightChange.callCount).to.equal(2);
expect(handleHighlightChange.args[1][0]).to.not.equal(undefined);
expect(handleHighlightChange.args[1][1]).to.equal(options[0]);
expect(handleHighlightChange.args[1][2]).to.equal('keyboard');

fireEvent.keyDown(textbox, { key: 'ArrowDown' });
expect(handleHighlightChange.callCount).to.equal(3);
expect(handleHighlightChange.args[2][0]).to.not.equal(undefined);
expect(handleHighlightChange.args[2][1]).to.equal(options[1]);
expect(handleHighlightChange.args[2][1]).to.equal(options[0]);
expect(handleHighlightChange.args[2][2]).to.equal('keyboard');

fireEvent.keyDown(textbox, { key: 'ArrowDown' });
expect(handleHighlightChange.callCount).to.equal(4);
expect(handleHighlightChange.args[3][0]).to.not.equal(undefined);
expect(handleHighlightChange.args[3][1]).to.equal(options[1]);
expect(handleHighlightChange.args[3][2]).to.equal('keyboard');
});

it('should support mouse event', () => {
Expand All @@ -2040,10 +2040,10 @@ describe('<Autocomplete />', () => {
);
const firstOption = getAllByRole('option')[0];
fireEvent.mouseOver(firstOption);
expect(handleHighlightChange.callCount).to.equal(2);
expect(handleHighlightChange.args[1][0]).to.not.equal(undefined);
expect(handleHighlightChange.args[1][1]).to.equal(options[0]);
expect(handleHighlightChange.args[1][2]).to.equal('mouse');
expect(handleHighlightChange.callCount).to.equal(3);
expect(handleHighlightChange.args[2][0]).to.not.equal(undefined);
expect(handleHighlightChange.args[2][1]).to.equal(options[0]);
expect(handleHighlightChange.args[2][2]).to.equal('mouse');
});

it('should pass to onHighlightChange the correct value after filtering', () => {
Expand All @@ -2067,6 +2067,26 @@ describe('<Autocomplete />', () => {
options[2],
);
});

it('should reset the highlight when the options change', () => {
const handleHighlightChange = [];
const { getByRole, setProps } = render(
<Autocomplete
onHighlightChange={(event, option) => {
handleHighlightChange.push(option);
}}
openOnFocus
autoHighlight
options={['one', 'two', 'three']}
renderInput={(params) => <TextField {...params} autoFocus />}
/>,
);

checkHighlightIs(getByRole('listbox'), 'one');
setProps({ options: ['four', 'five'] });
checkHighlightIs(getByRole('listbox'), 'four');
expect(handleHighlightChange).to.deep.equal([null, 'one', 'four']);
});
});

it('should filter options when new input value matches option', () => {
Expand Down
15 changes: 4 additions & 11 deletions packages/material-ui/src/useAutocomplete/useAutocomplete.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable no-constant-condition */
import * as React from 'react';
import isEqual from 'lodash/isEqual';
import { setRef, useEventCallback, useControlled, unstable_useId as useId } from '../utils';

// https://stackoverflow.com/questions/990904/remove-accents-diacritics-in-a-string-in-javascript
Expand Down Expand Up @@ -129,7 +128,6 @@ export default function useAutocomplete(props) {
const [focusedTag, setFocusedTag] = React.useState(-1);
const defaultHighlighted = autoHighlight ? 0 : -1;
const highlightedIndexRef = React.useRef(defaultHighlighted);
const lastHighlightedOption = React.useRef(null);

const [value, setValueState] = useControlled({
controlled: valueProp,
Expand Down Expand Up @@ -289,10 +287,9 @@ export default function useAutocomplete(props) {
inputRef.current.setAttribute('aria-activedescendant', `${id}-option-${index}`);
}

if (onHighlightChange && !isEqual(filteredOptions[index], lastHighlightedOption.current)) {
if (onHighlightChange) {
onHighlightChange(event, index === -1 ? null : filteredOptions[index], reason);
}
lastHighlightedOption.current = filteredOptions[index];

if (!listboxRef.current) {
return;
Expand Down Expand Up @@ -428,7 +425,7 @@ export default function useAutocomplete(props) {
const valueItem = multiple ? value[0] : value;

// The popup is empty, reset
if (filteredOptions.length === 0) {
if (filteredOptions.length === 0 || valueItem == null) {
changeHighlightedIndex({ diff: 'reset' });
return;
}
Expand Down Expand Up @@ -461,11 +458,6 @@ export default function useAutocomplete(props) {
return;
}

if (highlightedIndexRef.current === -1) {
changeHighlightedIndex({ diff: 'reset' });
return;
}

// Prevent the highlighted index to leak outside the boundaries.
if (highlightedIndexRef.current >= filteredOptions.length - 1) {
setHighlightedIndex({ index: filteredOptions.length - 1 });
Expand All @@ -478,7 +470,8 @@ export default function useAutocomplete(props) {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [
// Only sync the highlighted index when the option switch between empty and not
filteredOptions,
// eslint-disable-next-line react-hooks/exhaustive-deps
filteredOptions.length,
// Don't sync the highlighted index with the value when multiple
// eslint-disable-next-line react-hooks/exhaustive-deps
multiple ? false : value,
Expand Down

0 comments on commit 4a4d3ce

Please sign in to comment.