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.

Sign up for GitHub

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] Add home and end key support #14212

Merged
merged 7 commits into from
Jan 18, 2019
Merged

[MenuList] Add home and end key support #14212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6c17fb4
add home and end keys to be handled in meathod handlekeydown
Jan 16, 2019
72b9c30
Merge branch 'master' into menulist-home-end
Jan 16, 2019
dca19d2
added support for home and end keys in MenuList
Jan 17, 2019
0c21738
added two tests for MenuList home and end key functionality
Jan 17, 2019
cfe0587
increased size limit of @material-ui/core
Jan 17, 2019
f153908
Remove unrelated change
oliviertassinari Jan 17, 2019
24ccc8f
[ListItem] Improve focus style
oliviertassinari Jan 17, 2019
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
2 changes: 1 addition & 1 deletion .size-limit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ module.exports = [
name: 'The size of the @material-ui/core modules',
webpack: true,
path: 'packages/material-ui/build/index.js',
limit: '95.2 KB',
limit: '95.3 KB',
},
{
name: 'The size of the @material-ui/styles modules',
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/DialogContentText/DialogContentText.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import Typography from '../Typography';
export const styles = {
/* Styles applied to the root element. */
root: {
// Should use variant="body1" in v4.0.0
// Should use variant="body1" in v4
lineHeight: 1.5,
},
};
Expand Down
15 changes: 8 additions & 7 deletions packages/material-ui/src/ListItem/ListItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,19 @@ export const styles = theme => ({
width: '100%',
boxSizing: 'border-box',
textAlign: 'left',
paddingTop: 11, // To use 10px in v4.0.0
paddingBottom: 11, // To use 10px in v4.0.0
'&$selected, &$selected:hover': {
paddingTop: 11, // To use 10px in v4
paddingBottom: 11, // To use 10px in v4
'&$selected, &$selected:hover, &$selected:focus': {
backgroundColor: theme.palette.action.selected,
},
},
/* Styles applied to the `container` element if `children` includes `ListItemSecondaryAction`. */
container: {
position: 'relative',
},
// TODO: Sanity check this - why is focusVisibleClassName prop apparently applied to a div?
// To remove in v4
/* Styles applied to the `component`'s `focusVisibleClassName` property if `button={true}`. */
focusVisible: {
backgroundColor: theme.palette.action.hover,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This style is almost 3 years old: 9a9ea7e.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. The changes look great! And all tests passing 👍

},
focusVisible: {},
/* Legacy styles applied to the root element. Use `root` instead. */
default: {},
/* Styles applied to the `component` element if `dense={true}` or `children` includes `Avatar`. */
Expand Down Expand Up @@ -71,6 +69,9 @@ export const styles = theme => ({
backgroundColor: 'transparent',
},
},
'&:focus': {
backgroundColor: theme.palette.action.hover,
},
},
/* Styles applied to the `component` element if `children` includes `ListItemSecondaryAction`. */
secondaryAction: {
Expand Down
6 changes: 6 additions & 0 deletions packages/material-ui/src/MenuList/MenuList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ class MenuList extends React.Component {
} else if (!this.props.disableListWrap) {
list.lastChild.focus();
}
} else if (key === 'home') {
event.preventDefault();
list.firstChild.focus();
} else if (key === 'end') {
event.preventDefault();
list.lastChild.focus();
}

if (this.props.onKeyDown) {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Select/Select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import withFormControlContext from '../FormControl/withFormControlContext';
import withStyles from '../styles/withStyles';
import mergeClasses from '../styles/mergeClasses';
import ArrowDropDownIcon from '../internal/svg-icons/ArrowDropDown';
// To replace with InputBase in v4.0.0
// To replace with InputBase in v4
import Input from '../Input';
import { styles as nativeSelectStyles } from '../NativeSelect/NativeSelect';
import NativeSelectInput from '../NativeSelect/NativeSelectInput';
Expand Down
10 changes: 10 additions & 0 deletions packages/material-ui/test/integration/Menu.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ describe('<Menu> integration', () => {
assert.strictEqual(document.activeElement, portalLayer.querySelectorAll('li')[2]);
});

it('should switch focus from the 3rd item to the 1st item when home key is pressed', () => {
TestUtils.Simulate.keyDown(portalLayer.querySelector('ul'), { which: keycode('home') });
assert.strictEqual(document.activeElement, portalLayer.querySelectorAll('li')[0]);
});

it('should switch focus from the 1st item to the 3rd item when end key is pressed', () => {
TestUtils.Simulate.keyDown(portalLayer.querySelector('ul'), { which: keycode('end') });
assert.strictEqual(document.activeElement, portalLayer.querySelectorAll('li')[2]);
});

it('should keep focus on the last item when a key with no associated action is pressed', () => {
TestUtils.Simulate.keyDown(portalLayer.querySelector('ul'), { which: keycode('right') });
assert.strictEqual(document.activeElement, portalLayer.querySelectorAll('li')[2]);
Expand Down