From bdd070e55815e95fb66670e40b3ab27472bd2305 Mon Sep 17 00:00:00 2001
From: TJ Egan <twegan@us.ibm.com>
Date: Tue, 18 Sep 2018 14:48:08 -0500
Subject: [PATCH] fix(overflow-menu): improve overall a11y

---
 .../overflow-menu/overflow-menu.hbs           | 31 ++++++++++++-------
 src/components/overflow-menu/overflow-menu.js | 27 +++++++++++-----
 2 files changed, 39 insertions(+), 19 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..7e195a824bb8 100644
--- a/src/components/overflow-menu/overflow-menu.js
+++ b/src/components/overflow-menu/overflow-menu.js
@@ -81,7 +81,7 @@ class OverflowMenu extends mixin(createComponent, initComponentBySearch, evented
       })
     );
     this.manage(
-      on(this.element.ownerDocument, 'keypress', event => {
+      on(this.element.ownerDocument, 'keydown', event => {
         this._handleKeyPress(event);
       })
     );
@@ -99,6 +99,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,17 +162,24 @@ 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') {
-          event.preventDefault();
-        }
+        event.preventDefault();
         event.delegateTarget = element; // eslint-disable-line no-param-reassign
       }