fix: 🐛 improve accessibility in the grid menus and selection

add more keyboard handling and better announcements for checkboxes and menus
angular-ui · Jun 12, 2021 · e5ae7c0 · e5ae7c0
1 parent bb28b2f
commit e5ae7c0
Show file tree

Hide file tree

Showing 18 changed files with 140 additions and 89 deletions.
diff --git a/packages/cellnav/src/js/cellnav.js b/packages/cellnav/src/js/cellnav.js
@@ -830,15 +830,16 @@
                   }
 
                   function getCellDisplayValue(currentRowColumn) {
+                    var prefix = '';
+
                     if (currentRowColumn.col.field === 'selectionRowHeaderCol') {
                       // This is the case when the 'selection' feature is used in the grid and the user has moved
                       // to or inside of the left grid container which holds the checkboxes for selecting rows.
                       // This is necessary for Accessibility. Without this a screen reader cannot determine if the row
                       // is or is not currently selected.
-                        return currentRowColumn.row.isSelected ? i18nService.getSafeText('search.aria.selected') : i18nService.getSafeText('search.aria.notSelected');
-                      } else {
-                        return grid.getCellDisplayValue(currentRowColumn.row, currentRowColumn.col);
+                        prefix = (currentRowColumn.row.isSelected ? i18nService.getSafeText('search.aria.selected') : i18nService.getSafeText('search.aria.notSelected')) + ', ';
                       }
+                      return prefix + grid.getCellDisplayValue(currentRowColumn.row, currentRowColumn.col);
                     }
 
                   var values = [];
@@ -847,9 +848,7 @@
                     var cellDisplayValue = getCellDisplayValue(currentSelection[i]) + getAppendedColumnHeaderText(currentSelection[i].col);
                     values.push(cellDisplayValue);
                   }
-                  var cellText = values.toString();
-                  setNotifyText(cellText);
-
+                  setNotifyText(values.toString());
                 });
               }
               // Only add the ngAria stuff it will be used

diff --git a/packages/core/src/js/directives/ui-grid-column-menu.js b/packages/core/src/js/directives/ui-grid-column-menu.js
@@ -310,6 +310,11 @@ function ($timeout, gridUtil, uiGridConstants, uiGridColumnMenuService, $documen
       $scope.menuItems = $scope.defaultMenuItems;
       uiGridColumnMenuService.setColMenuItemWatch( $scope );
 
+      function updateCurrentColStatus(menuShown) {
+        if ($scope.col) {
+          $scope.col.menuShown = menuShown;
+        }
+      }
 
       /**
        * @ngdoc method
@@ -324,8 +329,11 @@ function ($timeout, gridUtil, uiGridConstants, uiGridColumnMenuService, $documen
        * @param {element} $columnElement the column element we want to position below
        */
       $scope.showMenu = function(column, $columnElement, event) {
+        // Update the menu status for the current column
+        updateCurrentColStatus(false);
         // Swap to this column
         $scope.col = column;
+        updateCurrentColStatus(true);
 
         // Get the position information for the column element
         var colElementPosition = uiGridColumnMenuService.getColumnElementPosition( $scope, column, $columnElement );
@@ -359,6 +367,7 @@ function ($timeout, gridUtil, uiGridConstants, uiGridColumnMenuService, $documen
        */
       $scope.hideMenu = function( broadcastTrigger ) {
         $scope.menuShown = false;
+        updateCurrentColStatus(false);
         if ( !broadcastTrigger ) {
           $scope.$broadcast('hide-menu');
         }

diff --git a/packages/core/src/js/directives/ui-grid-header-cell.js b/packages/core/src/js/directives/ui-grid-header-cell.js
@@ -69,6 +69,7 @@
 
             // Hide the menu by default
             $scope.menuShown = false;
+            $scope.col.menuShown = false;
 
             // Put asc and desc sort directions in scope
             $scope.asc = uiGridConstants.ASC;
@@ -347,44 +348,30 @@
 
             $scope.handleClick = function(event) {
               // If the shift key is being held down, add this column to the sort
-              var add = false;
-              if (event.shiftKey) {
-                add = true;
-              }
-
               // Sort this column then rebuild the grid's rows
-              uiGridCtrl.grid.sortColumn($scope.col, add)
+              uiGridCtrl.grid.sortColumn($scope.col, event.shiftKey)
                 .then(function () {
                   if (uiGridCtrl.columnMenuScope) { uiGridCtrl.columnMenuScope.hideMenu(); }
                   uiGridCtrl.grid.refresh();
                 }).catch(angular.noop);
             };
 
             $scope.headerCellArrowKeyDown = function(event) {
-              if (event.keyCode === 32 || event.keyCode === 13) {
+              if (event.keyCode === uiGridConstants.keymap.SPACE || event.keyCode === uiGridConstants.keymap.ENTER) {
                 event.preventDefault();
                 $scope.toggleMenu(event);
               }
             };
 
             $scope.toggleMenu = function(event) {
-
               event.stopPropagation();
 
-              // If the menu is already showing...
-              if (uiGridCtrl.columnMenuScope.menuShown) {
-                // ... and we're the column the menu is on...
-                if (uiGridCtrl.columnMenuScope.col === $scope.col) {
-                  // ... hide it
-                  uiGridCtrl.columnMenuScope.hideMenu();
-                }
-                // ... and we're NOT the column the menu is on
-                else {
-                  // ... move the menu to our column
-                  uiGridCtrl.columnMenuScope.showMenu($scope.col, $elm);
-                }
+              // If the menu is already showing and we're the column the menu is on
+              if (uiGridCtrl.columnMenuScope.menuShown && uiGridCtrl.columnMenuScope.col === $scope.col) {
+                // ... hide it
+                uiGridCtrl.columnMenuScope.hideMenu();
               }
-              // If the menu is NOT showing
+              // If the menu is NOT showing or is showing in a different column
               else {
                 // ... show it on our column
                 uiGridCtrl.columnMenuScope.showMenu($scope.col, $elm);

diff --git a/packages/core/src/js/directives/ui-grid-menu-button.js b/packages/core/src/js/directives/ui-grid-menu-button.js
@@ -376,6 +376,16 @@ function (gridUtil, uiGridConstants, uiGridGridMenuService, i18nService) {
 
       $scope.shown = false;
 
+      $scope.toggleOnKeydown = function(event) {
+        if (
+          event.keyCode === uiGridConstants.keymap.ENTER ||
+          event.keyCode === uiGridConstants.keymap.SPACE ||
+          (event.keyCode === uiGridConstants.keymap.ESC && $scope.shown)
+        ) {
+          $scope.toggleMenu();
+        }
+      };
+
       $scope.toggleMenu = function () {
         if ( $scope.shown ) {
           $scope.$broadcast('hide-menu');

diff --git a/packages/core/src/js/directives/ui-grid-menu.js b/packages/core/src/js/directives/ui-grid-menu.js
@@ -112,14 +112,10 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18
 
         // Turn off an existing document click handler
         angular.element(document).off('click touchstart', applyHideMenu);
-        $elm.off('keyup', checkKeyUp);
-        $elm.off('keydown', checkKeyDown);
 
         // Turn on the document click handler, but in a timeout so it doesn't apply to THIS click if there is one
         $timeout(function() {
           angular.element(document).on(docEventType, applyHideMenu);
-          $elm.on('keyup', checkKeyUp);
-          $elm.on('keydown', checkKeyDown);
         });
       };
 
@@ -144,8 +140,6 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18
         }
 
         angular.element(document).off('click touchstart', applyHideMenu);
-        $elm.off('keyup', checkKeyUp);
-        $elm.off('keydown', checkKeyDown);
       };
 
       $scope.$on('hide-menu', function (event, args) {
@@ -173,28 +167,22 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18
         }
       };
 
-      // close menu on ESC and keep tab cyclical
-      var checkKeyUp = function(event) {
-        if (event.keyCode === 27) {
-          $scope.hideMenu();
-        }
-      };
-
-      var checkKeyDown = function(event) {
+      $scope.checkKeyDown = function(event) {
         var setFocus = function(elm) {
           elm.focus();
           event.preventDefault();
-          return false;
         };
-        if (event.keyCode === 9) {
+        if (event.keyCode === uiGridConstants.keymap.ESC) {
+          $scope.hideMenu();
+        } else if (event.keyCode === uiGridConstants.keymap.TAB) {
           var firstMenuItem, lastMenuItem;
           var menuItemButtons = $elm[0].querySelectorAll('button:not(.ng-hide)');
           if (menuItemButtons.length > 0) {
             firstMenuItem = menuItemButtons[0];
             lastMenuItem = menuItemButtons[menuItemButtons.length - 1];
-            if (event.target === lastMenuItem && !event.shiftKey) {
+            if (event.target.parentElement.id === lastMenuItem.parentElement.id && !event.shiftKey) {
               setFocus(firstMenuItem);
-            } else if (event.target === firstMenuItem && event.shiftKey) {
+            } else if (event.target.parentElement.id === firstMenuItem.parentElement.id && event.shiftKey) {
               setFocus(lastMenuItem);
             }
           }
@@ -212,8 +200,6 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18
       $scope.$on('$destroy', function unbindEvents() {
         angular.element($window).off('resize', applyHideMenu);
         angular.element(document).off('click touchstart', applyHideMenu);
-        $elm.off('keyup', checkKeyUp);
-        $elm.off('keydown', checkKeyDown);
       });
 
       if (uiGridCtrl) {

diff --git a/packages/core/src/js/en.js b/packages/core/src/js/en.js
@@ -37,7 +37,11 @@
           last: 'Last Page'
         },
         selection: {
-          selectAll: 'Select All'
+          aria: {
+            row: 'Row'
+          },
+          selectAll: 'Select All',
+          displayName: 'Row Selection Checkbox'
         },
         menu: {
           text: 'Choose Columns:'

diff --git a/packages/core/src/js/factories/GridRow.js b/packages/core/src/js/factories/GridRow.js
@@ -31,6 +31,14 @@ angular.module('ui.grid')
       */
     this.entity = entity;
 
+    /**
+      *  @ngdoc object
+      *  @name index
+      *  @propertyOf  ui.grid.class:GridRow
+      *  @description The current position of the row in the array
+      */
+     this.index = index;
+
      /**
       *  @ngdoc object
       *  @name uid

diff --git a/packages/core/src/templates/ui-grid/ui-grid-menu-button.html b/packages/core/src/templates/ui-grid/ui-grid-menu-button.html
@@ -1,8 +1,12 @@
 <div class="ui-grid-menu-button">
   <div role='button'
        ui-grid-one-bind-id-grid="'grid-menu'"
+       ui-grid-one-bind-aria-label="i18n.aria.buttonLabel"
+       tabindex="0"
        class="ui-grid-icon-container"
        ng-click="toggleMenu()"
+       ng-keydown="toggleOnKeydown($event)"
+       aria-expanded="{{shown}}"
        aria-haspopup="true">
     <i class="ui-grid-icon-menu"
        ui-grid-one-bind-aria-label="i18n.aria.buttonLabel">&nbsp;</i>

diff --git a/packages/core/src/templates/ui-grid/uiGridHeaderCell.html b/packages/core/src/templates/ui-grid/uiGridHeaderCell.html
@@ -38,6 +38,7 @@
     ng-click="toggleMenu($event)"
     ng-keydown="headerCellArrowKeyDown($event)"
     ui-grid-one-bind-aria-label="i18n.headerCell.aria.columnMenuButtonLabel"
+    aria-expanded="{{col.menuShown}}"
     aria-haspopup="true">
     <i
       class="ui-grid-icon-angle-down"

diff --git a/packages/core/src/templates/ui-grid/uiGridMenu.html b/packages/core/src/templates/ui-grid/uiGridMenu.html
@@ -1,5 +1,6 @@
 <div
   class="ui-grid-menu"
+  ng-keydown="checkKeyDown($event)"
   ng-show="shown">
   <style ui-grid-style>
     {{dynamicStyles}}

diff --git a/packages/core/test/core/directives/ui-grid-column-menu.spec.js b/packages/core/test/core/directives/ui-grid-column-menu.spec.js
@@ -579,6 +579,10 @@ describe('ui-grid-column-menu uiGridColumnMenuService', function() {
         $scope.$destroy();
         uiGrid.remove();
       });
+      it('should update the extended state of the relate menu button', function() {
+        expect($('.ui-grid-column-menu-button').first().attr('aria-expanded')).toEqual('true');
+      });
+
       it('should raise the sortChanged event when unsort is clicked', function() {
         $($('.ui-grid-menu-item')[2]).click();
         $timeout.flush();

diff --git a/packages/core/test/core/directives/ui-grid-header-cell.spec.js b/packages/core/test/core/directives/ui-grid-header-cell.spec.js
@@ -1,8 +1,8 @@
 describe('uiGridHeaderCell', function() {
 	'use strict';
 
-	var grid, $scope, $compile, $document, $q, $timeout, $window, recompile, $animate, uiGridConstants, gridUtil, $httpBackend,
-		downEvent, upEvent, clickEvent,
+	var grid, $scope, $compile, $document, $timeout, recompile, uiGridConstants, gridUtil, $httpBackend,
+		downEvent, clickEvent,
 		data = [
 			{name: 'Ethel Price', gender: 'female', company: 'Enersol'},
 			{name: 'Claudine Neal', gender: 'female', company: 'Sealoud'},
@@ -26,14 +26,11 @@ describe('uiGridHeaderCell', function() {
 	beforeEach(function() {
 		module('ui.grid');
 
-		inject(function(_$compile_, $rootScope, _$document_, _$q_, _$timeout_, _$window_, _$animate_, _uiGridConstants_, _gridUtil_, _$httpBackend_) {
+		inject(function(_$compile_, $rootScope, _$document_, _$timeout_, _uiGridConstants_, _gridUtil_, _$httpBackend_) {
 			$scope = $rootScope;
 			$compile = _$compile_;
 			$document = _$document_;
-			$q = _$q_;
 			$timeout = _$timeout_;
-			$window = _$window_;
-			$animate = _$animate_;
 			uiGridConstants = _uiGridConstants_;
 			gridUtil = _gridUtil_;
 			$httpBackend = _$httpBackend_;
@@ -42,11 +39,9 @@ describe('uiGridHeaderCell', function() {
 		// Decide whether to use mouse or touch events based on which capabilities the browser has
 		if (gridUtil.isTouchEnabled()) {
 			downEvent = 'touchstart';
-			upEvent = 'touchend';
 			clickEvent = 'touchstart';
 		} else {
 			downEvent = 'mousedown';
-			upEvent = 'mouseup';
 			clickEvent = 'click';
 		}
 

diff --git a/packages/core/test/core/directives/ui-grid-menu-button.spec.js b/packages/core/test/core/directives/ui-grid-menu-button.spec.js
@@ -456,5 +456,39 @@ describe('ui-grid-menu-button uiGridGridMenuService', function() {
 			$scope.$broadcast('menu-hidden');
 			expect(gridUtil.focus.bySelector).toHaveBeenCalledWith(jasmine.any(Object), '.ui-grid-icon-container');
 		});
+		describe('toggleOnKeydown method', function() {
+			var event;
+
+			beforeEach(function() {
+				event = $.Event('keydown');
+				$scope.$broadcast('hide-menu');
+				spyOn(uiGridGridMenuService, 'getMenuItems').and.callThrough();
+			});
+			afterEach(function() {
+				uiGridGridMenuService.getMenuItems.calls.reset();
+			});
+			it('should toggle the menu on ENTER', function() {
+				event.keyCode = 13;
+				element.find('.ui-grid-icon-container').trigger(event);
+				expect(uiGridGridMenuService.getMenuItems).toHaveBeenCalled();
+			});
+			it('should toggle the menu on SPACE', function() {
+				event.keyCode = 32;
+				element.find('.ui-grid-icon-container').trigger(event);
+				expect(uiGridGridMenuService.getMenuItems).toHaveBeenCalled();
+			});
+			it('should not try to toggle the menu on ESC if it is closed', function() {
+				event.keyCode = 27;
+				element.find('.ui-grid-icon-container').trigger(event);
+				expect(uiGridGridMenuService.getMenuItems).not.toHaveBeenCalled();
+			});
+			it('should hide the menu on ESC if the menu is open', function() {
+				element.find('.ui-grid-icon-container').trigger('click');
+				uiGridGridMenuService.getMenuItems.calls.reset();
+				event.keyCode = 27;
+				element.find('.ui-grid-icon-container').trigger(event);
+				expect(uiGridGridMenuService.getMenuItems).not.toHaveBeenCalled();
+			});
+		});
 	});
 });