Skip to content

Commit

Permalink
fix(select): unable to set a tabindex
Browse files Browse the repository at this point in the history
Fixes users not being able to override the `tabIndex` on `md-select`.

Fixes angular#3474.
  • Loading branch information
crisbeto committed Mar 9, 2017
1 parent f27617a commit 934be6f
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 9 deletions.
13 changes: 11 additions & 2 deletions src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1081,10 +1081,17 @@ describe('MdSelect', () => {
expect(select.getAttribute('aria-label')).toEqual('Food');
});

it('should set the tabindex of the select to 0', () => {
it('should set the tabindex of the select to 0 by default', () => {
expect(select.getAttribute('tabindex')).toEqual('0');
});

it('should be able to override the tabindex', () => {
fixture.componentInstance.tabIndexOverride = 3;
fixture.detectChanges();

expect(select.getAttribute('tabindex')).toBe('3');
});

it('should set aria-required for required selects', () => {
expect(select.getAttribute('aria-required'))
.toEqual('false', `Expected aria-required attr to be false for normal selects.`);
Expand Down Expand Up @@ -1583,7 +1590,8 @@ describe('MdSelect', () => {
selector: 'basic-select',
template: `
<div [style.height.px]="heightAbove"></div>
<md-select placeholder="Food" [formControl]="control" [required]="isRequired">
<md-select placeholder="Food" [formControl]="control" [required]="isRequired"
[tabIndex]="tabIndexOverride">
<md-option *ngFor="let food of foods" [value]="food.value" [disabled]="food.disabled">
{{ food.viewValue }}
</md-option>
Expand All @@ -1606,6 +1614,7 @@ class BasicSelect {
isRequired: boolean;
heightAbove = 0;
heightBelow = 0;
tabIndexOverride: number;

@ViewChild(MdSelect) select: MdSelect;
@ViewChildren(MdOption) options: QueryList<MdOption>;
Expand Down
20 changes: 13 additions & 7 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export type MdSelectFloatPlaceholderType = 'always' | 'never' | 'auto';
encapsulation: ViewEncapsulation.None,
host: {
'role': 'listbox',
'[attr.tabindex]': '_getTabIndex()',
'[attr.tabindex]': 'tabIndex',
'[attr.aria-label]': 'placeholder',
'[attr.aria-required]': 'required.toString()',
'[attr.aria-disabled]': 'disabled.toString()',
Expand Down Expand Up @@ -151,6 +151,9 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
/** The animation state of the placeholder. */
private _placeholderState = '';

/** Tab index for the element. */
private _tabIndex: number = 0;

/**
* The width of the trigger. Must be saved to set the min width of the overlay panel
* and the width of the selected value.
Expand Down Expand Up @@ -266,6 +269,15 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
}
private _floatPlaceholder: MdSelectFloatPlaceholderType = 'auto';

/** Tab index for the select element. */
@Input()
get tabIndex(): number { return this._disabled ? -1 : this._tabIndex; }
set tabIndex(value: number) {
if (typeof value !== 'undefined') {
this._tabIndex = value;
}
}

/** Combined stream of all of the child options' change events. */
get optionSelectionChanges(): Observable<MdOptionSelectionChange> {
return Observable.merge(...this.options.map(option => option.onSelectionChange));
Expand Down Expand Up @@ -452,12 +464,6 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
}
}

/** Returns the correct tabindex for the select depending on disabled state. */
_getTabIndex() {
return this.disabled ? '-1' : '0';
}


/**
* Sets the scroll position of the scroll container. This must be called after
* the overlay pane is attached or the scroll container element will not yet be
Expand Down

0 comments on commit 934be6f

Please sign in to comment.