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

Commit d2c29b5

Browse files
crisbetoThomasBurleson
authored andcommitted
fix(select): disabled state, invalid html in unit tests
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
1 parent 5eab71b commit d2c29b5

File tree

2 files changed

+36
-23
lines changed

2 files changed

+36
-23
lines changed

src/components/select/select.js

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,9 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
184184
element.empty().append(valueEl);
185185
element.append(selectTemplate);
186186

187-
attr.tabindex = attr.tabindex || '0';
187+
if(!attr.tabindex){
188+
attr.$set('tabindex', 0);
189+
}
188190

189191
return function postLink(scope, element, attr, ctrls) {
190192
var untouched = true;
@@ -372,23 +374,25 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
372374
}
373375
isDisabled = disabled;
374376
if (disabled) {
375-
element.attr({'tabindex': -1, 'aria-disabled': 'true'});
376-
element.off('click', openSelect);
377-
element.off('keydown', handleKeypress);
377+
element
378+
.attr({'aria-disabled': 'true'})
379+
.removeAttr('tabindex')
380+
.off('click', openSelect)
381+
.off('keydown', handleKeypress);
378382
} else {
379-
element.attr({'tabindex': attr.tabindex, 'aria-disabled': 'false'});
380-
element.on('click', openSelect);
381-
element.on('keydown', handleKeypress);
383+
element
384+
.attr({'tabindex': attr.tabindex, 'aria-disabled': 'false'})
385+
.on('click', openSelect)
386+
.on('keydown', handleKeypress);
382387
}
383388
});
384389

385-
if (!attr.disabled && !attr.ngDisabled) {
386-
element.attr({'tabindex': attr.tabindex, 'aria-disabled': 'false'});
390+
if (!attr.hasOwnProperty('disabled') && !attr.hasOwnProperty('ngDisabled')) {
391+
element.attr({'aria-disabled': 'false'});
387392
element.on('click', openSelect);
388393
element.on('keydown', handleKeypress);
389394
}
390395

391-
392396
var ariaAttrs = {
393397
role: 'listbox',
394398
'aria-expanded': 'false',
@@ -1220,7 +1224,7 @@ function SelectProvider($$interimElementProvider) {
12201224
}
12211225
newOption = optionsArray[index];
12221226
if (newOption.hasAttribute('disabled')) newOption = undefined;
1223-
} while (!newOption && index < optionsArray.length - 1 && index > 0)
1227+
} while (!newOption && index < optionsArray.length - 1 && index > 0);
12241228
newOption && newOption.focus();
12251229
opts.focusedNode = newOption;
12261230
}

src/components/select/select.spec.js

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,25 +31,30 @@ describe('<md-select>', function() {
3131
}));
3232

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

38+
it('should set a tabindex if the element does not have one', function() {
39+
var select = setupSelect('ng-model="val"').find('md-select');
40+
expect(select.attr('tabindex')).toBeDefined();
41+
});
42+
3843
it('supports non-disabled state', function() {
3944
var select = setupSelect('ng-model="val"').find('md-select');
4045
expect(select.attr('aria-disabled')).toBe('false');
4146
});
4247

4348
it('supports disabled state', inject(function($document) {
44-
var select = setupSelect('disabled="disabled", ng-model="val"').find('md-select');
49+
var select = setupSelect('disabled ng-model="val"').find('md-select');
4550
openSelect(select);
4651
expectSelectClosed(select);
4752
expect($document.find('md-select-menu').length).toBe(0);
4853
expect(select.attr('aria-disabled')).toBe('true');
4954
}));
5055

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

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

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

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

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

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

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

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

187191
}));
188192

193+
it('should remove the tabindex from a disabled element', inject(function($document) {
194+
var select = setupSelect('ng-model="val" disabled tabindex="1"').find('md-select');
195+
expect(select.attr('tabindex')).toBeUndefined();
196+
}));
197+
189198
describe('input container', function() {
190199
it('should set has-value class on container for non-ng-model input', inject(function($rootScope) {
191200
var el = setupSelect('ng-model="$root.model"', [1, 2, 3]);
@@ -210,7 +219,7 @@ describe('<md-select>', function() {
210219
}));
211220

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

225234
describe('label behavior', function() {
226235
it('defaults to the placeholder text', function() {
227-
var select = setupSelect('ng-model="someVal", placeholder="Hello world"', null, true).find('md-select');
236+
var select = setupSelect('ng-model="someVal" placeholder="Hello world"', null, true).find('md-select');
228237
var label = select.find('md-select-value');
229238
expect(label.text()).toBe('Hello world');
230239
expect(label.hasClass('md-select-placeholder')).toBe(true);
@@ -444,7 +453,7 @@ describe('<md-select>', function() {
444453
changesCalled = true;
445454
};
446455

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

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

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

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

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

0 commit comments

Comments
 (0)