From 8f9f3c8fca2c7e046186772de848de693729c0d4 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 1 Sep 2017 17:08:02 +0200 Subject: [PATCH] fix(slide-toggle): support native tabindex attribute (#6613) Currently the slide-toggle only allows changing the tabIndex using the `tabIndex` binding. Using the native `tabindex` attribute doesn't have any affect. With this change the native tabindex property will be respected. References #6465 --- .../core/common-behaviors/tabindex.spec.ts | 48 +++++++++++++++++++ src/lib/core/common-behaviors/tabindex.ts | 34 +++++++++++++ src/lib/select/select.ts | 22 +++------ src/lib/slide-toggle/slide-toggle.spec.ts | 19 +++++++- src/lib/slide-toggle/slide-toggle.ts | 16 ++++--- 5 files changed, 115 insertions(+), 24 deletions(-) create mode 100644 src/lib/core/common-behaviors/tabindex.spec.ts create mode 100644 src/lib/core/common-behaviors/tabindex.ts diff --git a/src/lib/core/common-behaviors/tabindex.spec.ts b/src/lib/core/common-behaviors/tabindex.spec.ts new file mode 100644 index 00000000000..d796d1670bd --- /dev/null +++ b/src/lib/core/common-behaviors/tabindex.spec.ts @@ -0,0 +1,48 @@ +import {mixinTabIndex} from './tabindex'; + +describe('mixinTabIndex', () => { + + it('should augment an existing class with a tabIndex property', () => { + const classWithMixin = mixinTabIndex(TestClass); + const instance = new classWithMixin(); + + expect(instance.tabIndex) + .toBe(0, 'Expected the mixed-into class to have a tabIndex property'); + + instance.tabIndex = 4; + + expect(instance.tabIndex) + .toBe(4, 'Expected the mixed-into class to have an updated tabIndex property'); + }); + + it('should set tabIndex to `-1` if the disabled property is set to true', () => { + const classWithMixin = mixinTabIndex(TestClass); + const instance = new classWithMixin(); + + expect(instance.tabIndex) + .toBe(0, 'Expected tabIndex to be set to 0 initially'); + + instance.disabled = true; + + expect(instance.tabIndex) + .toBe(-1, 'Expected tabIndex to be set to -1 if the disabled property is set to true'); + }); + + it('should allow having a custom default tabIndex value', () => { + const classWithMixin = mixinTabIndex(TestClass, 20); + const instance = new classWithMixin(); + + expect(instance.tabIndex) + .toBe(20, 'Expected tabIndex to be set to 20 initially'); + + instance.tabIndex = 0; + + expect(instance.tabIndex) + .toBe(0, 'Expected tabIndex to still support 0 as value'); + }); + +}); + +class TestClass { + disabled = false; +} diff --git a/src/lib/core/common-behaviors/tabindex.ts b/src/lib/core/common-behaviors/tabindex.ts new file mode 100644 index 00000000000..a2efb44b399 --- /dev/null +++ b/src/lib/core/common-behaviors/tabindex.ts @@ -0,0 +1,34 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {Constructor} from './constructor'; +import {CanDisable} from './disabled'; + +/** @docs-private */ +export interface HasTabIndex { + tabIndex: number; +} + +/** Mixin to augment a directive with a `tabIndex` property. */ +export function mixinTabIndex>(base: T, defaultTabIndex = 0) + : Constructor & T { + return class extends base { + private _tabIndex: number = defaultTabIndex; + + get tabIndex(): number { return this.disabled ? -1 : this._tabIndex; } + set tabIndex(value: number) { + // If the specified tabIndex value is null or undefined, fall back to the default value. + this._tabIndex = value != null ? value : defaultTabIndex; + } + + constructor(...args: any[]) { + super(...args); + } + }; +} + diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 26b825faf49..8e6f4695ffe 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -66,6 +66,7 @@ import { PlaceholderOptions } from '../core/placeholder/placeholder-options'; import {Platform} from '@angular/cdk/platform'; +import {HasTabIndex, mixinTabIndex} from '../core/common-behaviors/tabindex'; /** * The following style constants are necessary to save here in order @@ -155,7 +156,8 @@ export class MdSelectChange { export class MdSelectBase { constructor(public _renderer: Renderer2, public _elementRef: ElementRef) {} } -export const _MdSelectMixinBase = mixinColor(mixinDisabled(MdSelectBase), 'primary'); +export const _MdSelectMixinBase = + mixinTabIndex(mixinColor(mixinDisabled(MdSelectBase), 'primary')); /** @@ -172,7 +174,7 @@ export class MdSelectTrigger {} selector: 'md-select, mat-select', templateUrl: 'select.html', styleUrls: ['select.css'], - inputs: ['color', 'disabled'], + inputs: ['color', 'disabled', 'tabIndex'], encapsulation: ViewEncapsulation.None, changeDetection: ChangeDetectionStrategy.OnPush, host: { @@ -200,7 +202,7 @@ export class MdSelectTrigger {} exportAs: 'mdSelect', }) export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, OnDestroy, OnInit, - ControlValueAccessor, CanColor, CanDisable { + ControlValueAccessor, CanColor, CanDisable, HasTabIndex { /** Whether or not the overlay panel is open. */ private _panelOpen = false; @@ -234,9 +236,6 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On /** The animation state of the placeholder. */ private _placeholderState = ''; - /** Tab index for the element. */ - private _tabIndex: number; - /** Deals with configuring placeholder options */ private _placeholderOptions: PlaceholderOptions; @@ -371,15 +370,6 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On } private _floatPlaceholder: FloatPlaceholderType; - /** 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; - } - } - /** Value of the select control. */ @Input() get value() { return this._value; } @@ -445,7 +435,7 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On this._control.valueAccessor = this; } - this._tabIndex = parseInt(tabIndex) || 0; + this.tabIndex = parseInt(tabIndex) || 0; this._placeholderOptions = placeholderOptions ? placeholderOptions : {}; this.floatPlaceholder = this._placeholderOptions.float || 'auto'; } diff --git a/src/lib/slide-toggle/slide-toggle.spec.ts b/src/lib/slide-toggle/slide-toggle.spec.ts index eda08374dc1..d8bb973f8b0 100644 --- a/src/lib/slide-toggle/slide-toggle.spec.ts +++ b/src/lib/slide-toggle/slide-toggle.spec.ts @@ -16,7 +16,7 @@ describe('MdSlideToggle without forms', () => { beforeEach(async(() => { TestBed.configureTestingModule({ imports: [MdSlideToggleModule], - declarations: [SlideToggleBasic], + declarations: [SlideToggleBasic, SlideToggleWithTabindexAttr], providers: [ {provide: HAMMER_GESTURE_CONFIG, useFactory: () => gestureConfig = new TestGestureConfig()} ] @@ -333,6 +333,18 @@ describe('MdSlideToggle without forms', () => { expect(fixture.componentInstance.lastEvent).toBeFalsy(); })); + + it('should be able to set the tabindex via the native attribute', async(() => { + const fixture = TestBed.createComponent(SlideToggleWithTabindexAttr); + + fixture.detectChanges(); + + const slideToggle = fixture.debugElement + .query(By.directive(MdSlideToggle)).componentInstance as MdSlideToggle; + + expect(slideToggle.tabIndex) + .toBe(5, 'Expected tabIndex property to have been set based on the native attribute'); + })); }); describe('with dragging', () => { @@ -789,3 +801,8 @@ class SlideToggleWithModel { class SlideToggleWithFormControl { formControl = new FormControl(); } + +@Component({ + template: `` +}) +class SlideToggleWithTabindexAttr {} diff --git a/src/lib/slide-toggle/slide-toggle.ts b/src/lib/slide-toggle/slide-toggle.ts index 28c4eaf0742..6de28f90c39 100644 --- a/src/lib/slide-toggle/slide-toggle.ts +++ b/src/lib/slide-toggle/slide-toggle.ts @@ -8,6 +8,7 @@ import { AfterContentInit, + Attribute, ChangeDetectionStrategy, ChangeDetectorRef, Component, @@ -35,6 +36,7 @@ import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import {mixinDisabled, CanDisable} from '../core/common-behaviors/disabled'; import {CanColor, mixinColor} from '../core/common-behaviors/color'; import {CanDisableRipple, mixinDisableRipple} from '../core/common-behaviors/disable-ripple'; +import {HasTabIndex, mixinTabIndex} from '../core/common-behaviors/tabindex'; // Increasing integer for generating unique ids for slide-toggle components. let nextUniqueId = 0; @@ -57,7 +59,7 @@ export class MdSlideToggleBase { constructor(public _renderer: Renderer2, public _elementRef: ElementRef) {} } export const _MdSlideToggleMixinBase = - mixinColor(mixinDisableRipple(mixinDisabled(MdSlideToggleBase)), 'accent'); + mixinTabIndex(mixinColor(mixinDisableRipple(mixinDisabled(MdSlideToggleBase)), 'accent')); /** Represents a slidable "switch" toggle that can be moved between on and off. */ @Component({ @@ -73,12 +75,12 @@ export const _MdSlideToggleMixinBase = templateUrl: 'slide-toggle.html', styleUrls: ['slide-toggle.css'], providers: [MD_SLIDE_TOGGLE_VALUE_ACCESSOR], - inputs: ['disabled', 'disableRipple', 'color'], + inputs: ['disabled', 'disableRipple', 'color', 'tabIndex'], encapsulation: ViewEncapsulation.None, changeDetection: ChangeDetectionStrategy.OnPush }) export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy, AfterContentInit, - ControlValueAccessor, CanDisable, CanColor, CanDisableRipple { + ControlValueAccessor, CanDisable, CanColor, HasTabIndex, CanDisableRipple { private onChange = (_: any) => {}; private onTouched = () => {}; @@ -97,9 +99,6 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy, /** A unique id for the slide-toggle input. If none is supplied, it will be auto-generated. */ @Input() id: string = this._uniqueId; - /** Used to specify the tabIndex value for the underlying input element. */ - @Input() tabIndex: number = 0; - /** Whether the label should appear after or before the slide-toggle. Defaults to 'after' */ @Input() labelPosition: 'before' | 'after' = 'after'; @@ -139,8 +138,11 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy, renderer: Renderer2, private _platform: Platform, private _focusOriginMonitor: FocusOriginMonitor, - private _changeDetectorRef: ChangeDetectorRef) { + private _changeDetectorRef: ChangeDetectorRef, + @Attribute('tabindex') tabIndex: string) { super(renderer, elementRef); + + this.tabIndex = parseInt(tabIndex) || 0; } ngAfterContentInit() {