Make Discover field chooser items keyboard accessible. (#11591)

* Make Discover field chooser items keyboard accessible.
* Make records count link and plus/minus icons tabbable.
* Prevent scrolling when you hit spacebar to toggle a field.
* Add accessibleClickKeys service and kbnAccessibleClick directive, with tests.

src/core_plugins/kibana/public/discover/components/field_chooser/discover_field.html

@@ -5,21 +5,21 @@
   <div
     data-test-subj="field-{{::field.name}}"
     ng-click="toggleDetails(field)"
+    kbn-accessible-click
     class="sidebar-item-title discover-sidebar-item"
   >
     <field-name
       class="discover-sidebar-item-label"
       field="field"
     ></field-name>
-    <div class="discover-sidebar-item-actions">
-      <button
-        ng-if="field.name !== '_source'"
-        ng-click="toggleDisplay(field)"
-        ng-class="::field.display ? 'kuiButton--danger' : 'kuiButton--primary'"
-        ng-bind="::field.display ? 'remove' : 'add'"
-        class="kuiButton kuiButton--small kuiButton--primary"
-        data-test-subj="fieldToggle-{{::field.name}}"
-      ></button>
-    </div>
+
+    <button
+      ng-if="field.name !== '_source'"
+      ng-click="toggleDisplay(field)"
+      ng-class="::field.display ? 'kuiButton--danger' : 'kuiButton--primary'"
+      ng-bind="::field.display ? 'remove' : 'add'"
+      class="discover-sidebar-item-action kuiButton kuiButton--small"
+      data-test-subj="fieldToggle-{{::field.name}}"
+    ></button>
   </div>
 </li>

src/core_plugins/kibana/public/discover/components/field_chooser/discover_field.js

@@ -3,12 +3,11 @@ import html from 'plugins/kibana/discover/components/field_chooser/discover_fiel
 import _ from 'lodash';
 import 'ui/directives/css_truncate';
 import 'ui/directives/field_name';
+import 'ui/accessibility/kbn_accessible_click';
 import detailsHtml from 'plugins/kibana/discover/components/field_chooser/lib/detail_views/string.html';
 import { uiModules } from 'ui/modules';
 const app = uiModules.get('apps/discover');
 
-
-
 app.directive('discoverField', function ($compile) {
   return {
     restrict: 'E',

src/core_plugins/kibana/public/discover/components/field_chooser/lib/detail_views/string.html

@@ -4,6 +4,7 @@
       <span ng-if="!field.details.error" class="small discover-field-details-count">
         (
         <a
+          href=""
           ng-show="!indexPattern.metaFields.includes(field.name)"
           ng-click="onAddFilter('_exists_', field.name, '+')">
           {{::field.details.exists}}
@@ -23,15 +24,7 @@
 
     <div ng-if="!field.details.error">
       <div ng-repeat="bucket in ::field.details.buckets" class="discover-field-details-item">
-        <div>
-          <!-- Add/remove filter buttons -->
-          <span ng-show="field.filterable" class="pull-right">
-            <span aria-hidden="true" class="fa fa-search-minus pull-right discover-field-details-filter"
-              ng-click="onAddFilter(field, bucket.value, '-')"  data-test-subj="minus-{{::field.name}}-{{::bucket.display}}"></span>
-            <span aria-hidden="true" class="fa fa-search-plus pull-right discover-field-details-filter"
-              ng-click="onAddFilter(field, bucket.value, '+')" data-test-subj="plus-{{::field.name}}-{{::bucket.display}}"></span>
-          </span>
-
+        <div class="discover-field-details-item-title">
           <!-- Field value -->
           <div
             css-truncate
@@ -41,6 +34,34 @@
           >
             {{::bucket.display}} <em ng-show="bucket.display === ''">Empty string</em>
           </div>
+
+          <!-- Add/remove filter buttons -->
+          <div
+            class="discover-field-details-item-buttons"
+            ng-show="field.filterable"
+          >
+            <button
+              class="discover-field-details-item-button"
+              ng-click="onAddFilter(field, bucket.value, '+')"
+              data-test-subj="plus-{{::field.name}}-{{::bucket.display}}"
+            >
+              <span
+                aria-hidden="true"
+                class="kuiIcon fa-search-plus discover-field-details-filter"
+              ></span>
+            </button>
+
+            <button
+              class="discover-field-details-item-button"
+              ng-click="onAddFilter(field, bucket.value, '-')"
+              data-test-subj="minus-{{::field.name}}-{{::bucket.display}}"
+            >
+              <span
+                aria-hidden="true"
+                class="kuiIcon fa-search-minus discover-field-details-filter"
+              ></span>
+            </button>
+          </div>
         </div>
         <kbn-tooltip text="{{::bucket.count}}" placement="right" append-to-body="1">
           <progressbar value="bucket.percent" max="100" animate="false"><span>{{bucket.percent}}%</span></progressbar>

src/core_plugins/kibana/public/discover/styles/main.less

@@ -152,8 +152,9 @@
   padding-bottom: 0 !important; /* 1 */
   height: 32px;
 
-  &:hover {
-    .discover-sidebar-item-actions {
+  &:hover,
+  &:focus {
+    .discover-sidebar-item-action {
       opacity: 1;
     }
   }
@@ -168,10 +169,15 @@
   }
 
   /**
-   * 1. Only visually hide the actions, so that they're still accessible to screen readers.
+   * 1. Only visually hide the action, so that it's still accessible to screen readers.
+   * 2. When tabbed to, this element needs to be visible for keyboard accessibility.
    */
-  .discover-sidebar-item-actions {
+  .discover-sidebar-item-action {
     opacity: 0; /* 1 */
+
+    &:focus {
+      opacity: 1; /* 2 */
+    }
   }
 
 .discover-field-details {
@@ -198,6 +204,26 @@
     cursor: pointer;
   }
 
+  .discover-field-details-item-title {
+    display: flex;
+    align-items: center;
+    justify-content: space-between;
+  }
+
+  /**
+   * 1. If the field name is very long, don't let it sqash the buttons.
+   */
+  .discover-field-details-item-buttons {
+    flex: 0 0 auto; /* 1 */
+  }
+
+  .discover-field-details-item-button {
+    appearance: none;
+    border: none;
+    padding: 0;
+    background-color: transparent;
+  }
+
 /**
  * TODO: Refactor these selectors to be less specific.
  */

src/ui/public/accessibility/__tests__/kbn_accessible_click.js

@@ -0,0 +1,135 @@
+import angular from 'angular';
+import sinon from 'auto-release-sinon';
+import expect from 'expect.js';
+import ngMock from 'ng_mock';
+import '../kbn_accessible_click';
+import {
+  ENTER_KEY,
+  SPACE_KEY,
+} from '../accessible_click_keys';
+
+describe('kbnAccessibleClick directive', () => {
+  let $compile;
+  let $rootScope;
+
+  beforeEach(ngMock.module('kibana'));
+
+  beforeEach(ngMock.inject(function (_$compile_, _$rootScope_) {
+    $compile = _$compile_;
+    $rootScope = _$rootScope_;
+  }));
+
+  describe('throws an error', () => {
+    it('when the element is a button', () => {
+      const html = `<button kbn-accessible-click></button>`;
+      expect(() => {
+        $compile(html)($rootScope);
+      }).to.throwError(/kbnAccessibleClick doesn't need to be used on a button./);
+    });
+
+    it('when the element is a link with an href', () => {
+      const html = `<a href="#" kbn-accessible-click></a>`;
+      expect(() => {
+        $compile(html)($rootScope);
+      }).to.throwError(/kbnAccessibleClick doesn't need to be used on a link if it has a href attribute./);
+    });
+
+    it(`when the element doesn't have an ng-click`, () => {
+      const html = `<div kbn-accessible-click></div>`;
+      expect(() => {
+        $compile(html)($rootScope);
+      }).to.throwError(/kbnAccessibleClick requires ng-click to be defined on its element./);
+    });
+  });
+
+  describe(`doesn't throw an error`, () => {
+    it('when the element is a link without an href', () => {
+      const html = `<a ng-click="noop" kbn-accessible-click></a>`;
+      expect(() => {
+        $compile(html)($rootScope);
+      }).not.to.throwError();
+    });
+  });
+
+  describe('adds accessibility attributes', () => {
+    it('tabindex', () => {
+      const html = `<div ng-click="noop" kbn-accessible-click></div>`;
+      const element = $compile(html)($rootScope);
+      expect(element.attr('tabindex')).to.be('0');
+    });
+
+    it('role', () => {
+      const html = `<div ng-click="noop" kbn-accessible-click></div>`;
+      const element = $compile(html)($rootScope);
+      expect(element.attr('role')).to.be('button');
+    });
+  });
+
+  describe(`doesn't override pre-existing accessibility attributes`, () => {
+    it('tabindex', () => {
+      const html = `<div ng-click="noop" kbn-accessible-click tabindex="1"></div>`;
+      const element = $compile(html)($rootScope);
+      expect(element.attr('tabindex')).to.be('1');
+    });
+
+    it('role', () => {
+      const html = `<div ng-click="noop" kbn-accessible-click role="submit"></div>`;
+      const element = $compile(html)($rootScope);
+      expect(element.attr('role')).to.be('submit');
+    });
+  });
+
+  describe(`calls ng-click`, () => {
+    let scope;
+    let element;
+
+    beforeEach(function () {
+      scope = $rootScope.$new();
+      scope.handleClick = sinon.stub();
+      const html = `<div ng-click="handleClick()" kbn-accessible-click></div>`;
+      element = $compile(html)(scope);
+    });
+
+    it(`on ENTER keyup`, () => {
+      const e = angular.element.Event('keyup'); // eslint-disable-line new-cap
+      e.keyCode = ENTER_KEY;
+      element.trigger(e);
+      sinon.assert.calledOnce(scope.handleClick);
+    });
+
+    it(`on SPACE keyup`, () => {
+      const e = angular.element.Event('keyup'); // eslint-disable-line new-cap
+      e.keyCode = SPACE_KEY;
+      element.trigger(e);
+      sinon.assert.calledOnce(scope.handleClick);
+    });
+  });
+
+  describe(`doesn't call ng-click when the element being interacted with is a child`, () => {
+    let scope;
+    let child;
+
+    beforeEach(function () {
+      scope = $rootScope.$new();
+      scope.handleClick = sinon.stub();
+      const html = `<div ng-click="handleClick()" kbn-accessible-click></div>`;
+      const element = $compile(html)(scope);
+      child = angular.element(`<button></button>`);
+      element.append(child);
+    });
+
+    it(`on ENTER keyup`, () => {
+      const e = angular.element.Event('keyup'); // eslint-disable-line new-cap
+      e.keyCode = ENTER_KEY;
+      child.trigger(e);
+      expect(scope.handleClick.callCount).to.be(0);
+    });
+
+    it(`on SPACE keyup`, () => {
+      const e = angular.element.Event('keyup'); // eslint-disable-line new-cap
+      e.keyCode = SPACE_KEY;
+      child.trigger(e);
+      expect(scope.handleClick.callCount).to.be(0);
+    });
+  });
+});

src/ui/public/accessibility/accessible_click_keys.js

@@ -0,0 +1,8 @@
+export const ENTER_KEY = 13;
+export const SPACE_KEY = 32;
+
+// These keys are used to execute click actions on interactive elements like buttons and links.
+export const accessibleClickKeys = {
+  [ENTER_KEY]: 'enter',
+  [SPACE_KEY]: 'space',
+};

src/ui/public/accessibility/kbn_accessible_click.js

@@ -0,0 +1,85 @@
+/**
+ * Interactive elements must be able to receive focus.
+ *
+ * Ideally, this means using elements that are natively keyboard accessible (<a href="">,
+ * <input type="button">, or <button>). Note that links should be used when navigating and buttons
+ * should be used when performing an action on the page.
+ *
+ * If you need to use a <div>, <p>, or <a> without the href attribute, then you need to allow
+ * them to receive focus and to respond to keyboard input. The workaround is to:
+ *
+ *   - Give the element tabindex="0" so that it can receive keyboard focus.
+ *   - Add a JavaScript onkeyup event handler that triggers element functionality if the Enter key
+ *     is pressed while the element is focused. This is necessary because some browsers do not trigger
+ *    onclick events for such elements when activated via the keyboard.
+ *   - If the item is meant to function as a button, the onkeyup event handler should also detect the
+ *     Spacebar in addition to the Enter key, and the element should be given role="button".
+ *
+ * Apply this directive to any of these elements to automatically do the above.
+ */
+
+import {
+  accessibleClickKeys,
+  SPACE_KEY,
+} from './accessible_click_keys';
+import { uiModules } from 'ui/modules';
+
+uiModules.get('kibana')
+.directive('kbnAccessibleClick', function () {
+  return {
+    restrict: 'A',
+    controller: $element => {
+      $element.on('keydown', e => {
+        // If the user is interacting with a different element, then we don't need to do anything.
+        if (e.currentTarget !== e.target) {
+          return;
+        }
+
+        // Prevent a scroll from occurring if the user has hit space.
+        if (e.keyCode === SPACE_KEY) {
+          e.preventDefault();
+        }
+      });
+    },
+    link: (scope, element, attrs) => {
+      // The whole point of this directive is to hack in functionality that native buttons provide
+      // by default.
+      const elementType = element.prop('tagName');
+
+      if (elementType === 'BUTTON') {
+        throw new Error(`kbnAccessibleClick doesn't need to be used on a button.`);
+      }
+
+      if (elementType === 'A' && attrs.href !== undefined) {
+        throw new Error(`kbnAccessibleClick doesn't need to be used on a link if it has a href attribute.`);
+      }
+
+      // We're emulating a click action, so we should already have a regular click handler defined.
+      if (!attrs.ngClick) {
+        throw new Error('kbnAccessibleClick requires ng-click to be defined on its element.');
+      }
+
+      // If the developer hasn't already specified attributes required for accessibility, add them.
+      if (attrs.tabindex === undefined) {
+        element.attr('tabindex', '0');
+      }
+
+      if (attrs.role === undefined) {
+        element.attr('role', 'button');
+      }
+
+      element.on('keyup', e => {
+        // If the user is interacting with a different element, then we don't need to do anything.
+        if (e.currentTarget !== e.target) {
+          return;
+        }
+
+        // Support keyboard accessibility by emulating mouse click on ENTER or SPACE keypress.
+        if (accessibleClickKeys[e.keyCode]) {
+          // Delegate to the click handler on the element (assumed to be ng-click).
+          element.click();
+        }
+      });
+    },
+  };
+});