From 11dec367ba4a3757ad53811809ed0df83b2cd1bd Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 10 Mar 2017 19:38:40 +0100 Subject: [PATCH] fix(select): unable to set a tabindex (#3479) * 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` --- src/lib/select/select.spec.ts | 35 ++++++++++++++++++++++++++++++++--- src/lib/select/select.ts | 26 ++++++++++++++++++-------- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index aceba1735fbf..a428799717e9 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -42,7 +42,8 @@ describe('MdSelect', () => { SelectWithErrorSibling, ThrowsErrorOnInit, BasicSelectOnPush, - BasicSelectOnPushPreselected + BasicSelectOnPushPreselected, + SelectWithPlainTabindex ], providers: [ {provide: OverlayContainer, useFactory: () => { @@ -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.`); @@ -1583,7 +1602,8 @@ describe('MdSelect', () => { selector: 'basic-select', template: `
- + {{ food.viewValue }} @@ -1606,6 +1626,7 @@ class BasicSelect { isRequired: boolean; heightAbove = 0; heightBelow = 0; + tabIndexOverride: number; @ViewChild(MdSelect) select: MdSelect; @ViewChildren(MdOption) options: QueryList; @@ -1864,6 +1885,14 @@ class MultiSelect { @ViewChildren(MdOption) options: QueryList; } +@Component({ + selector: 'select-with-plain-tabindex', + template: ` + + ` +}) +class SelectWithPlainTabindex { } + class FakeViewportRuler { getViewportRect() { diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index c0eb143bd086..32915f501b2c 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -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'; @@ -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()', @@ -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. @@ -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 { return Observable.merge(...this.options.map(option => option.onSelectionChange)); @@ -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() { @@ -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