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

Commit cc4f626

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 05a08c8 commit cc4f626

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
@@ -190,7 +190,9 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
190190
element.empty().append(valueEl);
191191
element.append(selectTemplate);
192192

193-
attr.tabindex = attr.tabindex || '0';
193+
if(!attr.tabindex){
194+
attr.$set('tabindex', 0);
195+
}
194196

195197
return function postLink(scope, element, attr, ctrls) {
196198
var untouched = true;
@@ -385,23 +387,25 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
385387
}
386388
isDisabled = disabled;
387389
if (disabled) {
388-
element.attr({'tabindex': -1, 'aria-disabled': 'true'});
389-
element.off('click', openSelect);
390-
element.off('keydown', handleKeypress);
390+
element
391+
.attr({'aria-disabled': 'true'})
392+
.removeAttr('tabindex')
393+
.off('click', openSelect)
394+
.off('keydown', handleKeypress);
391395
} else {
392-
element.attr({'tabindex': attr.tabindex, 'aria-disabled': 'false'});
393-
element.on('click', openSelect);
394-
element.on('keydown', handleKeypress);
396+
element
397+
.attr({'tabindex': attr.tabindex, 'aria-disabled': 'false'})
398+
.on('click', openSelect)
399+
.on('keydown', handleKeypress);
395400
}
396401
});
397402

398-
if (!attr.disabled && !attr.ngDisabled) {
399-
element.attr({'tabindex': attr.tabindex, 'aria-disabled': 'false'});
403+
if (!attr.hasOwnProperty('disabled') && !attr.hasOwnProperty('ngDisabled')) {
404+
element.attr({'aria-disabled': 'false'});
400405
element.on('click', openSelect);
401406
element.on('keydown', handleKeypress);
402407
}
403408

404-
405409
var ariaAttrs = {
406410
role: 'listbox',
407411
'aria-expanded': 'false',
@@ -1238,7 +1242,7 @@ function SelectProvider($$interimElementProvider) {
12381242
}
12391243
newOption = optionsArray[index];
12401244
if (newOption.hasAttribute('disabled')) newOption = undefined;
1241-
} while (!newOption && index < optionsArray.length - 1 && index > 0)
1245+
} while (!newOption && index < optionsArray.length - 1 && index > 0);
12421246
newOption && newOption.focus();
12431247
opts.focusedNode = newOption;
12441248
}

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);
@@ -486,7 +495,7 @@ describe('<md-select>', function() {
486495
changesCalled = true;
487496
};
488497

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

492501
var menuEl = $document.find('md-select-menu');
@@ -815,7 +824,7 @@ describe('<md-select>', function() {
815824
}));
816825

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

@@ -835,7 +844,7 @@ describe('<md-select>', function() {
835844
}));
836845

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

0 commit comments

Comments
 (0)