Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Commit

Permalink
fix(select): disabled state, invalid html in unit tests
Browse files Browse the repository at this point in the history 
Fixes:
* Disabled `mdSelect` elements being marked as `touched` when they get blurred.
* Click and key events being bound to elements with empty `disabled` attributes.
* Invalid, comma-separated attributes in the `mdSelect` unit tests.

Cheers to @montgomery1944 for the tip about removing the tabindex.
Closes #7773;

Closes #7778
  • Loading branch information
@crisbeto @ThomasBurleson
crisbeto authored and ThomasBurleson committed Apr 1, 2016
1 parent 5eab71b commit d2c29b5
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 23 deletions.
26 changes: 15 additions & 11 deletions src/components/select/select.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,9 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
element.empty().append(valueEl);
element.append(selectTemplate);

attr.tabindex = attr.tabindex || '0';
if(!attr.tabindex){
attr.$set('tabindex', 0);
}

return function postLink(scope, element, attr, ctrls) {
var untouched = true;
Expand Down Expand Up @@ -372,23 +374,25 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
}
isDisabled = disabled;
if (disabled) {
element.attr({'tabindex': -1, 'aria-disabled': 'true'});
element.off('click', openSelect);
element.off('keydown', handleKeypress);
element
.attr({'aria-disabled': 'true'})
.removeAttr('tabindex')
.off('click', openSelect)
.off('keydown', handleKeypress);
} else {
element.attr({'tabindex': attr.tabindex, 'aria-disabled': 'false'});
element.on('click', openSelect);
element.on('keydown', handleKeypress);
element
.attr({'tabindex': attr.tabindex, 'aria-disabled': 'false'})
.on('click', openSelect)
.on('keydown', handleKeypress);
}
});

if (!attr.disabled && !attr.ngDisabled) {
element.attr({'tabindex': attr.tabindex, 'aria-disabled': 'false'});
if (!attr.hasOwnProperty('disabled') && !attr.hasOwnProperty('ngDisabled')) {
element.attr({'aria-disabled': 'false'});
element.on('click', openSelect);
element.on('keydown', handleKeypress);
}


var ariaAttrs = {
role: 'listbox',
'aria-expanded': 'false',
Expand Down Expand Up @@ -1220,7 +1224,7 @@ function SelectProvider($$interimElementProvider) {
}
newOption = optionsArray[index];
if (newOption.hasAttribute('disabled')) newOption = undefined;
} while (!newOption && index < optionsArray.length - 1 && index > 0)
} while (!newOption && index < optionsArray.length - 1 && index > 0);
newOption && newOption.focus();
opts.focusedNode = newOption;
}
Expand Down
33 changes: 21 additions & 12 deletions src/components/select/select.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,30 @@ describe('<md-select>', function() {
}));

it('should preserve tabindex', function() {
var select = setupSelect('tabindex="2", ng-model="val"').find('md-select');
var select = setupSelect('tabindex="2" ng-model="val"').find('md-select');
expect(select.attr('tabindex')).toBe('2');
});

it('should set a tabindex if the element does not have one', function() {
var select = setupSelect('ng-model="val"').find('md-select');
expect(select.attr('tabindex')).toBeDefined();
});

it('supports non-disabled state', function() {
var select = setupSelect('ng-model="val"').find('md-select');
expect(select.attr('aria-disabled')).toBe('false');
});

it('supports disabled state', inject(function($document) {
var select = setupSelect('disabled="disabled", ng-model="val"').find('md-select');
var select = setupSelect('disabled ng-model="val"').find('md-select');
openSelect(select);
expectSelectClosed(select);
expect($document.find('md-select-menu').length).toBe(0);
expect(select.attr('aria-disabled')).toBe('true');
}));

it('supports passing classes to the container', inject(function($document) {
var select = setupSelect('ng-model="val", md-container-class="test"').find('md-select');
var select = setupSelect('ng-model="val" md-container-class="test"').find('md-select');
openSelect(select);

var container = $document[0].querySelector('.md-select-menu-container');
Expand All @@ -58,7 +63,7 @@ describe('<md-select>', function() {
}));

it('supports passing classes to the container using `data-` attribute prefix', inject(function($document) {
var select = setupSelect('ng-model="val", data-md-container-class="test"').find('md-select');
var select = setupSelect('ng-model="val" data-md-container-class="test"').find('md-select');
openSelect(select);

var container = $document[0].querySelector('.md-select-menu-container');
Expand All @@ -67,7 +72,7 @@ describe('<md-select>', function() {
}));

it('supports passing classes to the container using `x-` attribute prefix', inject(function($document) {
var select = setupSelect('ng-model="val", x-md-container-class="test"').find('md-select');
var select = setupSelect('ng-model="val" x-md-container-class="test"').find('md-select');
openSelect(select);

var container = $document[0].querySelector('.md-select-menu-container');
Expand All @@ -88,7 +93,7 @@ describe('<md-select>', function() {
$rootScope.onClose = function() {
called = true;
};
var select = setupSelect('ng-model="val", md-on-close="onClose()"', [1, 2, 3]).find('md-select');
var select = setupSelect('ng-model="val" md-on-close="onClose()"', [1, 2, 3]).find('md-select');
openSelect(select);
expectSelectOpen(select);

Expand Down Expand Up @@ -153,7 +158,6 @@ describe('<md-select>', function() {
expect($rootScope.myForm.select.$touched).toBe(true);
}));


it('restores focus to select when the menu is closed', inject(function($document) {
var select = setupSelect('ng-model="val"').find('md-select');
openSelect(select);
Expand Down Expand Up @@ -186,6 +190,11 @@ describe('<md-select>', function() {

}));

it('should remove the tabindex from a disabled element', inject(function($document) {
var select = setupSelect('ng-model="val" disabled tabindex="1"').find('md-select');
expect(select.attr('tabindex')).toBeUndefined();
}));

describe('input container', function() {
it('should set has-value class on container for non-ng-model input', inject(function($rootScope) {
var el = setupSelect('ng-model="$root.model"', [1, 2, 3]);
Expand All @@ -210,7 +219,7 @@ describe('<md-select>', function() {
}));

it('should match label to given input id', function() {
var el = setupSelect('ng-model="$root.value", id="foo"');
var el = setupSelect('ng-model="$root.value" id="foo"');
expect(el.find('label').attr('for')).toBe('foo');
expect(el.find('md-select').attr('id')).toBe('foo');
});
Expand All @@ -224,7 +233,7 @@ describe('<md-select>', function() {

describe('label behavior', function() {
it('defaults to the placeholder text', function() {
var select = setupSelect('ng-model="someVal", placeholder="Hello world"', null, true).find('md-select');
var select = setupSelect('ng-model="someVal" placeholder="Hello world"', null, true).find('md-select');
var label = select.find('md-select-value');
expect(label.text()).toBe('Hello world');
expect(label.hasClass('md-select-placeholder')).toBe(true);
Expand Down Expand Up @@ -444,7 +453,7 @@ describe('<md-select>', function() {
changesCalled = true;
};

var selectEl = setupSelect('ng-model="myModel", ng-change="changed()"', [1, 2, 3]).find('md-select');
var selectEl = setupSelect('ng-model="myModel" ng-change="changed()"', [1, 2, 3]).find('md-select');
openSelect(selectEl);

var menuEl = $document.find('md-select-menu');
Expand Down Expand Up @@ -763,7 +772,7 @@ describe('<md-select>', function() {
}));

it('adds an aria-label from placeholder', function() {
var select = setupSelect('ng-model="someVal", placeholder="Hello world"', null, true).find('md-select');
var select = setupSelect('ng-model="someVal" placeholder="Hello world"', null, true).find('md-select');
expect(select.attr('aria-label')).toBe('Hello world');
});

Expand All @@ -783,7 +792,7 @@ describe('<md-select>', function() {
}));

it('preserves existing aria-label', function() {
var select = setupSelect('ng-model="someVal", aria-label="Hello world", placeholder="Pick"').find('md-select');
var select = setupSelect('ng-model="someVal" aria-label="Hello world" placeholder="Pick"').find('md-select');
expect(select.attr('aria-label')).toBe('Hello world');
});

Expand Down

0 comments on commit d2c29b5

Please sign in to comment.