diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.js b/packages/material-ui/src/ButtonBase/ButtonBase.js index 4dbaaa4125e79f..e78d5266065458 100644 --- a/packages/material-ui/src/ButtonBase/ButtonBase.js +++ b/packages/material-ui/src/ButtonBase/ButtonBase.js @@ -291,6 +291,7 @@ class ButtonBase extends React.Component { buttonProps['aria-disabled'] = disabled; } + // tabIndex < -1 for disabled button indicates to MenuList to skip it in keyboard focus navigation return ( diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.test.js b/packages/material-ui/src/ButtonBase/ButtonBase.test.js index cbda9afd77c743..a5ac18fb6dd9c7 100644 --- a/packages/material-ui/src/ButtonBase/ButtonBase.test.js +++ b/packages/material-ui/src/ButtonBase/ButtonBase.test.js @@ -381,7 +381,7 @@ describe('', () => { describe('prop: disabled', () => { it('should not receive the focus', () => { const wrapper = mount(Hello); - assert.strictEqual(wrapper.find('button').props().tabIndex, '-1'); + assert.strictEqual(wrapper.find('button').props().tabIndex, '-2'); }); it('should also apply it when using component', () => { diff --git a/packages/material-ui/src/Divider/Divider.js b/packages/material-ui/src/Divider/Divider.js index 6ea804f6ca2e18..ab05b5d209c0f9 100644 --- a/packages/material-ui/src/Divider/Divider.js +++ b/packages/material-ui/src/Divider/Divider.js @@ -36,11 +36,23 @@ export const styles = theme => ({ }); const Divider = React.forwardRef(function Divider(props, ref) { - const { absolute, classes, className, component: Component, light, variant, ...other } = props; + const { + absolute, + classes, + className, + component: Component, + light, + tabIndex: tabIndexProp, + variant, + ...other + } = props; if (Component === 'li' && !other.role) { other.role = 'separator'; } + // tabIndex defaults to -1 in the DOM. Using something lower to indicate to + // MenuList to skip it in keyboard focus navigation. + const tabIndex = tabIndexProp !== undefined ? tabIndexProp : -2; return ( ); @@ -83,6 +96,10 @@ Divider.propTypes = { * If `true`, the divider will have a lighter color. */ light: PropTypes.bool, + /** + * @ignore + */ + tabIndex: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), /** * The variant to use. */ diff --git a/packages/material-ui/src/MenuItem/MenuItem.js b/packages/material-ui/src/MenuItem/MenuItem.js index a4880a3bb85f2b..20019a12ac411a 100644 --- a/packages/material-ui/src/MenuItem/MenuItem.js +++ b/packages/material-ui/src/MenuItem/MenuItem.js @@ -41,7 +41,9 @@ const MenuItem = React.forwardRef(function MenuItem(props, ref) { } = props; let tabIndex; - if (!disabled) { + if (disabled) { + tabIndex = -2; + } else { tabIndex = tabIndexProp !== undefined ? tabIndexProp : -1; } return ( diff --git a/packages/material-ui/src/MenuList/MenuList.js b/packages/material-ui/src/MenuList/MenuList.js index defe3b1be26e85..0284eb821c9dcb 100644 --- a/packages/material-ui/src/MenuList/MenuList.js +++ b/packages/material-ui/src/MenuList/MenuList.js @@ -8,6 +8,48 @@ import List from '../List'; import getScrollbarSize from '../utils/getScrollbarSize'; import { useForkRef } from '../utils/reactHelpers'; +function nextItem(list, item, disableListWrap) { + if (item && item.nextElementSibling) { + return item.nextElementSibling; + } + return disableListWrap ? null : list.firstChild; +} +function previousItem(list, item, disableListWrap) { + if (item && item.previousElementSibling) { + return item.previousElementSibling; + } + return disableListWrap ? null : list.lastChild; +} + +function moveFocus(list, currentFocus, disableListWrap, traversalFunction) { + let nextFocus = traversalFunction(list, currentFocus, currentFocus ? disableListWrap : false); + while (nextFocus) { + if (nextFocus === currentFocus) { + return; + } + if ( + nextFocus.tabIndex === undefined || + nextFocus.tabIndex === null || + nextFocus.tabIndex < -1 + ) { + nextFocus = traversalFunction(list, nextFocus, disableListWrap); + } else { + break; + } + } + if (nextFocus) { + nextFocus.focus(); + } +} + +function focusNextItem(list, currentFocus, disableListWrap) { + moveFocus(list, currentFocus, disableListWrap, nextItem); +} + +function focusPreviousItem(list, currentFocus, disableListWrap) { + moveFocus(list, currentFocus, disableListWrap, previousItem); +} + const MenuList = React.forwardRef(function MenuList(props, ref) { const { actions, autoFocus, children, className, onKeyDown, disableListWrap, ...other } = props; const listRef = React.useRef(); @@ -47,27 +89,19 @@ const MenuList = React.forwardRef(function MenuList(props, ref) { (key === 'ArrowUp' || key === 'ArrowDown') && (!currentFocus || (currentFocus && !list.contains(currentFocus))) ) { - list.firstChild.focus(); + focusNextItem(list, null, disableListWrap); } else if (key === 'ArrowDown') { event.preventDefault(); - if (currentFocus.nextElementSibling) { - currentFocus.nextElementSibling.focus(); - } else if (!disableListWrap) { - list.firstChild.focus(); - } + focusNextItem(list, currentFocus, disableListWrap); } else if (key === 'ArrowUp') { event.preventDefault(); - if (currentFocus.previousElementSibling) { - currentFocus.previousElementSibling.focus(); - } else if (!disableListWrap) { - list.lastChild.focus(); - } + focusPreviousItem(list, currentFocus, disableListWrap); } else if (key === 'Home') { event.preventDefault(); - list.firstChild.focus(); + focusNextItem(list, null, disableListWrap); } else if (key === 'End') { event.preventDefault(); - list.lastChild.focus(); + focusPreviousItem(list, null, disableListWrap); } if (onKeyDown) { diff --git a/packages/material-ui/test/integration/Menu.test.js b/packages/material-ui/test/integration/Menu.test.js index 003b406d75ce44..f9db511eddaa2e 100644 --- a/packages/material-ui/test/integration/Menu.test.js +++ b/packages/material-ui/test/integration/Menu.test.js @@ -176,6 +176,8 @@ describe(' integration', () => { if (instance && !instance.stubbed) { if (instance.tabIndex === 0) { tabIndexTracker[itemIndex] = true; + } else if (instance.tabIndex > 0) { + tabIndexTracker[itemIndex] = instance.tabIndex; } const offsetTop = instance.offsetTop; stub(instance, 'offsetTop').get(() => { @@ -205,6 +207,7 @@ describe(' integration', () => { variant, { selectedIndex, + selectedTabIndex, invalidIndex, autoFocusIndex, disabledIndex, @@ -246,6 +249,7 @@ describe(' integration', () => { disabled={itemIndex === disabledIndex ? true : undefined} itemIndex={itemIndex} selected={itemIndex === selectedIndex} + tabIndex={itemIndex === selectedIndex ? selectedTabIndex : undefined} autoFocus={itemIndex === autoFocusIndex ? true : undefined} > Menu Item {itemIndex} @@ -272,14 +276,14 @@ describe(' integration', () => { }); }); - it('[variant=menu] nothing selected, content anchor: first, focus: MenuList, tabIndex: first', () => { + it('[variant=menu] nothing selected', () => { assert.deepStrictEqual(contentAnchorTracker, [true, false, false]); assert.deepStrictEqual(focusTracker, [false, false, false]); assert.strictEqual(menuListFocusTracker, true); assert.deepStrictEqual(tabIndexTracker, [false, false, false]); }); - it('[variant=menu] nothing selected, autoFocus on third, content anchor: first, focus: third, tabIndex: third', () => { + it('[variant=menu] nothing selected, autoFocus on third', () => { mountTrackingMenu('menu', { autoFocusIndex: 2 }); assert.deepStrictEqual(contentAnchorTracker, [true, false, false]); assert.deepStrictEqual(focusTracker, [false, false, true]); @@ -287,7 +291,7 @@ describe(' integration', () => { assert.deepStrictEqual(tabIndexTracker, [false, false, false]); }); - it('[variant=selectedMenu] nothing selected, content anchor: first, focus: MenuList, tabIndex: first', () => { + it('[variant=selectedMenu] nothing selected', () => { mountTrackingMenu('selectedMenu'); assert.deepStrictEqual(contentAnchorTracker, [true, false, false]); assert.deepStrictEqual(focusTracker, [false, false, false]); @@ -295,7 +299,7 @@ describe(' integration', () => { assert.deepStrictEqual(tabIndexTracker, [false, false, false]); }); - it('[variant=selectedMenu] nothing selected, content anchor: first valid, focus: first valid, tabIndex: first valid', () => { + it('[variant=selectedMenu] nothing selected, first index invalid', () => { mountTrackingMenu('selectedMenu', { invalidIndex: 0 }); assert.deepStrictEqual(contentAnchorTracker, [false, true, false]); assert.deepStrictEqual(focusTracker, [false, false, false]); @@ -303,7 +307,7 @@ describe(' integration', () => { assert.deepStrictEqual(tabIndexTracker, [false, false, false]); }); - it('[variant=menu] second item selected, content anchor: first, focus: MenuList, tabIndex: first', () => { + it('[variant=menu] second item selected', () => { mountTrackingMenu('menu', { selectedIndex: 1 }); assert.deepStrictEqual(contentAnchorTracker, [true, false, false]); assert.deepStrictEqual(focusTracker, [false, false, false]); @@ -311,7 +315,15 @@ describe(' integration', () => { assert.deepStrictEqual(tabIndexTracker, [false, false, false]); }); - it('[variant=selectedMenu] second item selected, content anchor: second, focus: second, tabIndex: second', () => { + it('[variant=selectedMenu] second item selected, explicit tabIndex', () => { + mountTrackingMenu('selectedMenu', { selectedIndex: 1, selectedTabIndex: 2 }); + assert.deepStrictEqual(contentAnchorTracker, [false, true, false]); + assert.deepStrictEqual(focusTracker, [false, true, false]); + assert.strictEqual(menuListFocusTracker, true); + assert.deepStrictEqual(tabIndexTracker, [false, 2, false]); + }); + + it('[variant=selectedMenu] second item selected', () => { mountTrackingMenu('selectedMenu', { selectedIndex: 1 }); assert.deepStrictEqual(contentAnchorTracker, [false, true, false]); assert.deepStrictEqual(focusTracker, [false, true, false]); @@ -319,7 +331,7 @@ describe(' integration', () => { assert.deepStrictEqual(tabIndexTracker, [false, true, false]); }); - it('[variant=selectedMenu] second item selected and disabled, content anchor: second, focus: second, tabIndex: second', () => { + it('[variant=selectedMenu] second item selected and disabled', () => { mountTrackingMenu('selectedMenu', { selectedIndex: 1, disabledIndex: 1 }); assert.deepStrictEqual(contentAnchorTracker, [true, false, false]); assert.deepStrictEqual(focusTracker, [false, false, false]); @@ -327,7 +339,7 @@ describe(' integration', () => { assert.deepStrictEqual(tabIndexTracker, [false, false, false]); }); - it('[variant=selectedMenu] second item selected, no autoFocus, content anchor: second, focus: none, tabIndex: second', () => { + it('[variant=selectedMenu] second item selected, no autoFocus', () => { mountTrackingMenu('selectedMenu', { selectedIndex: 1, autoFocus: false }); assert.deepStrictEqual(contentAnchorTracker, [false, true, false]); assert.deepStrictEqual(focusTracker, [false, false, false]); diff --git a/packages/material-ui/test/integration/MenuList.test.js b/packages/material-ui/test/integration/MenuList.test.js index 28f304001dfbcb..29ad31f2654ecb 100644 --- a/packages/material-ui/test/integration/MenuList.test.js +++ b/packages/material-ui/test/integration/MenuList.test.js @@ -3,6 +3,7 @@ import { assert } from 'chai'; import { spy } from 'sinon'; import MenuList from 'packages/material-ui/src/MenuList'; import MenuItem from 'packages/material-ui/src/MenuItem'; +import Divider from 'packages/material-ui/src/Divider'; import { createMount } from 'packages/material-ui/src/test-utils'; import PropTypes from 'prop-types'; @@ -57,6 +58,8 @@ function assertMenuItemFocused(wrapper, focusedIndex) { items.forEach((item, index) => { if (index === focusedIndex) { assert.strictEqual(item.find('li').instance(), document.activeElement); + } else { + assert.notStrictEqual(item.find('li').instance(), document.activeElement); } }); } @@ -296,4 +299,119 @@ describe(' integration', () => { assertMenuItemFocused(wrapper, 3); }); }); + + describe('MenuList with divider and disabled item', () => { + let wrapper; + + const resetWrapper = () => { + wrapper = mount( + + Menu Item 1 + + Menu Item 2 + Menu Item 3 + Menu Item 4 + , + ); + }; + + before(resetWrapper); + + it('should skip divider and disabled menu item', () => { + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 0); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 1); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 3); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 0); + + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 3); + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 1); + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 0); + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 3); + }); + }); + + describe('MenuList with focusable divider', () => { + let wrapper; + + const resetWrapper = () => { + wrapper = mount( + + Menu Item 1 + + Menu Item 2 + Menu Item 3 + Menu Item 4 + , + ); + }; + + before(resetWrapper); + + it('should include divider with tabIndex specified', () => { + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 0); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + // Focus is on divider instead of a menu item + assertMenuItemFocused(wrapper, -1); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 1); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 2); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 3); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 0); + + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 3); + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 2); + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 1); + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + // Focus is on divider instead of a menu item + assertMenuItemFocused(wrapper, -1); + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 0); + }); + }); + + describe('MenuList with only one focusable menu item', () => { + let wrapper; + + const resetWrapper = () => { + wrapper = mount( + + Menu Item 1 + Menu Item 2 + Menu Item 3 + Menu Item 4 + , + ); + }; + + before(resetWrapper); + + it('should go to only focusable item', () => { + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 1); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 1); + wrapper.simulate('keyDown', { key: 'ArrowDown' }); + assertMenuItemFocused(wrapper, 1); + + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 1); + wrapper.simulate('keyDown', { key: 'ArrowUp' }); + assertMenuItemFocused(wrapper, 1); + }); + }); });