From c7bdb3105e128d6e7f3253d25769979e6aa15c37 Mon Sep 17 00:00:00 2001
From: TJ Egan <tw15egan@gmail.com>
Date: Mon, 24 Sep 2018 10:48:18 -0500
Subject: [PATCH] fix(overflow-menu): improve overall a11y (#1115)

* fix(overflow-menu): improve overall a11y

* fix(overflow-menu): prevent fewer defaults

* fix(overflow-menu): update logic

* fix(overflow-menu): add comment
---
 .../overflow-menu/overflow-menu.hbs           | 31 +++++++++-------
 src/components/overflow-menu/overflow-menu.js | 35 ++++++++++++++++---
 2 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/src/components/overflow-menu/overflow-menu.hbs b/src/components/overflow-menu/overflow-menu.hbs
index 46b3c93ca80c..6f7a20018575 100644
--- a/src/components/overflow-menu/overflow-menu.hbs
+++ b/src/components/overflow-menu/overflow-menu.hbs
@@ -1,33 +1,40 @@
-<div data-overflow-menu tabindex="0" aria-label="Overflow menu description" class="bx--overflow-menu">
-  <svg class="bx--overflow-menu__icon" width="3" height="15" viewBox="0 0 3 15">
+<div data-overflow-menu tabindex="0" aria-label="Overflow" class="bx--overflow-menu" role="button" aria-haspopup="true"
+  aria-expanded="false">
+  <svg aria-hidden="true" class="bx--overflow-menu__icon" width="3" height="15" viewBox="0 0 3 15">
     <g fill-rule="evenodd">
       <circle cx="1.5" cy="1.5" r="1.5" />
       <circle cx="1.5" cy="7.5" r="1.5" />
       <circle cx="1.5" cy="13.5" r="1.5" />
     </g>
   </svg>
-  <ul class="bx--overflow-menu-options" tabindex="-1" data-floating-menu-direction="{{direction}}">
+  <ul class="bx--overflow-menu-options" tabindex="-1" role="menu" aria-label="Overflow" data-floating-menu-direction="{{direction}}">
     {{#each items}}
-      <li class="bx--overflow-menu-options__option{{#if disabled}} bx--overflow-menu-options__option--disabled{{/if}}{{#if danger}} bx--overflow-menu-options__option--danger{{/if}}">
-        <button class="bx--overflow-menu-options__btn"{{#if title}} title="{{title}}"{{/if}}{{#if primaryFocus}} data-floating-menu-primary-focus{{/if}}{{#if disabled}} tabindex="-1"{{/if}}>{{label}}</button>
-      </li>
+    <li class="bx--overflow-menu-options__option{{#if disabled}} bx--overflow-menu-options__option--disabled{{/if}}{{#if danger}} bx--overflow-menu-options__option--danger{{/if}}"
+      role="presentation">
+      <button class="bx--overflow-menu-options__btn" role="menuitem" {{#if title}} title="{{title}}" {{/if}}
+        {{#if primaryFocus}} data-floating-menu-primary-focus{{/if}}{{#if disabled}} tabindex="-1" {{/if}}>{{label}}</button>
+    </li>
     {{/each}}
   </ul>
 </div>
 
-<div data-overflow-menu tabindex="0" aria-label="Overflow menu description" class="bx--overflow-menu">
-  <svg class="bx--overflow-menu__icon" width="3" height="15" viewBox="0 0 3 15">
+<div data-overflow-menu tabindex="0" aria-label="Overflow" class="bx--overflow-menu" role="button" aria-haspopup="true"
+  aria-expanded="false">
+  <svg aria-hidden="true" class="bx--overflow-menu__icon" width="3" height="15" viewBox="0 0 3 15">
     <g fill-rule="evenodd">
       <circle cx="1.5" cy="1.5" r="1.5" />
       <circle cx="1.5" cy="7.5" r="1.5" />
       <circle cx="1.5" cy="13.5" r="1.5" />
     </g>
   </svg>
-  <ul class="bx--overflow-menu-options bx--overflow-menu--flip" tabindex="-1" data-floating-menu-direction="{{direction}}">
+  <ul class="bx--overflow-menu-options bx--overflow-menu--flip" tabindex="-1" data-floating-menu-direction="{{direction}}"
+    role="menu" aria-label="Overflow">
     {{#each items}}
-      <li class="bx--overflow-menu-options__option{{#if disabled}} bx--overflow-menu-options__option--disabled{{/if}}{{#if danger}} bx--overflow-menu-options__option--danger{{/if}}">
-        <button class="bx--overflow-menu-options__btn"{{#if title}} title="{{title}}"{{/if}}{{#if primaryFocus}} data-floating-menu-primary-focus{{/if}}{{#if disabled}} tabindex="-1"{{/if}}>{{label}}</button>
-      </li>
+    <li class="bx--overflow-menu-options__option{{#if disabled}} bx--overflow-menu-options__option--disabled{{/if}}{{#if danger}} bx--overflow-menu-options__option--danger{{/if}}"
+      role="presentation">
+      <button class="bx--overflow-menu-options__btn" role="menuitem" {{#if title}} title="{{title}}" {{/if}}
+        {{#if primaryFocus}} data-floating-menu-primary-focus{{/if}}{{#if disabled}} tabindex="-1" {{/if}}>{{label}}</button>
+    </li>
     {{/each}}
   </ul>
 </div>
diff --git a/src/components/overflow-menu/overflow-menu.js b/src/components/overflow-menu/overflow-menu.js
index 297eb3e8bb90..1af3e7ddda3a 100644
--- a/src/components/overflow-menu/overflow-menu.js
+++ b/src/components/overflow-menu/overflow-menu.js
@@ -80,9 +80,18 @@ class OverflowMenu extends mixin(createComponent, initComponentBySearch, evented
         this.wasOpenBeforeClick = undefined;
       })
     );
+    this.manage(
+      on(this.element.ownerDocument, 'keydown', event => {
+        if (event.which === 27) {
+          this._handleKeyPress(event);
+        }
+      })
+    );
     this.manage(
       on(this.element.ownerDocument, 'keypress', event => {
-        this._handleKeyPress(event);
+        if (event.which !== 27) {
+          this._handleKeyPress(event);
+        }
       })
     );
     this.manage(
@@ -99,6 +108,12 @@ class OverflowMenu extends mixin(createComponent, initComponentBySearch, evented
    * @param {Function} callback Callback called when change in state completes.
    */
   changeState(state, detail, callback) {
+    if (state === 'hidden') {
+      this.element.setAttribute('aria-expanded', 'false');
+    } else {
+      this.element.setAttribute('aria-expanded', 'true');
+    }
+
     if (!this.optionMenu) {
       const optionMenu = this.element.querySelector(this.options.selectorOptionMenu);
       if (!optionMenu) {
@@ -156,15 +171,25 @@ class OverflowMenu extends mixin(createComponent, initComponentBySearch, evented
    */
   _handleKeyPress(event) {
     const key = event.which;
-    if (key === 13) {
-      const { element, optionMenu, options } = this;
+    const { element, optionMenu, options } = this;
+    const isOfMenu = optionMenu && optionMenu.element.contains(event.target);
+
+    if (key === 27) {
+      this.changeState('hidden', getLaunchingDetails(event), () => {
+        if (isOfMenu) {
+          element.focus();
+        }
+      });
+    }
+
+    if (key === 13 || key === 32) {
       const isOfSelf = element.contains(event.target);
-      const isOfMenu = optionMenu && optionMenu.element.contains(event.target);
       const shouldBeOpen = isOfSelf && !element.classList.contains(options.classShown);
       const state = shouldBeOpen ? 'shown' : 'hidden';
 
       if (isOfSelf) {
-        if (element.tagName === 'A') {
+        // 32 is to prevent screen from jumping when menu is opened with spacebar
+        if (element.tagName === 'A' || key === 32) {
           event.preventDefault();
         }
         event.delegateTarget = element; // eslint-disable-line no-param-reassign