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.

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

EuiNavDrawer focus management, flyout auto-close #2749

Merged
merged 14 commits into from
Jan 10, 2020
6 changes: 3 additions & 3 deletions src-docs/src/views/nav_drawer/nav_drawer.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,9 @@ export default class extends Component {

onKeyDown = event => {
if (event.keyCode === keyCodes.ESCAPE) {
event.preventDefault();
event.stopPropagation();
this.closeFullScreen();
// event.preventDefault();
// event.stopPropagation();
// this.closeFullScreen();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will revert this or make this not collide with new ESC behavior

}
};

Expand Down
72 changes: 66 additions & 6 deletions src/components/nav_drawer/__snapshots__/nav_drawer.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,28 @@ exports[`EuiNavDrawer is rendered 1`] = `
id="navDrawerFlyoutTitle"
tabindex="-1"
/>
<ul
class="euiListGroup euiListGroup-maxWidthDefault euiNavDrawerGroup euiNavDrawerFlyout__listGroup"
<div
data-focus-guard="true"
style="width:1px;height:0px;padding:0;overflow:hidden;position:fixed;top:1px;left:1px"
tabindex="0"
/>
<div
data-focus-guard="true"
style="width:1px;height:0px;padding:0;overflow:hidden;position:fixed;top:1px;left:1px"
tabindex="1"
/>
<div
data-focus-lock-disabled="false"
>
<ul
class="euiListGroup euiListGroup-maxWidthDefault euiNavDrawerGroup euiNavDrawerFlyout__listGroup"
id="flyoutLinks"
/>
</div>
<div
data-focus-guard="true"
style="width:1px;height:0px;padding:0;overflow:hidden;position:fixed;top:1px;left:1px"
tabindex="0"
/>
</div>
</div>
Expand Down Expand Up @@ -534,8 +554,28 @@ exports[`EuiNavDrawer renders with falsy children 1`] = `
id="navDrawerFlyoutTitle"
tabindex="-1"
/>
<ul
class="euiListGroup euiListGroup-maxWidthDefault euiNavDrawerGroup euiNavDrawerFlyout__listGroup"
<div
data-focus-guard="true"
style="width:1px;height:0px;padding:0;overflow:hidden;position:fixed;top:1px;left:1px"
tabindex="0"
/>
<div
data-focus-guard="true"
style="width:1px;height:0px;padding:0;overflow:hidden;position:fixed;top:1px;left:1px"
tabindex="1"
/>
<div
data-focus-lock-disabled="false"
>
<ul
class="euiListGroup euiListGroup-maxWidthDefault euiNavDrawerGroup euiNavDrawerFlyout__listGroup"
id="flyoutLinks"
/>
</div>
<div
data-focus-guard="true"
style="width:1px;height:0px;padding:0;overflow:hidden;position:fixed;top:1px;left:1px"
tabindex="0"
/>
</div>
</div>
Expand Down Expand Up @@ -836,8 +876,28 @@ exports[`EuiNavDrawer renders with fragments 1`] = `
id="navDrawerFlyoutTitle"
tabindex="-1"
/>
<ul
class="euiListGroup euiListGroup-maxWidthDefault euiNavDrawerGroup euiNavDrawerFlyout__listGroup"
<div
data-focus-guard="true"
style="width:1px;height:0px;padding:0;overflow:hidden;position:fixed;top:1px;left:1px"
tabindex="0"
/>
<div
data-focus-guard="true"
style="width:1px;height:0px;padding:0;overflow:hidden;position:fixed;top:1px;left:1px"
tabindex="1"
/>
<div
data-focus-lock-disabled="false"
>
<ul
class="euiListGroup euiListGroup-maxWidthDefault euiNavDrawerGroup euiNavDrawerFlyout__listGroup"
id="flyoutLinks"
/>
</div>
<div
data-focus-guard="true"
style="width:1px;height:0px;padding:0;overflow:hidden;position:fixed;top:1px;left:1px"
tabindex="0"
/>
</div>
</div>
Expand Down
40 changes: 30 additions & 10 deletions src/components/nav_drawer/nav_drawer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { EuiI18n } from '../i18n';
import { EuiFlexItem, EuiFlexGroup } from '../flex';
import { throttle } from '../color_picker/utils';

const MENU_ELEMENT_ID = 'navDrawerMenu';

export class EuiNavDrawer extends Component {
constructor(props) {
super(props);
Expand All @@ -21,6 +23,7 @@ export class EuiNavDrawer extends Component {
outsideClickDisabled: true,
isManagingFocus: false,
toolTipsEnabled: true,
focusReturnRef: null,
};
}

Expand Down Expand Up @@ -161,7 +164,7 @@ export class EuiNavDrawer extends Component {
}, 0);
};

expandFlyout = (links, title) => {
expandFlyout = (links, title, item) => {
const content = links;

if (this.state.navFlyoutTitle === title) {
Expand All @@ -174,22 +177,38 @@ export class EuiNavDrawer extends Component {
isCollapsed: this.state.isLocked ? false : true,
toolTipsEnabled: false,
outsideClickDisabled: false,
focusReturnRef: item.label,
});
}
};

collapseFlyout = () => {
this.setState({
flyoutIsCollapsed: true,
navFlyoutTitle: null,
navFlyoutContent: null,
toolTipsEnabled: this.state.isLocked ? false : true,
});
collapseFlyout = (shouldReturnFocus = true) => {
const focusReturn = this.state.focusReturnRef;
this.setState(
{
flyoutIsCollapsed: true,
navFlyoutTitle: null,
navFlyoutContent: null,
toolTipsEnabled: this.state.isLocked ? false : true,
focusReturnRef: null,
},
() => {
if (shouldReturnFocus) {
// Ideally this uses React `ref` instead of `querySelector`, but the menu composition
// does not allow for deep `ref` element management at present
const element = document.querySelector(
`#${MENU_ELEMENT_ID} [aria-label='${focusReturn}']`
);
if (!element) return;
requestAnimationFrame(() => element.focus());
}
}
);
};

closeBoth = () => {
if (!this.state.isLocked) this.collapseDrawer();
this.collapseFlyout();
this.collapseFlyout(false);
};

handleDrawerMenuClick = e => {
Expand Down Expand Up @@ -325,6 +344,7 @@ export class EuiNavDrawer extends Component {
isCollapsed={this.state.flyoutIsCollapsed}
listItems={this.state.navFlyoutContent}
wrapText
onClose={this.collapseFlyout}
/>
);

Expand All @@ -349,7 +369,7 @@ export class EuiNavDrawer extends Component {
onFocus={this.manageFocus}>
<EuiFlexItem grow={false}>
<div
id="navDrawerMenu"
id={MENU_ELEMENT_ID}
className={menuClasses}
onClick={this.handleDrawerMenuClick}>
{/* TODO: Add a "skip navigation" keyboard only button */}
Expand Down
36 changes: 30 additions & 6 deletions src/components/nav_drawer/nav_drawer_flyout.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,20 @@ import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';

import { keyCodes } from '../../services';

import { EuiTitle } from '../title';
import { EuiNavDrawerGroup } from './nav_drawer_group';
import { EuiListGroup } from '../list_group/list_group';
import { EuiFocusTrap } from '../focus_trap';

export const EuiNavDrawerFlyout = ({
className,
title,
isCollapsed,
listItems,
wrapText,
onClose,
...rest
}) => {
const classes = classNames(
Expand All @@ -23,16 +27,30 @@ export const EuiNavDrawerFlyout = ({
className
);

const handleKeyDown = e => {
if (e.keyCode === keyCodes.ESCAPE) {
onClose();
}
};

return (
<div className={classes} aria-labelledby="navDrawerFlyoutTitle" {...rest}>
<div
className={classes}
aria-labelledby="navDrawerFlyoutTitle"
onKeyDown={handleKeyDown}
{...rest}>
<EuiTitle className="euiNavDrawerFlyout__title" tabIndex="-1" size="xxs">
<div id="navDrawerFlyoutTitle">{title}</div>
</EuiTitle>
<EuiNavDrawerGroup
className="euiNavDrawerFlyout__listGroup"
listItems={listItems}
wrapText={wrapText}
/>
<EuiFocusTrap returnFocus={false}>
<EuiNavDrawerGroup
id="flyoutLinks"
className="euiNavDrawerFlyout__listGroup"
listItems={listItems}
wrapText={wrapText}
onClose={() => onClose(false)}
/>
</EuiFocusTrap>
</div>
);
};
Expand All @@ -51,6 +69,12 @@ EuiNavDrawerFlyout.propTypes = {
* Toggle the nav drawer between collapsed and expanded
*/
isCollapsed: PropTypes.bool,

/**
* Passthrough function to be called when the flyout is closing
* See ./nav_drawer.js
*/
onClose: PropTypes.func,
};

EuiNavDrawerFlyout.defaultProps = {
Expand Down
17 changes: 15 additions & 2 deletions src/components/nav_drawer/nav_drawer_group.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const EuiNavDrawerGroup = ({
className,
listItems,
flyoutMenuButtonClick,
onClose = () => {},
...rest
}) => {
const classes = classNames('euiNavDrawerGroup', className);
Expand All @@ -20,11 +21,18 @@ export const EuiNavDrawerGroup = ({
? undefined
: listItems.map(item => {
// If the flyout menu exists, pass back the list of times and the title with the onClick handler of the item
const { flyoutMenu, ...itemProps } = item;
const { flyoutMenu, onClick, ...itemProps } = item;
if (flyoutMenu && flyoutMenuButtonClick) {
const items = [...flyoutMenu.listItems];
const title = `${flyoutMenu.title}`;
itemProps.onClick = () => flyoutMenuButtonClick(items, title);
itemProps.onClick = e => flyoutMenuButtonClick(items, title, item, e);
} else {
itemProps.onClick = (...args) => {
if (onClick) {
onClick(...args);
}
onClose();
};
}

// Make some declarations of props for the side nav implementation
Expand Down Expand Up @@ -69,4 +77,9 @@ EuiNavDrawerGroup.propTypes = {
* of the flyout menu button click
*/
flyoutMenuButtonClick: PropTypes.func,
/**
* Passthrough function to be called when the flyout is closing
* See ./nav_drawer.js
*/
onClose: PropTypes.func,
};