Skip to content

Commit

Permalink
[Menu] Add onFocusIndexChange property
Browse files Browse the repository at this point in the history 
Add a callback property that is called whenever the MenuItem focus is changed.

Sometimes it will be called when the focus isn't changed.  For example:
* If top menu item is keyboard focused (call mui#1) and then the up arrow is pressed (call mui#2).
* If some menu item is keyboard focused (call mui#1) and then that same menu item is clicked (call mui#2).
  • Loading branch information
@gabrieldeal
gabrieldeal committed Dec 29, 2016
1 parent 4d5de14 commit 69d07ab
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 17 deletions.
59 changes: 43 additions & 16 deletions src/Menu/Menu.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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,
};
Expand All @@ -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,
});
}
Expand All @@ -207,7 +228,7 @@ class Menu extends Component {
return;
}

this.setFocusIndex(-1, false);
this.setFocusIndex(-1, false, event);
};

// Do not use outside of this component, it will be removed once valueLink is deprecated
Expand Down Expand Up @@ -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(index, true, event);
}

getMenuItemCount(filteredChildren) {
Expand Down Expand Up @@ -308,35 +329,35 @@ class Menu extends Component {
switch (key) {
case 'down':
event.preventDefault();
this.incrementKeyboardFocusIndex(filteredChildren);
this.incrementKeyboardFocusIndex(event, filteredChildren);
break;
case 'esc':
this.props.onEscKeyDown(event);
break;
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(hotKeys, event)) {
event.preventDefault();
}
}
}
this.props.onKeyDown(event);
};

setFocusIndexStartsWith(keys) {
setFocusIndexStartsWith(keys, event) {
let foundIndex = -1;
React.Children.forEach(this.props.children, (child, index) => {
if (foundIndex >= 0) {
Expand All @@ -348,7 +369,7 @@ class Menu extends Component {
}
});
if (foundIndex >= 0) {
this.setFocusIndex(foundIndex, true);
this.setFocusIndex(foundIndex, true, event);
return true;
}
return false;
Expand All @@ -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(focusIndex, false, event);

if (multiple) {
const itemIndex = menuValue.indexOf(itemValue);
Expand All @@ -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(index, true, event);
}

isChildSelected(child, props) {
Expand All @@ -402,7 +423,12 @@ class Menu extends Component {
}
}

setFocusIndex(newIndex, isKeyboardFocused) {
setFocusIndex(newIndex, isKeyboardFocused, event) {
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,
Expand Down Expand Up @@ -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,
Expand Down
84 changes: 83 additions & 1 deletion src/Menu/Menu.spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,97 @@
/* 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('<Menu />', () => {
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 (
<Menu {...props}>
<MenuItem primaryText="item 1" />
<Divider />
<MenuItem primaryText="item 2" />
<MenuItem primaryText="item 3" />
</Menu>
);
}

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();
});

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();
});
});

it('should render MenuItem and Divider children', () => {
const wrapper = shallowWithContext(
Expand Down

0 comments on commit 69d07ab

Please sign in to comment.