Skip to content

Commit

Permalink
fix(ui-grid-menu-button.js): Change "Columns:" item to a heading.
Browse files Browse the repository at this point in the history
Fix accessibility of "Columns:" menu item by making it a heading instead of a button since it has no
action.
  • Loading branch information
Portugal, Marcelo authored and mportuga committed Apr 30, 2018
1 parent 7143c0b commit 85ad462
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 7 deletions.
3 changes: 2 additions & 1 deletion src/js/core/directives/ui-grid-menu-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,8 @@ angular.module('ui.grid')
// add header for columns
showHideColumns.push({
title: i18nService.getSafeText('gridMenu.columns'),
order: 300
order: 300,
templateUrl: 'ui-grid/ui-grid-menu-header-item'
});

$scope.grid.options.gridMenuTitleFilter = $scope.grid.options.gridMenuTitleFilter ? $scope.grid.options.gridMenuTitleFilter : function( title ) { return title; };
Expand Down
4 changes: 1 addition & 3 deletions src/js/core/directives/ui-grid-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18
}])

.directive('uiGridMenuItem', ['gridUtil', '$compile', 'i18nService', function (gridUtil, $compile, i18nService) {
var uiGridMenuItem = {
return {
priority: 0,
scope: {
name: '=',
Expand Down Expand Up @@ -321,8 +321,6 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18
};
}
};

return uiGridMenuItem;
}]);

})();
10 changes: 7 additions & 3 deletions src/less/menu.less
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,18 @@
list-style-type: none;

li {
padding: 0px;
button {
padding: 0;
.ui-grid-menu-item {
color: @menuTextColor;
min-width: 100%;
padding: 8px;
text-align: left;
background: transparent;
border: none;
cursor: default;
}
button.ui-grid-menu-item {
cursor: pointer;

// Show a shadow when hovering over a menu item
&:hover,
Expand All @@ -78,7 +82,7 @@
}

// Show a bottom border on all but the last menu item
li:not(:last-child) > button {
li:not(:last-child) > .ui-grid-menu-item {
border-bottom: @gridBorderWidth solid @borderColor;
}
}
12 changes: 12 additions & 0 deletions src/templates/ui-grid/ui-grid-menu-header-item.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<li role="menuitem">
<div
class="ui-grid-menu-item"
role="heading"
aria-level="2"
ng-show="itemShown()">
<i aria-hidden='true'>
&nbsp;
</i>
<span ng-bind="label()"></span>
</div>
</li>
3 changes: 3 additions & 0 deletions test/unit/core/directives/ui-grid-menu-button.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ describe('ui-grid-menu-button uiGridGridMenuService', function() {

expect(menuItems[4].title).toEqual('Clear all filters', 'Menu item 4 Clear all filters');
expect(menuItems[5].title).toEqual('Columns:', 'Menu item 5 should be header');
expect(menuItems[5].templateUrl).toEqual('ui-grid/ui-grid-menu-header-item');
expect(menuItems[6].title.toLowerCase()).toEqual('fn_col1', 'Menu item 5 should be col1');
expect(menuItems[7].title.toLowerCase()).toEqual('fn_col3', 'Menu item 6 should be col3');
expect(menuItems[8].title.toLowerCase()).toEqual('fn_col4', 'Menu item 7 should be col4');
Expand All @@ -243,6 +244,7 @@ describe('ui-grid-menu-button uiGridGridMenuService', function() {
expect(menuItems.length).toEqual(6, 'Should be 6 items, 1 columns header, 4 columns that allow hiding and clear all filters');
expect(menuItems[0].title).toEqual('Clear all filters', 'Menu item 0 should be Clear all filters');
expect(menuItems[1].title).toEqual('Columns:', 'Menu item 1 should be header');
expect(menuItems[1].templateUrl).toEqual('ui-grid/ui-grid-menu-header-item');
expect(menuItems[2].title).toEqual('', 'Promise not resolved');
expect(menuItems[3].title).toEqual('', 'Promise not resolved');
expect(menuItems[4].title).toEqual('', 'Promise not resolved');
Expand All @@ -256,6 +258,7 @@ describe('ui-grid-menu-button uiGridGridMenuService', function() {
expect(menuItems.length).toEqual(6, 'Should be 10 items, 1 columns header, 4 columns that allow hiding and Clean all filters');
expect(menuItems[0].title).toEqual('Clear all filters', 'Menu item 0 should be Clear all filters');
expect(menuItems[1].title).toEqual('Columns:', 'Menu item 0 should be header');
expect(menuItems[1].templateUrl).toEqual('ui-grid/ui-grid-menu-header-item');
expect(menuItems[2].title).toEqual('resolve_0', 'Promise now resolved');
expect(menuItems[3].title).toEqual('resolve_1', 'Promise now resolved');
expect(menuItems[4].title).toEqual('resolve_2', 'Promise now resolved');
Expand Down

0 comments on commit 85ad462

Please sign in to comment.