From 1299399eabc23f96b0ba40d1d8f1233213bbf372 Mon Sep 17 00:00:00 2001
From: Andrea Del Rio <delrio.andre@gmail.com>
Date: Mon, 14 Oct 2019 09:07:14 -0500
Subject: [PATCH] Improve a11y in EuiNavDrawer (#2417)

* updated docs and added focus state to button

* cl

* trying to make aria-label dynamic

* trying to make aria-label dynamic

* added missing underline property

* making hover and focus behave differently

* updated aria-labels

* remove unneeded line

* cl

* use nav and avoid a11y error

* remove role=presentation
---
 CHANGELOG.md                                |  1 +
 src-docs/src/views/nav_drawer/nav_drawer.js | 32 ++++++++------
 src/components/nav_drawer/_nav_drawer.scss  |  6 ++-
 src/components/nav_drawer/nav_drawer.js     | 47 ++++++++++++---------
 4 files changed, 50 insertions(+), 36 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 298710f9ff7..37dd62474da 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@
 - Added `component` prop to `EuiPageBody`, switching the default from `div` to `main` ([#2410](https://github.com/elastic/eui/pull/2410))
 - Added focus state to `EuiListGroupItem` ([#2406](https://github.com/elastic/eui/pull/2406))
 - Added `keyboardShorcut` glyph to 'EuiIcon ([#2413](https://github.com/elastic/eui/pull/2413))
+- Improved a11y in `EuiNavDrawer` ([#2417](https://github.com/elastic/eui/pull/2417))
 
 **Bug fixes**
 
diff --git a/src-docs/src/views/nav_drawer/nav_drawer.js b/src-docs/src/views/nav_drawer/nav_drawer.js
index 04df1470330..29a81960b07 100644
--- a/src-docs/src/views/nav_drawer/nav_drawer.js
+++ b/src-docs/src/views/nav_drawer/nav_drawer.js
@@ -50,7 +50,11 @@ export default class extends Component {
       color: 'subdued',
       iconType: 'pin',
       iconSize: 's',
-      'aria-label': 'Pin to top',
+    };
+
+    const pinExtraActionFn = val => {
+      pinExtraAction['aria-label'] = `Pin ${val} to top`;
+      return pinExtraAction;
     };
 
     this.topLinks = [
@@ -124,7 +128,7 @@ export default class extends Component {
         iconType: 'canvasApp',
         isActive: true,
         extraAction: {
-          ...pinExtraAction,
+          ...pinExtraActionFn('Canvas'),
           alwaysShow: true,
         },
       },
@@ -132,35 +136,35 @@ export default class extends Component {
         label: 'Discover',
         href: '#/layout/nav-drawer',
         iconType: 'discoverApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Discover') },
       },
       {
         label: 'Visualize',
         href: '#/layout/nav-drawer',
         iconType: 'visualizeApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Visualize') },
       },
       {
         label: 'Dashboard',
         href: '#/layout/nav-drawer',
         iconType: 'dashboardApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Dashboard') },
       },
       {
         label: 'Machine learning',
         href: '#/layout/nav-drawer',
         iconType: 'machineLearningApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Machine learning') },
       },
       {
         label: 'Custom Plugin (no icon)',
         href: '#/layout/nav-drawer',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Custom Plugin') },
       },
       {
         label: 'Nature Plugin (image as icon)',
         href: '#/layout/nav-drawer',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Nature Plugin') },
         icon: (
           <EuiImage
             size="s"
@@ -176,37 +180,37 @@ export default class extends Component {
         label: 'APM',
         href: '#/layout/nav-drawer',
         iconType: 'apmApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('APM') },
       },
       {
         label: 'Infrastructure',
         href: '#/layout/nav-drawer',
         iconType: 'infraApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Infrastructure') },
       },
       {
         label: 'Log viewer',
         href: '#/layout/nav-drawer',
         iconType: 'loggingApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Log viewer') },
       },
       {
         label: 'Uptime',
         href: '#/layout/nav-drawer',
         iconType: 'upgradeAssistantApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Uptime') },
       },
       {
         label: 'Maps',
         href: '#/layout/nav-drawer',
         iconType: 'gisApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('Maps') },
       },
       {
         label: 'SIEM',
         href: '#/layout/nav-drawer',
         iconType: 'securityAnalyticsApp',
-        extraAction: pinExtraAction,
+        extraAction: { ...pinExtraActionFn('SIEM') },
       },
     ];
 
diff --git a/src/components/nav_drawer/_nav_drawer.scss b/src/components/nav_drawer/_nav_drawer.scss
index 64511bd4669..bc47941f0b1 100644
--- a/src/components/nav_drawer/_nav_drawer.scss
+++ b/src/components/nav_drawer/_nav_drawer.scss
@@ -33,9 +33,13 @@
     transition: width $euiAnimSpeedExtraFast;
     z-index: $euiZHeader + 1;
 
-    .euiListGroupItem__button {
+    .navDrawerExpandButton-isExpanded .euiListGroupItem__button {
       padding: $euiSizeM $euiSize;
     }
+
+    .navDrawerExpandButton-isCollapsed .euiListGroupItem__button {
+      max-width: $euiSizeXL;
+    }
   }
 
   &.euiNavDrawer-isCollapsed  {
diff --git a/src/components/nav_drawer/nav_drawer.js b/src/components/nav_drawer/nav_drawer.js
index 2b67e1c810e..4c9c54ea840 100644
--- a/src/components/nav_drawer/nav_drawer.js
+++ b/src/components/nav_drawer/nav_drawer.js
@@ -6,7 +6,7 @@ import { EuiNavDrawerFlyout } from './nav_drawer_flyout';
 import { EuiNavDrawerGroup } from './nav_drawer_group';
 import { EuiOutsideClickDetector } from '../outside_click_detector';
 import { EuiI18n } from '../i18n';
-import { EuiFlexItem } from '../flex';
+import { EuiFlexItem, EuiFlexGroup } from '../flex';
 import { throttle } from '../color_picker/utils';
 
 export class EuiNavDrawer extends Component {
@@ -229,7 +229,7 @@ export class EuiNavDrawer extends Component {
     let footerContent;
     if (showExpandButton) {
       footerContent = (
-        <EuiListGroup className="euiNavDrawer__expandButton" flush>
+        <EuiListGroup className="euiNavDrawer__expandButton">
           <EuiI18n
             tokens={[
               'euiNavDrawer.sideNavCollapse',
@@ -256,6 +256,11 @@ export class EuiNavDrawer extends Component {
                 label={this.state.isCollapsed ? sideNavExpand : sideNavCollapse}
                 iconType={this.state.isCollapsed ? 'menuRight' : 'menuLeft'}
                 size="s"
+                className={
+                  this.state.isCollapsed
+                    ? 'navDrawerExpandButton-isCollapsed'
+                    : 'navDrawerExpandButton-isExpanded'
+                }
                 showToolTip={this.state.isCollapsed}
                 extraAction={{
                   className: 'euiNavDrawer__expandButtonLockAction',
@@ -319,25 +324,25 @@ export class EuiNavDrawer extends Component {
       <EuiOutsideClickDetector
         onOutsideClick={() => this.closeBoth()}
         isDisabled={this.state.outsideClickDisabled}>
-        <div
-          className={classes}
-          onBlur={this.focusOut}
-          onFocus={this.manageFocus}
-          {...rest}>
-          <EuiFlexItem grow={false}>
-            <div
-              id="navDrawerMenu"
-              className={menuClasses}
-              onClick={this.handleDrawerMenuClick}>
-              {/* Put expand button first so it's first in tab order then on toggle starts the tabbing of the items from the top */}
-              {/* TODO: Add a "skip navigation" keyboard only button */}
-              {footerContent}
-              {modifiedChildren}
-            </div>
-          </EuiFlexItem>
-
-          {flyoutContent}
-        </div>
+        <nav className={classes} {...rest}>
+          <EuiFlexGroup
+            gutterSize="none"
+            onBlur={this.focusOut}
+            onFocus={this.manageFocus}>
+            <EuiFlexItem grow={false}>
+              <div
+                id="navDrawerMenu"
+                className={menuClasses}
+                onClick={this.handleDrawerMenuClick}>
+                {/* Put expand button first so it's first in tab order then on toggle starts the tabbing of the items from the top */}
+                {/* TODO: Add a "skip navigation" keyboard only button */}
+                {modifiedChildren}
+                {footerContent}
+              </div>
+            </EuiFlexItem>
+            {flyoutContent}
+          </EuiFlexGroup>
+        </nav>
       </EuiOutsideClickDetector>
     );
   }