Sorting #312
Sorting #312
Conversation
| <th scope="col" class="county_code">County Code</th> | ||
| <th scope="col" class="census_tract">Census Tract</th> | ||
| <th scope="col" class="recommended col-group-first">MSA/MD</th> | ||
| <th scope="col" role="columnheader" class="id" ng-class="{'cf-icon icon-up': sortedBy === 'properties[\'LAR number\']' && !reverse, 'cf-icon icon-down': sortedBy === 'properties[\'LAR number\']' && reverse}" ng-click="sort('properties[\'LAR number\']')" ng-attr-aria-sort="{{isSortedBy('properties[\'LAR number\']')}}">LAR Number</th> |
There was a problem hiding this comment.
You don't need to use role="columnheader" here because we are already conveying the same information by using <th scope="col".
ref: http://www.w3.org/TR/wai-aria/roles#columnheader
Can we move the logic for setting the class to the pagination controller?
Why do we need to use ng-attr-aria-sort instead of just the aria-sort attribute?
There was a problem hiding this comment.
According to http://stackoverflow.com/questions/24863531/using-aria-sort-in-validated-html5 and possibly http://www.w3.org/TR/wai-aria/states_and_properties#aria-sort but it's harder for me to interpret, role="columnheader" is needed despite scope="col".
I can move it to the controller but wasn't sure how much of an advantage it would gain.
Guess I didn't need the ng-attr-, whoops.
There was a problem hiding this comment.
Thanks for the SO article about the use of columnheader. I agree though with the comment on the approved answer that the implementation is broken and table's should be considered data tables by default. But that's a different fight.
In the meantime, it does mean that you need to add a role="grid" attr to your table element and a role="row" to the tr's in your table header.
There was a problem hiding this comment.
Added additional roles in 9cd2976
| @@ -26,23 +28,48 @@ module.exports = /*@ngInject*/ function ($scope, $routeParams, $location, $http, | |||
| $scope.error = {}; | |||
| } | |||
|
|
|||
| $scope.sort = function(property) { | |||
| $scope.reverse = $scope.sortedBy === property ? !$scope.reverse : false; | |||
There was a problem hiding this comment.
$scope.reverse seems like an odd variable name. What is this actually tracking?
There was a problem hiding this comment.
I named it reverse because that's what Angular calls it in their orderBy filter. I can rename it to something like sortAsc if you think that makes more sense.
Alternatively, I can group sortedBy and reverse in an object, and possibly rename sortedBy to orderBy.
There was a problem hiding this comment.
Ahh...so reverse is supposed to indicate the direction of the sort by inferring that it is either in a reverse sort or not. That's backwards. :(
If that's the case, then I think sortAsc makes the most sense.
For #259.