diff --git a/src/Menu/Menu.js b/src/Menu/Menu.js index 1479a6823ec482..c2fe1595480503 100644 --- a/src/Menu/Menu.js +++ b/src/Menu/Menu.js @@ -115,6 +115,19 @@ class Menu extends Component { onItemTouchTap: PropTypes.func, /** @ignore */ onKeyDown: PropTypes.func, + /** + * Callback function fired when the focus on a `MenuItem` is changed. + * There will be some "duplicate" changes reported if two different + * focusing event happen, for example if a `MenuItem` is focused via + * the keyboard and then it is clicked on. + * + * @param {object} event The event that triggered the focus change. + * The event can be null since the focus can be changed for non-event + * reasons such as prop changes. + * @param {number} newFocusIndex The index of the newly focused + * `MenuItem` or `-1` if focus was lost. + */ + onMenuItemFocusChange: PropTypes.func, /** * Override the inline-styles of selected menu items. */ @@ -164,8 +177,12 @@ class Menu extends Component { const filteredChildren = this.getFilteredChildren(props.children); const selectedIndex = this.getSelectedIndex(props, filteredChildren); + const newFocusIndex = props.disableAutoFocus ? -1 : selectedIndex >= 0 ? selectedIndex : 0; + if (newFocusIndex !== -1 && props.onMenuItemFocusChange) { + props.onMenuItemFocusChange(null, newFocusIndex); + } this.state = { - focusIndex: props.disableAutoFocus ? -1 : selectedIndex >= 0 ? selectedIndex : 0, + focusIndex: newFocusIndex, isKeyboardFocused: props.initiallyKeyboardFocused, keyWidth: props.desktop ? 64 : 56, }; @@ -184,8 +201,12 @@ class Menu extends Component { const filteredChildren = this.getFilteredChildren(nextProps.children); const selectedIndex = this.getSelectedIndex(nextProps, filteredChildren); + const newFocusIndex = nextProps.disableAutoFocus ? -1 : selectedIndex >= 0 ? selectedIndex : 0; + if (newFocusIndex !== this.state.focusIndex && this.props.onMenuItemFocusChange) { + this.props.onMenuItemFocusChange(null, newFocusIndex); + } this.setState({ - focusIndex: nextProps.disableAutoFocus ? -1 : selectedIndex >= 0 ? selectedIndex : 0, + focusIndex: newFocusIndex, keyWidth: nextProps.desktop ? 64 : 56, }); } @@ -207,7 +228,7 @@ class Menu extends Component { return; } - this.setFocusIndex(-1, false); + this.setFocusIndex(event, -1, false); }; // Do not use outside of this component, it will be removed once valueLink is deprecated @@ -269,13 +290,13 @@ class Menu extends Component { }); } - decrementKeyboardFocusIndex() { + decrementKeyboardFocusIndex(event) { let index = this.state.focusIndex; index--; if (index < 0) index = 0; - this.setFocusIndex(index, true); + this.setFocusIndex(event, index, true); } getMenuItemCount(filteredChildren) { @@ -308,7 +329,7 @@ class Menu extends Component { switch (key) { case 'down': event.preventDefault(); - this.incrementKeyboardFocusIndex(filteredChildren); + this.incrementKeyboardFocusIndex(event, filteredChildren); break; case 'esc': this.props.onEscKeyDown(event); @@ -316,19 +337,19 @@ class Menu extends Component { case 'tab': event.preventDefault(); if (event.shiftKey) { - this.decrementKeyboardFocusIndex(); + this.decrementKeyboardFocusIndex(event); } else { - this.incrementKeyboardFocusIndex(filteredChildren); + this.incrementKeyboardFocusIndex(event, filteredChildren); } break; case 'up': event.preventDefault(); - this.decrementKeyboardFocusIndex(); + this.decrementKeyboardFocusIndex(event); break; default: if (key && key.length === 1) { const hotKeys = this.hotKeyHolder.append(key); - if (this.setFocusIndexStartsWith(hotKeys)) { + if (this.setFocusIndexStartsWith(event, hotKeys)) { event.preventDefault(); } } @@ -336,7 +357,7 @@ class Menu extends Component { this.props.onKeyDown(event); }; - setFocusIndexStartsWith(keys) { + setFocusIndexStartsWith(event, keys) { let foundIndex = -1; React.Children.forEach(this.props.children, (child, index) => { if (foundIndex >= 0) { @@ -348,7 +369,7 @@ class Menu extends Component { } }); if (foundIndex >= 0) { - this.setFocusIndex(foundIndex, true); + this.setFocusIndex(event, foundIndex, true); return true; } return false; @@ -362,7 +383,7 @@ class Menu extends Component { const itemValue = item.props.value; const focusIndex = React.isValidElement(children) ? 0 : children.indexOf(item); - this.setFocusIndex(focusIndex, false); + this.setFocusIndex(event, focusIndex, false); if (multiple) { const itemIndex = menuValue.indexOf(itemValue); @@ -381,14 +402,14 @@ class Menu extends Component { this.props.onItemTouchTap(event, item, index); } - incrementKeyboardFocusIndex(filteredChildren) { + incrementKeyboardFocusIndex(event, filteredChildren) { let index = this.state.focusIndex; const maxIndex = this.getMenuItemCount(filteredChildren) - 1; index++; if (index > maxIndex) index = maxIndex; - this.setFocusIndex(index, true); + this.setFocusIndex(event, index, true); } isChildSelected(child, props) { @@ -402,7 +423,12 @@ class Menu extends Component { } } - setFocusIndex(newIndex, isKeyboardFocused) { + setFocusIndex(event, newIndex, isKeyboardFocused) { + if (this.props.onMenuItemFocusChange) { + // Do this even if `newIndex === this.state.focusIndex` to allow users + // to detect up-arrow on the first MenuItem or down-arrow on the last. + this.props.onMenuItemFocusChange(event, newIndex); + } this.setState({ focusIndex: newIndex, isKeyboardFocused: isKeyboardFocused, @@ -479,6 +505,7 @@ class Menu extends Component { multiple, // eslint-disable-line no-unused-vars onItemTouchTap, // eslint-disable-line no-unused-vars onEscKeyDown, // eslint-disable-line no-unused-vars + onMenuItemFocusChange, // eslint-disable-line no-unused-vars selectedMenuItemStyle, // eslint-disable-line no-unused-vars menuItemStyle, // eslint-disable-line no-unused-vars style, diff --git a/src/Menu/Menu.spec.js b/src/Menu/Menu.spec.js index 4a2b60c21ba7d9..18fe2603be5b78 100644 --- a/src/Menu/Menu.spec.js +++ b/src/Menu/Menu.spec.js @@ -1,15 +1,142 @@ /* eslint-env mocha */ import React from 'react'; -import {shallow} from 'enzyme'; +import {mount, shallow} from 'enzyme'; +import {match, spy} from 'sinon'; import {assert} from 'chai'; import Menu from './Menu'; import MenuItem from '../MenuItem'; import Divider from '../Divider'; import getMuiTheme from '../styles/getMuiTheme'; +import keycode from 'keycode'; describe('
', () => { const muiTheme = getMuiTheme(); const shallowWithContext = (node) => shallow(node, {context: {muiTheme}}); + const mountWithContext = (node) => mount(node, {context: {muiTheme}}); + const keycodeEvent = (key) => ({keyCode: keycode(key)}); + + describe('onMenuItemFocusChange', () => { + function createMenu(props) { + return ( + + ); + } + + it('is invoked when using the arrow key to go down to the bottom and back up to the top', () => { + const onMenuItemFocusChangeSpy = spy(); + const menu = createMenu({ + disableAutoFocus: false, + onMenuItemFocusChange: onMenuItemFocusChangeSpy, + }); + const wrapper = mountWithContext(menu); + + assert(onMenuItemFocusChangeSpy.calledWith(null, 0), + 'initial focus should invoke callback with 0'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.simulate('keydown', keycodeEvent('down')); + assert(onMenuItemFocusChangeSpy.calledWith(match.object, 1), + 'down-arrow invokes callback with index 1'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.simulate('keydown', keycodeEvent('down')); + assert(onMenuItemFocusChangeSpy.calledWith(match.object, 2), + 'down-arrow invokes callback with index 2'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.simulate('keydown', keycodeEvent('down')); + assert(onMenuItemFocusChangeSpy.calledWith(match.object, 2), + 'down-arrow at end invokes callback with unchanged index'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.simulate('keydown', keycodeEvent('up')); + assert(onMenuItemFocusChangeSpy.calledWith(match.object, 1), + 'up-arrow invokes callback with 1'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.simulate('keydown', keycodeEvent('up')); + assert(onMenuItemFocusChangeSpy.calledWith(match.object, 0), + 'up-arrow invokes callback with 0'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.simulate('keydown', keycodeEvent('up')); + assert(onMenuItemFocusChangeSpy.calledWith(match.object, 0), + 'up-arrow at top invokes callback with unchanged index'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.unmount(); // Otherwise the timer in FocusRipple keeps Node from exiting + }); + + it('is invoked when props change', () => { + const onMenuItemFocusChangeSpy = spy(); + const menu = createMenu({ + disableAutoFocus: true, + onMenuItemFocusChange: onMenuItemFocusChangeSpy, + }); + const wrapper = mountWithContext(menu); + assert(onMenuItemFocusChangeSpy.notCalled, + 'should not be called when creating with disableAutoFocus=true'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.setProps({disableAutoFocus: false}); + assert(onMenuItemFocusChangeSpy.calledWith(null, 0), + 'changing disableAutoFocus to false invokes callback'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.setProps({disableAutoFocus: true}); + assert(onMenuItemFocusChangeSpy.calledWith(null, -1), + 'changing disableAutoFocus to true invokes callback'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.unmount(); // Otherwise the timer in FocusRipple keeps Node from exiting + }); + + it('is invoked for hotkeys', () => { + const onMenuItemFocusChangeSpy = spy(); + const props = { + disableAutoFocus: false, + onMenuItemFocusChange: onMenuItemFocusChangeSpy, + }; + const menu = ( + + ); + const wrapper = mountWithContext(menu); + + wrapper.simulate('keydown', keycodeEvent('b')); + assert(onMenuItemFocusChangeSpy.calledWith(match.object, 1), + '"b" invokes callback with focus index 1'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.simulate('keydown', keycodeEvent('0')); + // The Divider is incorrectly counted by Menu.setFocusIndexStartsWith(). + assert(onMenuItemFocusChangeSpy.calledWith(match.object, 3), + '"b0" invokes callback with focus index 3, which is probably a bug'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.simulate('keydown', keycodeEvent('0')); + assert(onMenuItemFocusChangeSpy.calledWith(match.object, 3), + '"b0" invokes callback with focus index 3'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.simulate('keydown', keycodeEvent('!')); + // It seems like the focus index should be changed to -1 here. + assert(onMenuItemFocusChangeSpy.notCalled, + '"b00!" does not change the focus index, which is probably a bug'); + onMenuItemFocusChangeSpy.reset(); + + wrapper.unmount(); // Otherwise the timer in FocusRipple keeps Node from exiting + }); + }); it('should render MenuItem and Divider children', () => { const wrapper = shallowWithContext(