Skip to content

Commit

Permalink
Merge pull request #5851 from dicksonlabs/menu-handle-focus-index-change
Browse files Browse the repository at this point in the history 
[Menu] Add onFocusIndexChange property
  • Loading branch information
@oliviertassinari
oliviertassinari authored Jan 3, 2017
2 parents b7d9a37 + b627c27 commit e14bb06
Show file tree
Hide file tree
Showing 2 changed files with 171 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(event, -1, false);
};

// 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(event, index, true);
}

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(event, hotKeys)) {
event.preventDefault();
}
}
}
this.props.onKeyDown(event);
};

setFocusIndexStartsWith(keys) {
setFocusIndexStartsWith(event, keys) {
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(event, foundIndex, true);
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(event, focusIndex, false);

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(event, index, true);
}

isChildSelected(child, props) {
Expand All @@ -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,
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
129 changes: 128 additions & 1 deletion src/Menu/Menu.spec.js
View file
Original file line number Diff line number Diff line change
@@ -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('<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();

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 = (
<Menu {...props}>
<MenuItem primaryText="a00" />
<MenuItem primaryText="b11" />
<Divider />
<MenuItem primaryText="b00" />
</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(
Expand Down

0 comments on commit e14bb06

Please sign in to comment.