From 9ecc07492a29b22f077de68f25e4f4a3e6653241 Mon Sep 17 00:00:00 2001
From: Kasper Nilsson <kaspern@google.com>
Date: Fri, 16 Jun 2017 13:14:31 -0700
Subject: [PATCH] Add keyboard nav to gr-dropdown

Emulates native select behavior using iron-a11y-keys.

Bug: Issue 6435
Change-Id: I73f114845762700c4daf099ee76d795bf4329d35
---
 .../shared/gr-dropdown/gr-dropdown.html       | 25 +++++-
 .../shared/gr-dropdown/gr-dropdown.js         | 81 +++++++++++++++++--
 .../shared/gr-dropdown/gr-dropdown_test.html  | 56 ++++++++++++-
 3 files changed, 149 insertions(+), 13 deletions(-)

diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
index 1e12b04ecd..9041031aab 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
@@ -15,9 +15,11 @@ limitations under the License.
 -->
 
 <link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
+<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
 <link rel="import" href="../../../bower_components/polymer/polymer.html">
 <link rel="import" href="../../../bower_components/iron-dropdown/iron-dropdown.html">
 <link rel="import" href="../../shared/gr-button/gr-button.html">
+<link rel="import" href="../../shared/gr-cursor-manager/gr-cursor-manager.html">
 <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
 <link rel="import" href="../../../styles/shared-styles.html">
 
@@ -80,6 +82,11 @@ limitations under the License.
         background-color: #6B82D6;
         color: #fff;
       }
+      li:focus,
+      li.selected {
+        background-color: #EBF5FB;
+        outline: none;
+      }
       .topContent {
         display: block;
         padding: .85em 1em;
@@ -123,7 +130,9 @@ limitations under the License.
                   items="[[topContent]]"
                   as="item"
                   initial-count="75">
-                <div class$="[[_getClassIfBold(item.bold)]] top-item">
+                <div
+                    class$="[[_getClassIfBold(item.bold)]] top-item"
+                    tabindex="-1">
                   [[item.text]]
                 </div>
               </template>
@@ -134,23 +143,31 @@ limitations under the License.
               items="[[items]]"
               as="link"
               initial-count="75">
-            <li>
+            <li tabindex="-1">
               <span
                   class$="itemAction [[_computeDisabledClass(link.id, disabledIds.*)]]"
                   data-id$="[[link.id]]"
                   on-tap="_handleItemTap"
-                  hidden$="[[link.url]]">[[link.name]]</span>
+                  hidden$="[[link.url]]"
+                  tabindex="-1">[[link.name]]</span>
               <a
                   class="itemAction"
                   href$="[[_computeLinkURL(link)]]"
                   rel$="[[_computeLinkRel(link)]]"
                   target$="[[link.target]]"
-                  hidden$="[[!link.url]]">[[link.name]]</a>
+                  hidden$="[[!link.url]]"
+                  tabindex="-1">[[link.name]]</a>
             </li>
           </template>
         </ul>
       </div>
     </iron-dropdown>
+    <gr-cursor-manager
+        id="cursor"
+        cursor-target-class="selected"
+        scroll-behavior="never"
+        focus-on-move
+        stops="[[_els]]"</gr-cursor-manager>
     <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
   </template>
   <script src="gr-dropdown.js"></script>
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
index a034eb869d..4c463747b7 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
@@ -30,7 +30,10 @@
      */
 
     properties: {
-      items: Array,
+      items: {
+        type: Array,
+        observer: '_resetCursorStops',
+      },
       topContent: Object,
       horizontalAlign: {
         type: String,
@@ -60,18 +63,76 @@
       },
 
       _hasAvatars: String,
+
+      /**
+       * The elements of the list.
+       */
+      _listElements: {
+        type: Array,
+        value() { return []; },
+      },
     },
 
     behaviors: [
       Gerrit.BaseUrlBehavior,
+      Gerrit.KeyboardShortcutBehavior,
     ],
 
+    keyBindings: {
+      'down': '_handleDown',
+      'enter space': '_handleEnter',
+      'tab': '_handleTab',
+      'up': '_handleUp',
+    },
+
     attached() {
       this.$.restAPI.getConfig().then(cfg => {
         this._hasAvatars = !!(cfg && cfg.plugin && cfg.plugin.has_avatars);
       });
     },
 
+    _handleUp(e) {
+      if (this.$.dropdown.opened) {
+        e.preventDefault();
+        e.stopPropagation();
+        this.$.cursor.previous();
+      } else {
+        this._open();
+      }
+    },
+
+    _handleDown(e) {
+      if (this.$.dropdown.opened) {
+        e.preventDefault();
+        e.stopPropagation();
+        this.$.cursor.next();
+      } else {
+        this._open();
+      }
+    },
+
+    _handleTab(e) {
+      if (this.$.dropdown.opened) {
+        // Tab in a native select is a no-op. Emulate this.
+        e.preventDefault();
+        e.stopPropagation();
+      }
+    },
+
+    _handleEnter(e) {
+      e.preventDefault();
+      e.stopPropagation();
+      if (this.$.dropdown.opened) {
+        // TODO(kaspern): This solution will not work in Shadow DOM, and
+        // is not particularly robust in general. Find a better solution
+        // when page.js has been abstracted away from components.
+        const el = this.$.cursor.target.querySelector(':not([hidden])');
+        if (el) { el.click(); }
+      } else {
+        this._open();
+      }
+    },
+
     _handleDropdownTap(e) {
       // async is needed so that that the click event is fired before the
       // dropdown closes (This was a bug for touch devices).
@@ -81,7 +142,14 @@
     },
 
     _showDropdownTapHandler(e) {
+      this._open();
+    },
+
+    _open() {
       this.$.dropdown.open();
+      this.$.cursor.setCursorAtIndex(0);
+      Polymer.dom.flush();
+      this.$.cursor.target.focus();
     },
 
     _getClassIfBold(bold) {
@@ -113,9 +181,7 @@
 
     _handleItemTap(e) {
       const id = e.target.getAttribute('data-id');
-      const item = this.items.find(item => {
-        return item.id === id;
-      });
+      const item = this.items.find(item => item.id === id);
       if (id && !this.disabledIds.includes(id)) {
         if (item) {
           this.dispatchEvent(new CustomEvent('tap-item', {detail: item}));
@@ -127,5 +193,10 @@
     _computeDisabledClass(id, disabledIdsRecord) {
       return disabledIdsRecord.base.includes(id) ? 'disabled' : '';
     },
+
+    _resetCursorStops() {
+      Polymer.dom.flush();
+      this._els = this.querySelectorAll('li');
+    },
   });
-})();
+})();
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
index c951cd4d65..e335870e83 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
@@ -34,15 +34,22 @@ limitations under the License.
 <script>
   suite('gr-dropdown tests', () => {
     let element;
+    let sandbox;
 
     setup(() => {
       stub('gr-rest-api-interface', {
         getConfig() { return Promise.resolve({}); },
       });
       element = fixture('basic');
+      sandbox = sinon.sandbox.create();
+    });
+
+    teardown(() => {
+      sandbox.restore();
     });
 
     test('tap on trigger opens menu', () => {
+      sandbox.stub(element, '_open', () => {element.$.dropdown.open();});
       assert.isFalse(element.$.dropdown.opened);
       MockInteractions.tap(element.$.trigger);
       assert.isTrue(element.$.dropdown.opened);
@@ -91,8 +98,8 @@ limitations under the License.
     test('non link items', () => {
       const item0 = {name: 'item one', id: 'foo'};
       element.items = [item0, {name: 'item two', id: 'bar'}];
-      const fooTapped = sinon.stub();
-      const tapped = sinon.stub();
+      const fooTapped = sandbox.stub();
+      const tapped = sandbox.stub();
       element.addEventListener('tap-item-foo', fooTapped);
       element.addEventListener('tap-item', tapped);
       flushAsynchronousOperations();
@@ -106,8 +113,8 @@ limitations under the License.
       element.items = [{name: 'item one', id: 'foo'}];
       element.disabledIds = ['foo'];
 
-      const stub = sinon.stub();
-      const tapped = sinon.stub();
+      const stub = sandbox.stub();
+      const tapped = sandbox.stub();
       element.addEventListener('tap-item-foo', stub);
       element.addEventListener('tap-item', tapped);
       flushAsynchronousOperations();
@@ -115,5 +122,46 @@ limitations under the License.
       assert.isFalse(stub.called);
       assert.isFalse(tapped.called);
     });
+
+    suite('keyboard navigation', () => {
+      setup(() => {
+        element.items = [
+          {name: 'item one', id: 'foo'},
+          {name: 'item two', id: 'bar'},
+        ];
+        flushAsynchronousOperations();
+      });
+
+      test('down', () => {
+        const stub = sandbox.stub(element.$.cursor, 'next');
+        assert.isFalse(element.$.dropdown.opened);
+        MockInteractions.pressAndReleaseKeyOn(element, 40);
+        assert.isTrue(element.$.dropdown.opened);
+        MockInteractions.pressAndReleaseKeyOn(element, 40);
+        assert.isTrue(stub.called);
+      });
+
+      test('up', () => {
+        const stub = sandbox.stub(element.$.cursor, 'previous');
+        assert.isFalse(element.$.dropdown.opened);
+        MockInteractions.pressAndReleaseKeyOn(element, 38);
+        assert.isTrue(element.$.dropdown.opened);
+        MockInteractions.pressAndReleaseKeyOn(element, 38);
+        assert.isTrue(stub.called);
+      });
+
+      test('enter/space', () => {
+        // Because enter and space are handled by the same fn, we need only to
+        // test one.
+        assert.isFalse(element.$.dropdown.opened);
+        MockInteractions.pressAndReleaseKeyOn(element, 32); // Space
+        assert.isTrue(element.$.dropdown.opened);
+
+        const el = element.$.cursor.target.querySelector(':not([hidden])');
+        const stub = sandbox.stub(el, 'click');
+        MockInteractions.pressAndReleaseKeyOn(element, 32); // Space
+        assert.isTrue(stub.called);
+      });
+    });
   });
 </script>