Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Commit

Permalink
fix(radioButton): arrowkey navigation with disabled buttons
Browse files Browse the repository at this point in the history
Closes #1040.
  • Loading branch information
ThomasBurleson committed Dec 23, 2014
1 parent 504c946 commit f520d50
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 4 deletions.
8 changes: 7 additions & 1 deletion src/components/radioButton/radioButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,16 @@ function mdRadioGroupDirective($mdUtil, $mdConstant, $mdTheming) {
);

if (buttons.count()) {
var validate = function (button) {
// If disabled, then NOT valid
return !angular.element(button).attr("disabled");
};
var selected = parent[0].querySelector('md-radio-button.md-checked');
var target = buttons[increment < 0 ? 'previous' : 'next'](selected) || buttons.first();
var target = buttons[increment < 0 ? 'previous' : 'next'](selected, validate) || buttons.first();
// Activate radioButton's click listener (triggerHandler won't create a real click event)
angular.element(target).triggerHandler('click');


}
}

Expand Down
34 changes: 34 additions & 0 deletions src/components/radioButton/radioButton.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,40 @@ describe('radioButton', function() {
expect($rootScope.color).toBe('white');
}));

it('should skip disabled on arrow key', inject(function($compile, $rootScope, $mdConstant) {
var element = $compile(
'<md-radio-group ng-model="color">' +
' <md-radio-button value="red" ></md-radio-button>' +
' <md-radio-button value="white" ng-disabled="isDisabled"></md-radio-button>' +
' <md-radio-button value="blue" ></md-radio-button>' +
'</md-radio-group>'
)($rootScope);
var rbGroupElement = element.eq(0);

$rootScope.$apply('isDisabled = true');
$rootScope.$apply('color = "red"');
expect($rootScope.color).toBe("red");


rightArrow(); expect($rootScope.color).toEqual('blue');
rightArrow(); expect($rootScope.color).toEqual('red');
rightArrow(); expect($rootScope.color).toEqual('blue');


$rootScope.$apply('isDisabled = false');

rightArrow();
rightArrow(); expect($rootScope.color).toEqual('white');
rightArrow(); expect($rootScope.color).toEqual('blue');

function rightArrow() {
rbGroupElement.triggerHandler({
type: 'keydown',
keyCode: $mdConstant.KEY_CODE.RIGHT_ARROW
});
}
}));


it('should allow {{expr}} as value', inject(function($compile, $rootScope) {
$rootScope.some = 11;
Expand Down
26 changes: 26 additions & 0 deletions src/core/util/iterator.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,32 @@ describe('iterator', function() {

});

});

describe('use to provide navigation with validation ', function () {
var list, iter;
var validate = function(item) { return (item !== 14) && (item !== 'Andrew'); };

beforeEach(inject(function ($mdUtil) {
list = [ 13, 14, 'Marcy', 15, 'Andrew', 16, 21, 'Adam', 37, 'Max', 99 ];
iter = $mdUtil.iterator(list);
}));

it('should use next() properly', function () {

expect( iter.next(13, validate) ).toBe('Marcy');
expect( iter.next('Marcy', validate) ).toBe(15);
expect( iter.next(15, validate) ).toBe(16);

});

it('should use previous() properly', function () {

expect( iter.previous(16, validate) ).toBe(15);
expect( iter.previous(15, validate) ).toBe('Marcy');
expect( iter.previous('Marcy', validate) ).toBe(13);

});

});

Expand Down
9 changes: 6 additions & 3 deletions src/core/util/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -329,14 +329,17 @@ angular.module('material.core')
* @param {optional} validate
* @returns {*}
*/
function next(item, validate) {
function next(item, validate, limit) {

This comment has been minimized.

Copy link
@gkalpak

gkalpak Dec 23, 2014

Member

Didn't really looked into what this does exactly, but this function seems to be in need of a generous refactoring :)

This comment has been minimized.

Copy link
@ThomasBurleson

ThomasBurleson Dec 23, 2014

Author Contributor

@gkalpak - agreed that it is a bit messy. You are welcome to present a better refactoring.

This comment has been minimized.

Copy link
@ThomasBurleson

ThomasBurleson Dec 23, 2014

Author Contributor

@gkalpak - Actually just noticed that the function previous() is missing the recurse limit parameter.
Commit SHA ff79c91 fixes this.

validate = validate || trueFn;

if (contains(item)) {
var index = indexOf(item) + 1,
found = inRange(index) ? _items[ index ] : (reloop ? first() : null);
found = inRange(index) ? _items[ index ] : (reloop ? first() : null);

return validate(found) ? found : next(found, validate);
// Infinite loop check...
if ( reloop && (indexOf(found) === limit) ) return null;

return validate(found) ? found : next(found, validate, limit || index);
}

return null;
Expand Down

0 comments on commit f520d50

Please sign in to comment.