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

[MenuList] Allow including disabled items in keyboard navigation #19967

Merged
merged 17 commits into from
Mar 26, 2020
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
1 change: 1 addition & 0 deletions docs/pages/api-docs/autocomplete.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ You can learn more about the difference by [reading this guide](/guides/minimizi
| <span class="prop-name">disableClearable</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the input can't be cleared. |
| <span class="prop-name">disableCloseOnSelect</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the popup won't close when a value is selected. |
| <span class="prop-name">disabled</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the input will be disabled. |
| <span class="prop-name">disabledItemsFocusable</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, will allow focus on disabled items. |
| <span class="prop-name">disableListWrap</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the list box in the popup will not wrap focus. |
| <span class="prop-name">disablePortal</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Disable the portal behavior. The children stay within it's parent DOM hierarchy. |
| <span class="prop-name">filterOptions</span> | <span class="prop-type">func</span> | | A filter function that determines the options that are eligible.<br><br>**Signature:**<br>`function(options: T[], state: object) => undefined`<br>*options:* The options to render.<br>*state:* The state of the component. |
Expand Down
1 change: 1 addition & 0 deletions docs/pages/api-docs/menu-list.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ the focus is placed inside the component it is fully keyboard accessible.
| <span class="prop-name">autoFocus</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, will focus the `[role="menu"]` container and move into tab order. |
| <span class="prop-name">autoFocusItem</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, will focus the first menuitem if `variant="menu"` or selected item if `variant="selectedMenu"`. |
| <span class="prop-name">children</span> | <span class="prop-type">node</span> | | MenuList contents, normally `MenuItem`s. |
| <span class="prop-name">disabledItemsFocusable</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, will allow focus on disabled items. |
| <span class="prop-name">disableListWrap</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the menu items will not wrap focus. |
| <span class="prop-name">variant</span> | <span class="prop-type">'menu'<br>&#124;&nbsp;'selectedMenu'</span> | <span class="prop-default">'selectedMenu'</span> | The variant to use. Use `menu` to prevent selected items from impacting the initial focus and the vertical alignment relative to the anchor element. |

Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/components/autocomplete/LimitTags.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export default function LimitTags() {
<Autocomplete
multiple
limitTags={2}
id="tags-standard"
id="limit-tags"
options={top100Films}
getOptionLabel={(option) => option.title}
defaultValue={[top100Films[13], top100Films[12], top100Films[11]]}
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/components/autocomplete/LimitTags.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default function LimitTags() {
<Autocomplete
multiple
limitTags={2}
id="tags-standard"
id="limit-tags"
options={top100Films}
getOptionLabel={(option) => option.title}
defaultValue={[top100Films[13], top100Films[12], top100Films[11]]}
Expand Down
5 changes: 5 additions & 0 deletions packages/material-ui-lab/src/Autocomplete/Autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
disableClearable = false,
disableCloseOnSelect = false,
disabled = false,
disabledItemsFocusable = false,
disableListWrap = false,
disablePortal = false,
filterOptions,
Expand Down Expand Up @@ -577,6 +578,10 @@ Autocomplete.propTypes = {
* If `true`, the input will be disabled.
*/
disabled: PropTypes.bool,
/**
* If `true`, will allow focus on disabled items.
*/
disabledItemsFocusable: PropTypes.bool,
/**
* If `true`, the list box in the popup will not wrap focus.
*/
Expand Down
20 changes: 12 additions & 8 deletions packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ export interface UseAutocompleteCommonProps<T> {
* If `true`, the popup won't close when a value is selected.
*/
disableCloseOnSelect?: boolean;
/**
* If `true`, will allow focus on disabled items.
*/
disabledItemsFocusable?: boolean;
/**
* If `true`, the list box in the popup will not wrap focus.
*/
Expand Down Expand Up @@ -180,6 +184,10 @@ export type AutocompleteCloseReason = 'toggleInput' | 'escape' | 'select-option'
export type AutocompleteInputChangeReason = 'input' | 'reset' | 'clear';

export interface UseAutocompleteMultipleProps<T> extends UseAutocompleteCommonProps<T> {
/**
* The default input value. Use when the component is not controlled.
*/
defaultValue?: T[];
/**
* If `true`, `value` must be an array and the menu will support multiple selections.
*/
Expand All @@ -191,10 +199,6 @@ export interface UseAutocompleteMultipleProps<T> extends UseAutocompleteCommonPr
* You can customize the equality behavior with the `getOptionSelected` prop.
*/
value?: T[];
/**
* The default input value. Use when the component is not controlled.
*/
defaultValue?: T[];
/**
* Callback fired when the value changes.
*
Expand All @@ -211,6 +215,10 @@ export interface UseAutocompleteMultipleProps<T> extends UseAutocompleteCommonPr
}

export interface UseAutocompleteSingleProps<T> extends UseAutocompleteCommonProps<T> {
/**
* The default input value. Use when the component is not controlled.
*/
defaultValue?: T;
/**
* If `true`, `value` must be an array and the menu will support multiple selections.
*/
Expand All @@ -222,10 +230,6 @@ export interface UseAutocompleteSingleProps<T> extends UseAutocompleteCommonProp
* You can customize the equality behavior with the `getOptionSelected` prop.
*/
value?: T | null;
/**
* The default input value. Use when the component is not controlled.
*/
defaultValue?: T;
/**
* Callback fired when the value changes.
*
Expand Down
26 changes: 19 additions & 7 deletions packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export default function useAutocomplete(props) {
defaultValue = props.multiple ? [] : null,
disableClearable = false,
disableCloseOnSelect = false,
disabledItemsFocusable = false,
disableListWrap = false,
filterOptions = defaultFilterOptions,
filterSelectedOptions = false,
Expand Down Expand Up @@ -312,12 +313,12 @@ export default function useAutocomplete(props) {
const option = listboxRef.current.querySelector(`[data-option-index="${nextFocus}"]`);

// Same logic as MenuList.js
if (
option &&
(!option.hasAttribute('tabindex') ||
option.disabled ||
option.getAttribute('aria-disabled') === 'true')
) {
const nextFocusDisabled = disabledItemsFocusable
? false
: option && (option.disabled || option.getAttribute('aria-disabled') === 'true');

if ((option && !option.hasAttribute('tabindex')) || nextFocusDisabled) {
// Move to the next element.
nextFocus += direction === 'next' ? 1 : -1;
} else {
return nextFocus;
Expand Down Expand Up @@ -641,9 +642,16 @@ export default function useAutocomplete(props) {
break;
}
if (highlightedIndexRef.current !== -1 && popupOpen) {
const option = filteredOptions[highlightedIndexRef.current];
const disabled = getOptionDisabled ? getOptionDisabled(option) : false;

if (disabled) {
return;
}

// We don't want to validate the form.
event.preventDefault();
selectNewValue(event, filteredOptions[highlightedIndexRef.current], 'select-option');
selectNewValue(event, option, 'select-option');

// Move the selection to the end.
if (autoComplete) {
Expand Down Expand Up @@ -1011,6 +1019,10 @@ useAutocomplete.propTypes = {
* If `true`, the popup won't close when a value is selected.
*/
disableCloseOnSelect: PropTypes.bool,
/**
* If `true`, will allow focus on disabled items.
*/
disabledItemsFocusable: PropTypes.bool,
/**
* If `true`, the list box in the popup will not wrap focus.
*/
Expand Down
12 changes: 7 additions & 5 deletions packages/material-ui/src/ButtonBase/ButtonBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,8 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) {
},
false,
);
const handleFocus = useEventCallback((event) => {
if (disabled) {
return;
}

const handleFocus = useEventCallback((event) => {
// Fix for https://github.com/facebook/react/issues/7769
if (!buttonRef.current) {
buttonRef.current = event.currentTarget;
Expand Down Expand Up @@ -215,7 +212,12 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) {
}

// Keyboard accessibility for non interactive elements
if (event.target === event.currentTarget && isNonNativeButton() && event.key === 'Enter') {
if (
event.target === event.currentTarget &&
isNonNativeButton() &&
event.key === 'Enter' &&
!disabled
) {
event.preventDefault();
if (onClick) {
onClick(event);
Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/src/ButtonBase/ButtonBase.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ describe('<ButtonBase />', () => {
});

describe('event: focus', () => {
it('when disabled should not call onFocus', () => {
it('when disabled should be called onFocus', () => {
const onFocusSpy = spy();
const { getByRole } = render(
<ButtonBase component="div" disabled onFocus={onFocusSpy}>
Expand All @@ -621,7 +621,7 @@ describe('<ButtonBase />', () => {

getByRole('button').focus();

expect(onFocusSpy.callCount).to.equal(0);
expect(onFocusSpy.callCount).to.equal(1);
});

it('has a focus-visible polyfill', () => {
Expand Down
4 changes: 4 additions & 0 deletions packages/material-ui/src/MenuList/MenuList.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ export interface MenuListProps extends StandardProps<ListProps, MenuListClassKey
* MenuList contents, normally `MenuItem`s.
*/
children?: React.ReactNode;
/**
* If `true`, will allow focus on disabled items.
*/
disabledItemsFocusable?: boolean;
/**
* If `true`, the menu items will not wrap focus.
*/
Expand Down
46 changes: 31 additions & 15 deletions packages/material-ui/src/MenuList/MenuList.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,33 +46,43 @@ function textCriteriaMatches(nextFocus, textCriteria) {
return text.indexOf(textCriteria.keys.join('')) === 0;
}

function moveFocus(list, currentFocus, disableListWrap, traversalFunction, textCriteria) {
function moveFocus(
list,
currentFocus,
disableListWrap,
disabledItemsFocusable,
traversalFunction,
textCriteria,
) {
let wrappedOnce = false;
let nextFocus = traversalFunction(list, currentFocus, currentFocus ? disableListWrap : false);

while (nextFocus) {
// Prevent infinite loop.
if (nextFocus === list.firstChild) {
if (wrappedOnce) {
return false;
return;
}
wrappedOnce = true;
}
// Move to the next element.

// Same logic as useAutocomplete.js
const nextFocusDisabled = disabledItemsFocusable
? false
: nextFocus.disabled || nextFocus.getAttribute('aria-disabled') === 'true';

if (
!nextFocus.hasAttribute('tabindex') ||
nextFocus.disabled ||
nextFocus.getAttribute('aria-disabled') === 'true' ||
!textCriteriaMatches(nextFocus, textCriteria)
!textCriteriaMatches(nextFocus, textCriteria) ||
nextFocusDisabled
) {
// Move to the next element.
nextFocus = traversalFunction(list, nextFocus, disableListWrap);
} else {
nextFocus.focus();
return true;
return;
}
}

return false;
}

const useEnhancedEffect = typeof window === 'undefined' ? React.useEffect : React.useLayoutEffect;
Expand All @@ -92,8 +102,9 @@ const MenuList = React.forwardRef(function MenuList(props, ref) {
autoFocusItem = false,
children,
className,
onKeyDown,
disabledItemsFocusable = false,
disableListWrap = false,
onKeyDown,
variant = 'selectedMenu',
...other
} = props;
Expand Down Expand Up @@ -145,16 +156,16 @@ const MenuList = React.forwardRef(function MenuList(props, ref) {
if (key === 'ArrowDown') {
// Prevent scroll of the page
event.preventDefault();
moveFocus(list, currentFocus, disableListWrap, nextItem);
moveFocus(list, currentFocus, disableListWrap, disabledItemsFocusable, nextItem);
} else if (key === 'ArrowUp') {
event.preventDefault();
moveFocus(list, currentFocus, disableListWrap, previousItem);
moveFocus(list, currentFocus, disableListWrap, disabledItemsFocusable, previousItem);
} else if (key === 'Home') {
event.preventDefault();
moveFocus(list, null, disableListWrap, nextItem);
moveFocus(list, null, disableListWrap, disabledItemsFocusable, nextItem);
} else if (key === 'End') {
event.preventDefault();
moveFocus(list, null, disableListWrap, previousItem);
moveFocus(list, null, disableListWrap, disabledItemsFocusable, previousItem);
} else if (key.length === 1) {
const criteria = textCriteriaRef.current;
const lowerKey = key.toLowerCase();
Expand All @@ -175,7 +186,8 @@ const MenuList = React.forwardRef(function MenuList(props, ref) {
currentFocus && !criteria.repeating && textCriteriaMatches(currentFocus, criteria);
if (
criteria.previousKeyMatched &&
(keepFocusOnCurrent || moveFocus(list, currentFocus, false, nextItem, criteria))
(keepFocusOnCurrent ||
moveFocus(list, currentFocus, false, disabledItemsFocusable, nextItem, criteria))
) {
event.preventDefault();
} else {
Expand Down Expand Up @@ -280,6 +292,10 @@ MenuList.propTypes = {
* @ignore
*/
className: PropTypes.string,
/**
* If `true`, will allow focus on disabled items.
*/
disabledItemsFocusable: PropTypes.bool,
/**
* If `true`, the menu items will not wrap focus.
*/
Expand Down
Loading