Skip to content
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

[Autocomplete] can't change option highlight color #23297

Closed
wants to merge 9 commits into from
3 changes: 3 additions & 0 deletions docs/pages/api-docs/autocomplete.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ Any other props supplied will be provided to the root element (native element).
| <span class="prop-name">loading</span> | <span class="prop-name">.MuiAutocomplete-loading</span> | Styles applied to the loading wrapper.
| <span class="prop-name">noOptions</span> | <span class="prop-name">.MuiAutocomplete-noOptions</span> | Styles applied to the no option wrapper.
| <span class="prop-name">option</span> | <span class="prop-name">.MuiAutocomplete-option</span> | Styles applied to the option elements.
| <span class="prop-name">optionFocused</span> | <span class="prop-name">.MuiAutocomplete-optionFocused</span> | Styles applied to the option if keyboard or mouse focused.
| <span class="prop-name">optionSelected</span> | <span class="prop-name">.MuiAutocomplete-optionSelected</span> | Styles applied to the option if option is selected.
| <span class="prop-name">optionDisabled</span> | <span class="prop-name">.MuiAutocomplete-optionDisabled</span> | Styles applied to the option if option is disabled.
| <span class="prop-name">groupLabel</span> | <span class="prop-name">.MuiAutocomplete-groupLabel</span> | Styles applied to the group's label elements.
| <span class="prop-name">groupUl</span> | <span class="prop-name">.MuiAutocomplete-groupUl</span> | Styles applied to the group's ul elements.

Expand Down
6 changes: 6 additions & 0 deletions packages/material-ui/src/Autocomplete/Autocomplete.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ export interface AutocompleteProps<
noOptions?: string;
/** Styles applied to the option elements. */
option?: string;
/** Styles applied to the option if keyboard or mouse focused. */
optionFocused?: string;
/** Styles applied to the option if option is selected. */
optionSelected?: string;
/** Styles applied to the option if option is disabled. */
optionDisabled?: string;
/** Styles applied to the group's label elements. */
groupLabel?: string;
/** Styles applied to the group's ul elements. */
Expand Down
85 changes: 56 additions & 29 deletions packages/material-ui/src/Autocomplete/Autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { withStyles } from '../styles';
import { alpha } from '../styles/colorManipulator';
import Popper from '../Popper';
import ListSubheader from '../ListSubheader';
import Paper from '../Paper';
Expand Down Expand Up @@ -205,20 +206,28 @@ export const styles = (theme) => ({
[theme.breakpoints.up('sm')]: {
minHeight: 'auto',
},
'&[aria-selected="true"]': {
backgroundColor: theme.palette.action.selected,
},
'&[data-focus="true"]': {
backgroundColor: theme.palette.action.hover,
},
'&:active': {
backgroundColor: theme.palette.action.selected,
},
'&[aria-disabled="true"]': {
opacity: theme.palette.action.disabledOpacity,
pointerEvents: 'none',
},
/* Styles applied to the option if keyboard or mouse focused. */
optionFocused: {
backgroundColor: theme.palette.action.hover,
},

/* Styles applied to the option if option is selected. */
optionSelected: {
backgroundColor: theme.palette.action.selected,
'&$focused': {
backgroundColor: alpha(
theme.palette.action.selected,
theme.palette.action.selectedOpacity + theme.palette.action.hoverOpacity,
),
},
},

/* Styles applied to the option if option is disabled. */
optionDisabled: {
opacity: theme.palette.action.disabledOpacity,
pointerEvents: 'none',
},
/* Styles applied to the group's label elements. */
groupLabel: {
backgroundColor: theme.palette.background.paper,
Expand Down Expand Up @@ -327,6 +336,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
setAnchorEl,
inputValue,
groupedOptions,
focusedIndex,
} = useAutocomplete({ ...props, componentName: 'Autocomplete' });

let startAdornment;
Expand Down Expand Up @@ -381,13 +391,41 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {

const renderListOption = (option, index) => {
const optionProps = getOptionProps({ option, index });

return renderOption({ ...optionProps, className: classes.option }, option, {
selected: optionProps['aria-selected'],
inputValue,
});
return renderOption(
{
...optionProps,
className: clsx(classes.option, {
[classes.optionFocused]: focusedIndex === optionProps.key,
[classes.optionSelected]: optionProps['aria-selected'],
[classes.optionDisabled]: optionProps['aria-disabled'],
}),
},
option,
{
selected: optionProps['aria-selected'],
inputValue,
},
);
};

const renderGroupedOptions = React.useMemo(
() => (groupedOptions) => {
return groupedOptions.map((option, index) => {
if (groupBy) {
return renderGroup({
key: option.key,
group: option.group,
children: option.options.map((option2, index2) =>
renderListOption(option2, option.index + index2),
),
});
}
return renderListOption(option, index);
});
},
[groupedOptions],
);

const hasClearIcon = !disableClearable && !disabled && dirty;
const hasPopupIcon = (!freeSolo || forcePopupIcon === true) && forcePopupIcon !== false;

Expand Down Expand Up @@ -480,18 +518,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
{...getListboxProps()}
{...ListboxProps}
>
{groupedOptions.map((option, index) => {
if (groupBy) {
return renderGroup({
key: option.key,
group: option.group,
children: option.options.map((option2, index2) =>
renderListOption(option2, option.index + index2),
),
});
}
return renderListOption(option, index);
})}
{renderGroupedOptions(groupedOptions)}
</ListboxComponent>
) : null}
</PaperComponent>
Expand Down
44 changes: 22 additions & 22 deletions packages/material-ui/src/Autocomplete/Autocomplete.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ describe('<Autocomplete />', () => {

it('should trigger a form expectedly', () => {
const handleSubmit = spy();

const { setProps } = render(
<Autocomplete
options={['one', 'two']}
Expand All @@ -544,35 +545,34 @@ describe('<Autocomplete />', () => {
renderInput={(props2) => <TextField {...props2} autoFocus />}
/>,
);
let textbox = screen.getByRole('textbox');

fireEvent.keyDown(textbox, { key: 'Enter' });
expect(handleSubmit.callCount).to.equal(1);
let textbox = screen.getByRole('textbox');

fireEvent.change(textbox, { target: { value: 'o' } });
fireEvent.keyDown(textbox, { key: 'ArrowDown' });
fireEvent.keyDown(textbox, { key: 'Enter' });
expect(handleSubmit.callCount).to.equal(1);

fireEvent.keyDown(textbox, { key: 'Enter' });
expect(handleSubmit.callCount).to.equal(2);

setProps({ key: 'test-2', multiple: true, freeSolo: true });
textbox = screen.getByRole('textbox');
expect(() => {
setProps({ key: 'test-2', multiple: true, freeSolo: true });
textbox = screen.getByRole('textbox');

fireEvent.change(textbox, { target: { value: 'o' } });
fireEvent.keyDown(textbox, { key: 'Enter' });
expect(handleSubmit.callCount).to.equal(2);

fireEvent.keyDown(textbox, { key: 'Enter' });
expect(handleSubmit.callCount).to.equal(3);
fireEvent.change(textbox, { target: { value: 'o' } });
fireEvent.keyDown(textbox, { key: 'Enter' });
expect(handleSubmit.callCount).to.equal(2);

setProps({ key: 'test-3', freeSolo: true });
textbox = screen.getByRole('textbox');
fireEvent.keyDown(textbox, { key: 'Enter' });
expect(handleSubmit.callCount).to.equal(3);
}).not.toErrorDev();

fireEvent.change(textbox, { target: { value: 'o' } });
fireEvent.keyDown(textbox, { key: 'Enter' });
expect(handleSubmit.callCount).to.equal(4);
expect(() => {
setProps({ key: 'test-3', freeSolo: true });
textbox = screen.getByRole('textbox');
fireEvent.change(textbox, { target: { value: 'o' } });
fireEvent.keyDown(textbox, { key: 'Enter' });
expect(handleSubmit.callCount).to.equal(4);
}).not.toErrorDev();
});

describe('prop: getOptionDisabled', () => {
Expand Down Expand Up @@ -1252,6 +1252,8 @@ describe('<Autocomplete />', () => {
/>,
);
}).toWarnDev([
`Material-UI: The options provided combined with the \`groupBy\` method of Autocomplete returns duplicated headers.`,
'You can solve the issue by sorting the options with the output of `groupBy`.',
// strict mode renders twice
'returns duplicated headers',
'returns duplicated headers',
Expand Down Expand Up @@ -1402,17 +1404,15 @@ describe('<Autocomplete />', () => {
const textbox = screen.getByRole('textbox');

fireEvent.change(document.activeElement, { target: { value: 'O' } });

expect(document.activeElement.value).to.equal('O');

fireEvent.keyDown(textbox, { key: 'ArrowDown' });

fireEvent.change(document.activeElement, { target: { value: 'one' } });
expect(document.activeElement.value).to.equal('one');
expect(document.activeElement.selectionStart).to.equal(1);
expect(document.activeElement.selectionStart).to.equal(3);
expect(document.activeElement.selectionEnd).to.equal(3);

fireEvent.keyDown(textbox, { key: 'Enter' });

expect(document.activeElement.value).to.equal('one');
expect(document.activeElement.selectionStart).to.equal(3);
expect(document.activeElement.selectionEnd).to.equal(3);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,5 +317,6 @@ export default function useAutocomplete<
anchorEl: null | HTMLElement;
setAnchorEl: () => void;
focusedTag: number;
focusedIndex: number;
groupedOptions: T[];
};
4 changes: 4 additions & 0 deletions packages/material-ui/src/useAutocomplete/useAutocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export default function useAutocomplete(props) {
const [focusedTag, setFocusedTag] = React.useState(-1);
const defaultHighlighted = autoHighlight ? 0 : -1;
const highlightedIndexRef = React.useRef(defaultHighlighted);
const [focusedIndex, setFocusedIndex] = React.useState(defaultHighlighted);
Copy link
Member

@oliviertassinari oliviertassinari Oct 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's too slow to rerender for this state. Try https://deploy-preview-23297--material-ui.netlify.app/components/autocomplete/#virtualization

Suggested change
const [focusedIndex, setFocusedIndex] = React.useState(defaultHighlighted);

Copy link
Contributor Author

@sujinleeme sujinleeme Oct 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, virtualization is one case, we also need to consider when people use the render option API with "heavy" components, like the MenuItem.


const [value, setValueState] = useControlled({
controlled: valueProp,
Expand Down Expand Up @@ -280,6 +281,7 @@ export default function useAutocomplete(props) {

const setHighlightedIndex = useEventCallback(({ event, index, reason = 'auto' }) => {
highlightedIndexRef.current = index;
setFocusedIndex(index);

// does the index exist?
if (index === -1) {
Expand All @@ -297,6 +299,7 @@ export default function useAutocomplete(props) {
}

const prev = listboxRef.current.querySelector('[data-focus]');

if (prev) {
prev.removeAttribute('data-focus');
}
Expand Down Expand Up @@ -1023,6 +1026,7 @@ export default function useAutocomplete(props) {
anchorEl,
setAnchorEl,
focusedTag,
focusedIndex,
groupedOptions,
};
}