Skip to content

Commit

Permalink
Make Discover field chooser items keyboard accessible. (#11591) (#11683)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
cjcenizal authored May 9, 2017
1 parent 9eb0af7 commit 8013644
Show file tree
Hide file tree
Showing 7 changed files with 299 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ <h5 ng-show="!field.details.error">Quick Count <kbn-info info="Top 5 values base
<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}}
Expand All @@ -23,15 +24,7 @@ <h5 ng-show="!field.details.error">Quick Count <kbn-info info="Top 5 values base

<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
Expand All @@ -41,6 +34,34 @@ <h5 ng-show="!field.details.error">Quick Count <kbn-info info="Top 5 values base
>
{{::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>
Expand Down
34 changes: 30 additions & 4 deletions src/core_plugins/kibana/public/discover/styles/main.less
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,9 @@
padding-bottom: 0 !important; /* 1 */
height: 32px;

&:hover {
.discover-sidebar-item-actions {
&:hover,
&:focus {
.discover-sidebar-item-action {
opacity: 1;
}
}
Expand All @@ -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 {
Expand All @@ -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.
*/
Expand Down
135 changes: 135 additions & 0 deletions src/ui/public/accessibility/__tests__/kbn_accessible_click.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
8 changes: 8 additions & 0 deletions src/ui/public/accessibility/accessible_click_keys.js
Original file line number Diff line number Diff line change
@@ -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',
};
85 changes: 85 additions & 0 deletions src/ui/public/accessibility/kbn_accessible_click.js
Original file line number Diff line number Diff line change
@@ -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();
}
});
},
};
});

0 comments on commit 8013644

Please sign in to comment.