Improve accessibility of the discover app (elastic#13498)

* Add sectioning to discover app, fix elastic#12633 * Set unique page title for discover app, fix elastic#12641 * Remove sort button if column isn't sortable Until now the button was just empty, but could still be focused via keyboard. This is bad accessibility, so the sort button is now removed completelyw hen the column isn't sortable.
Bargs · Aug 15, 2017 · 67e6d1d · 67e6d1d
1 parent ff40688
commit 67e6d1d
Show file tree

Hide file tree

Showing 7 changed files with 141 additions and 123 deletions.
diff --git a/src/core_plugins/kibana/public/discover/components/field_chooser/field_chooser.html b/src/core_plugins/kibana/public/discover/components/field_chooser/field_chooser.html
@@ -1,4 +1,4 @@
-<div class="sidebar-list">
+<section class="sidebar-list" aria-label="Index and fields">
   <div ng-show="indexPatternList.length > 1">
     <ui-select
       class="index-pattern-selection"
@@ -158,4 +158,4 @@ <h6>Popular</h6>
     </discover-field>
   </ul>
 
-</div>
+</section>
diff --git a/src/core_plugins/kibana/public/discover/controllers/discover.js b/src/core_plugins/kibana/public/discover/controllers/discover.js
@@ -167,9 +167,8 @@ function discoverController(
   .highlightAll(true)
   .version(true);
 
-  if (savedSearch.id) {
-    docTitle.change(savedSearch.title);
-  }
+  const pageTitleSuffix = savedSearch.id && savedSearch.title ? `: ${savedSearch.title}` : '';
+  docTitle.change(`Discover${pageTitleSuffix}`);
 
   let stateMonitor;
   const $appStatus = $scope.appStatus = this.appStatus = {

diff --git a/src/core_plugins/kibana/public/discover/index.html b/src/core_plugins/kibana/public/discover/index.html
@@ -75,7 +75,7 @@ <h2>Searching</h2>
               </span>
             </button>
 
-            <div class="discover-timechart" ng-if="opts.timefield">
+            <section aria-label="Histogram of found documents" class="discover-timechart" ng-if="opts.timefield">
               <header>
                 <center class="small">
                   <span tooltip="To change the time, click the clock icon in the navigation bar">{{timeRange.from | moment}} - {{timeRange.to | moment}}</span>
@@ -114,9 +114,9 @@ <h2>Searching</h2>
                  style="height: 200px"
                 >
               </visualization>
-            </div>
+            </section>
 
-            <div class="discover-table" fixed-scroll>
+            <section class="discover-table" fixed-scroll aria-label="Documents">
               <doc-table
                 hits="rows"
                 index-pattern="indexPattern"
@@ -143,7 +143,7 @@ <h2>Searching</h2>
                 your search, refine your search to see others.
                 <a kbn-accessible-click ng-click="scrollToTop()">Back to top.</a>
               </div>
-            </div>
+            </section>
           </div>
         </div>
       </div>

diff --git a/src/ui/public/doc_table/__tests__/lib/rows_headers.js b/src/ui/public/doc_table/__tests__/lib/rows_headers.js
@@ -120,6 +120,22 @@ describe('Doc Table', function () {
       columnTests('[data-test-subj~="docTableHeaderField"]', $elem);
     });
 
+    describe('sorting button', function () {
+
+      beforeEach(function () {
+        $parentScope.columns = ['bytes', '_source'];
+        $elem.scope().$digest();
+      });
+
+      it('should show for sortable columns', function () {
+        expect($elem.find(`[data-test-subj="docTableHeaderFieldSort_bytes"]`).length).to.be(1);
+      });
+
+      it('should not be shown for unsortable columns', function () {
+        expect($elem.find(`[data-test-subj="docTableHeaderFieldSort__source"]`).length).to.be(0);
+      });
+    });
+
     describe('cycleSortOrder function', function () {
       it('should exist', function () {
         expect($scope.cycleSortOrder).to.be.a(Function);

diff --git a/src/ui/public/doc_table/components/table_header.html b/src/ui/public/doc_table/components/table_header.html
@@ -25,8 +25,9 @@
     <span class="table-header-name">
       {{name | shortDots}}
       <button
+        data-test-subj="docTableHeaderFieldSort_{{name}}"
         id="docTableHeaderFieldSort{{name}}"
-        tabindex="0"
+        ng-if="isSortableColumn(name)"
         aria-label="{{ getAriaLabelForColumn(name) }}"
         class="docTableHeaderButton"
         ng-class="headerClass(name)"

diff --git a/src/ui/public/doc_table/components/table_header.js b/src/ui/public/doc_table/components/table_header.js
@@ -18,7 +18,7 @@ module.directive('kbnTableHeader', function (shortDotsFilter) {
     },
     template: headerHtml,
     controller: function ($scope) {
-      const isSortableColumn = function isSortableColumn(columnName) {
+      $scope.isSortableColumn = function isSortableColumn(columnName) {
         return (
           !!$scope.indexPattern
           && _.isFunction($scope.onChangeSortOrder)
@@ -27,7 +27,7 @@ module.directive('kbnTableHeader', function (shortDotsFilter) {
       };
 
       $scope.tooltip = function (column) {
-        if (!isSortableColumn(column)) return '';
+        if (!$scope.isSortableColumn(column)) return '';
         return 'Sort by ' + shortDotsFilter(column);
       };
 
@@ -53,7 +53,7 @@ module.directive('kbnTableHeader', function (shortDotsFilter) {
       };
 
       $scope.headerClass = function (column) {
-        if (!isSortableColumn(column)) return;
+        if (!$scope.isSortableColumn(column)) return;
 
         const sortOrder = $scope.sortOrder;
         const defaultClass = ['fa', 'fa-sort-up', 'table-header-sortchange'];
@@ -83,7 +83,7 @@ module.directive('kbnTableHeader', function (shortDotsFilter) {
       };
 
       $scope.cycleSortOrder = function cycleSortOrder(columnName) {
-        if (!isSortableColumn(columnName)) {
+        if (!$scope.isSortableColumn(columnName)) {
           return;
         }
 
@@ -98,7 +98,7 @@ module.directive('kbnTableHeader', function (shortDotsFilter) {
       };
 
       $scope.getAriaLabelForColumn = function getAriaLabelForColumn(name) {
-        if (!isSortableColumn(name)) return null;
+        if (!$scope.isSortableColumn(name)) return null;
 
         const [currentColumnName, currentDirection = 'asc'] = $scope.sortOrder;
         if(name === currentColumnName && currentDirection === 'asc') {

diff --git a/src/ui/public/filter_bar/filter_bar.html b/src/ui/public/filter_bar/filter_bar.html
@@ -1,123 +1,125 @@
-<div class="filter-bar-confirm" ng-show="newFilters.length">
-  <form ng-submit="applyFilters(newFilters)">
-  <ul class="list-unstyled">
-    <li>Apply these filters?</li>
-    <li ng-repeat="filter in newFilters track by $index" class="filter" ng-click="filter.meta.apply = !filter.meta.apply"><input type="checkbox" ng-checked="filter.meta.apply"/> {{ filter.meta.key }}: {{ filter.meta.value }}</li>
-    <li ng-if="changeTimeFilter" class="changeTimeFilter filter" ng-click="changeTimeFilter.meta.apply = !changeTimeFilter.meta.apply"><input type="checkbox" ng-checked="changeTimeFilter.meta.apply"/> <strong>Change time to:</strong> {{changeTimeFilter.meta.value}} </li>
-    <li>
-      <div class="kuiButtonGroup">
-        <button class="kuiButton kuiButton--primary kuiButton--small">
-          Apply Now
-        </button>
+<section aria-label="Filters">
+  <div class="filter-bar-confirm" ng-show="newFilters.length">
+    <form ng-submit="applyFilters(newFilters)">
+    <ul class="list-unstyled">
+      <li>Apply these filters?</li>
+      <li ng-repeat="filter in newFilters track by $index" class="filter" ng-click="filter.meta.apply = !filter.meta.apply"><input type="checkbox" ng-checked="filter.meta.apply"/> {{ filter.meta.key }}: {{ filter.meta.value }}</li>
+      <li ng-if="changeTimeFilter" class="changeTimeFilter filter" ng-click="changeTimeFilter.meta.apply = !changeTimeFilter.meta.apply"><input type="checkbox" ng-checked="changeTimeFilter.meta.apply"/> <strong>Change time to:</strong> {{changeTimeFilter.meta.value}} </li>
+      <li>
+        <div class="kuiButtonGroup">
+          <button class="kuiButton kuiButton--primary kuiButton--small">
+            Apply Now
+          </button>
 
-        <button
-          class="kuiButton kuiButton--hollow"
-          ng-click="clearFilterBar();"
-        >
-          Cancel
-        </button>
-      </div>
-    </li>
-  </ul>
-  </form>
-</div>
-
-<div
-  class="filter-bar"
-  ng-show="filters.length || showAddFilterButton()"
->
-  <filter-pill
-    ng-repeat="filter in filters track by $index"
-    filter="filter"
-    on-toggle-filter="toggleFilter"
-    on-pin-filter="pinFilter"
-    on-invert-filter="invertFilter"
-    on-delete-filter="deleteFilter"
-    on-edit-filter="editFilter"
-  ></filter-pill>
+          <button
+            class="kuiButton kuiButton--hollow"
+            ng-click="clearFilterBar();"
+          >
+            Cancel
+          </button>
+        </div>
+      </li>
+    </ul>
+    </form>
+  </div>
 
   <div
-    class="filter-link"
-    ng-show="showAddFilterButton()"
+    class="filter-bar"
+    ng-show="filters.length || showAddFilterButton()"
   >
-    <div class="filter-description small">
-      <a
-        ng-click="addFilter()"
-        kbn-accessible-click
-      >
-        Add a filter
-        <span class="fa fa-plus"></span>
-      </a>
+    <filter-pill
+      ng-repeat="filter in filters track by $index"
+      filter="filter"
+      on-toggle-filter="toggleFilter"
+      on-pin-filter="pinFilter"
+      on-invert-filter="invertFilter"
+      on-delete-filter="deleteFilter"
+      on-edit-filter="editFilter"
+    ></filter-pill>
+
+    <div
+      class="filter-link"
+      ng-show="showAddFilterButton()"
+    >
+      <div class="filter-description small">
+        <a
+          ng-click="addFilter()"
+          kbn-accessible-click
+        >
+          Add a filter
+          <span class="fa fa-plus"></span>
+        </a>
+      </div>
     </div>
-  </div>
 
-  <div
-    class="filter-link pull-right"
-    ng-show="filters.length"
-  >
-    <div class="filter-description small">
-      <a
-        ng-click="showFilterActions = !showFilterActions"
-        kbn-accessible-click
-        aria-expanded="{{!!showFilterActions}}"
-        aria-controls="filterActionsAllContainer"
-      >
-        Actions
-        <span
-          class="fa"
-          ng-class="{
-            'fa-caret-down': showFilterActions,
-            'fa-caret-right': !showFilterActions
-          }"
-          data-test-subj="showFilterActions"
-        ></span>
-      </a>
+    <div
+      class="filter-link pull-right"
+      ng-show="filters.length"
+    >
+      <div class="filter-description small">
+        <a
+          ng-click="showFilterActions = !showFilterActions"
+          kbn-accessible-click
+          aria-expanded="{{!!showFilterActions}}"
+          aria-controls="filterActionsAllContainer"
+        >
+          Actions
+          <span
+            class="fa"
+            ng-class="{
+              'fa-caret-down': showFilterActions,
+              'fa-caret-right': !showFilterActions
+            }"
+            data-test-subj="showFilterActions"
+          ></span>
+        </a>
+      </div>
+    </div>
+
+    <div
+      class="filter-edit-container"
+      ng-if="editingFilter"
+    >
+      <filter-editor
+        filter="editingFilter"
+        index-patterns="indexPatterns"
+        on-delete="deleteFilter(editingFilter)"
+        on-save="saveEdit(filter, newFilter, isPinned)"
+        on-cancel="cancelEdit()"
+      ></filter-editor>
     </div>
   </div>
 
   <div
-    class="filter-edit-container"
-    ng-if="editingFilter"
+    class="filter-bar filter-bar-condensed"
+    ng-show="filters.length && showFilterActions"
+    id="filterActionsAllContainer"
   >
-    <filter-editor
-      filter="editingFilter"
-      index-patterns="indexPatterns"
-      on-delete="deleteFilter(editingFilter)"
-      on-save="saveEdit(filter, newFilter, isPinned)"
-      on-cancel="cancelEdit()"
-    ></filter-editor>
-  </div>
-</div>
-
-<div
-  class="filter-bar filter-bar-condensed"
-  ng-show="filters.length && showFilterActions"
-  id="filterActionsAllContainer"
->
-  <div class="filter-actions-all">
-    <div class="filter-link">
-      <div class="filter-description"><strong>All filters:</strong></div>
-    </div>
+    <div class="filter-actions-all">
       <div class="filter-link">
-        <div class="filter-description"><a ng-click="toggleAll(false)" kbn-accessible-click>Enable</a></div>
+        <div class="filter-description"><strong>All filters:</strong></div>
+      </div>
+        <div class="filter-link">
+          <div class="filter-description"><a ng-click="toggleAll(false)" kbn-accessible-click>Enable</a></div>
+        </div>
+      <div class="filter-link">
+        <div class="filter-description"><a ng-click="toggleAll(true)" kbn-accessible-click>Disable</a></div>
+      </div>
+      <div class="filter-link">
+        <div class="filter-description"><a ng-click="pinAll(true)" kbn-accessible-click>Pin</a></div>
+      </div>
+      <div class="filter-link">
+        <div class="filter-description"><a ng-click="pinAll(false)" kbn-accessible-click>Unpin</a></div>
+      </div>
+      <div class="filter-link">
+        <div class="filter-description"><a ng-click="invertAll()" kbn-accessible-click>Invert</a></div>
+      </div>
+      <div class="filter-link">
+        <div class="filter-description"><a ng-click="toggleAll()" kbn-accessible-click>Toggle</a></div>
+      </div>
+      <div class="filter-link">
+        <div class="filter-description"><a ng-click="removeAll()" data-test-subj="removeAllFilters" kbn-accessible-click>Remove</a></div>
       </div>
-    <div class="filter-link">
-      <div class="filter-description"><a ng-click="toggleAll(true)" kbn-accessible-click>Disable</a></div>
-    </div>
-    <div class="filter-link">
-      <div class="filter-description"><a ng-click="pinAll(true)" kbn-accessible-click>Pin</a></div>
-    </div>
-    <div class="filter-link">
-      <div class="filter-description"><a ng-click="pinAll(false)" kbn-accessible-click>Unpin</a></div>
-    </div>
-    <div class="filter-link">
-      <div class="filter-description"><a ng-click="invertAll()" kbn-accessible-click>Invert</a></div>
-    </div>
-    <div class="filter-link">
-      <div class="filter-description"><a ng-click="toggleAll()" kbn-accessible-click>Toggle</a></div>
-    </div>
-    <div class="filter-link">
-      <div class="filter-description"><a ng-click="removeAll()" data-test-subj="removeAllFilters" kbn-accessible-click>Remove</a></div>
     </div>
   </div>
-</div>
+</section>