Skip to content

Commit

Permalink
fix(select): unable to set a tabindex (#3479)
Browse files Browse the repository at this point in the history
* fix(select): unable to set a tabindex

Fixes users not being able to override the `tabIndex` on `md-select`.

Fixes #3474.

* fix: allow use of `tabindex`
  • Loading branch information
crisbeto authored and tinayuangao committed Mar 10, 2017
1 parent fb75a13 commit 11dec36
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 11 deletions.
35 changes: 32 additions & 3 deletions src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ describe('MdSelect', () => {
SelectWithErrorSibling,
ThrowsErrorOnInit,
BasicSelectOnPush,
BasicSelectOnPushPreselected
BasicSelectOnPushPreselected,
SelectWithPlainTabindex
],
providers: [
{provide: OverlayContainer, useFactory: () => {
Expand Down Expand Up @@ -1081,10 +1082,28 @@ 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 be able to set the tabindex via the native attribute', () => {
fixture.destroy();

const plainTabindexFixture = TestBed.createComponent(SelectWithPlainTabindex);

plainTabindexFixture.detectChanges();
select = plainTabindexFixture.debugElement.query(By.css('md-select')).nativeElement;

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

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 +1602,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 +1626,7 @@ class BasicSelect {
isRequired: boolean;
heightAbove = 0;
heightBelow = 0;
tabIndexOverride: number;

@ViewChild(MdSelect) select: MdSelect;
@ViewChildren(MdOption) options: QueryList<MdOption>;
Expand Down Expand Up @@ -1864,6 +1885,14 @@ class MultiSelect {
@ViewChildren(MdOption) options: QueryList<MdOption>;
}

@Component({
selector: 'select-with-plain-tabindex',
template: `
<md-select tabindex="5"></md-select>
`
})
class SelectWithPlainTabindex { }


class FakeViewportRuler {
getViewportRect() {
Expand Down
26 changes: 18 additions & 8 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
ViewEncapsulation,
ViewChild,
ChangeDetectorRef,
Attribute,
} from '@angular/core';
import {MdOption, MdOptionSelectionChange} from '../core/option/option';
import {ENTER, SPACE} from '../core/keyboard/keycodes';
Expand Down Expand Up @@ -99,7 +100,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 +152,9 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr
/** The animation state of the placeholder. */
private _placeholderState = '';

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

/**
* 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 +270,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 All @@ -282,10 +295,13 @@ export class MdSelect implements AfterContentInit, ControlValueAccessor, OnDestr

constructor(private _element: ElementRef, private _renderer: Renderer,
private _viewportRuler: ViewportRuler, private _changeDetectorRef: ChangeDetectorRef,
@Optional() private _dir: Dir, @Self() @Optional() public _control: NgControl) {
@Optional() private _dir: Dir, @Self() @Optional() public _control: NgControl,
@Attribute('tabindex') tabIndex: string) {
if (this._control) {
this._control.valueAccessor = this;
}

this._tabIndex = parseInt(tabIndex) || 0;
}

ngAfterContentInit() {
Expand Down Expand Up @@ -452,12 +468,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 11dec36

Please sign in to comment.