From 8bead03ef3c2a2329453dd0057a76dc0ac1aca2b Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 23 Aug 2017 19:55:26 +0200 Subject: [PATCH 1/3] fix(slide-toggle): support native tabindex attribute 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 | 43 +++++++++++++++++++ src/lib/core/common-behaviors/tabindex.ts | 35 +++++++++++++++ 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, 111 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 000000000000..a833a6ad603e --- /dev/null +++ b/src/lib/core/common-behaviors/tabindex.spec.ts @@ -0,0 +1,43 @@ +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'); + }); + +}); + +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 000000000000..129b944527d1 --- /dev/null +++ b/src/lib/core/common-behaviors/tabindex.ts @@ -0,0 +1,35 @@ +/** + * @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 CanTabIndex { + 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; + + get tabIndex(): number { return this.disabled ? -1 : this._tabIndex || defaultTabIndex; } + set tabIndex(value: number) { + if (typeof value !== 'undefined') { + this._tabIndex = value; + } + } + + constructor(...args: any[]) { + super(...args); + } + }; +} + diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 6e89d0880289..4d66d683b3ae 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 {CanTabIndex, 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, CanTabIndex { /** 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; } @@ -446,7 +436,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 eda08374dc12..d8bb973f8b0f 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 07b4e69bf613..b0c6886a5ca1 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 {CanTabIndex, 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, CanTabIndex, 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() { From 4a97a932de8a6108d276df17d083220eec1be017 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 24 Aug 2017 00:51:44 +0200 Subject: [PATCH 2/3] Address feedback --- src/lib/core/common-behaviors/tabindex.ts | 4 ++-- src/lib/select/select.ts | 4 ++-- src/lib/slide-toggle/slide-toggle.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/core/common-behaviors/tabindex.ts b/src/lib/core/common-behaviors/tabindex.ts index 129b944527d1..d5b219cd3418 100644 --- a/src/lib/core/common-behaviors/tabindex.ts +++ b/src/lib/core/common-behaviors/tabindex.ts @@ -10,13 +10,13 @@ import {Constructor} from './constructor'; import {CanDisable} from './disabled'; /** @docs-private */ -export interface CanTabIndex { +export interface HasTabIndex { tabIndex: number; } /** Mixin to augment a directive with a `tabIndex` property. */ export function mixinTabIndex>(base: T, defaultTabIndex = 0) - : Constructor & T { + : Constructor & T { return class extends base { private _tabIndex: number; diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 4d66d683b3ae..4e620dfe4e1c 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -66,7 +66,7 @@ import { PlaceholderOptions } from '../core/placeholder/placeholder-options'; import {Platform} from '@angular/cdk/platform'; -import {CanTabIndex, mixinTabIndex} from '../core/common-behaviors/tabindex'; +import {HasTabIndex, mixinTabIndex} from '../core/common-behaviors/tabindex'; /** * The following style constants are necessary to save here in order @@ -202,7 +202,7 @@ export class MdSelectTrigger {} exportAs: 'mdSelect', }) export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, OnDestroy, OnInit, - ControlValueAccessor, CanColor, CanDisable, CanTabIndex { + ControlValueAccessor, CanColor, CanDisable, HasTabIndex { /** Whether or not the overlay panel is open. */ private _panelOpen = false; diff --git a/src/lib/slide-toggle/slide-toggle.ts b/src/lib/slide-toggle/slide-toggle.ts index b0c6886a5ca1..3881831a4780 100644 --- a/src/lib/slide-toggle/slide-toggle.ts +++ b/src/lib/slide-toggle/slide-toggle.ts @@ -36,7 +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 {CanTabIndex, mixinTabIndex} from '../core/common-behaviors/tabindex'; +import {HasTabIndex, mixinTabIndex} from '../core/common-behaviors/tabindex'; // Increasing integer for generating unique ids for slide-toggle components. let nextUniqueId = 0; @@ -80,7 +80,7 @@ export const _MdSlideToggleMixinBase = changeDetection: ChangeDetectionStrategy.OnPush }) export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy, AfterContentInit, - ControlValueAccessor, CanDisable, CanColor, CanTabIndex, CanDisableRipple { + ControlValueAccessor, CanDisable, CanColor, HasTabIndex, CanDisableRipple { private onChange = (_: any) => {}; private onTouched = () => {}; From ce92da872c667c162868d405fb785763c902200f Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 24 Aug 2017 11:31:10 +0200 Subject: [PATCH 3/3] Better fall back for default value --- src/lib/core/common-behaviors/tabindex.spec.ts | 5 +++++ src/lib/core/common-behaviors/tabindex.ts | 9 ++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/lib/core/common-behaviors/tabindex.spec.ts b/src/lib/core/common-behaviors/tabindex.spec.ts index a833a6ad603e..d796d1670bd1 100644 --- a/src/lib/core/common-behaviors/tabindex.spec.ts +++ b/src/lib/core/common-behaviors/tabindex.spec.ts @@ -34,6 +34,11 @@ describe('mixinTabIndex', () => { 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'); }); }); diff --git a/src/lib/core/common-behaviors/tabindex.ts b/src/lib/core/common-behaviors/tabindex.ts index d5b219cd3418..a2efb44b3994 100644 --- a/src/lib/core/common-behaviors/tabindex.ts +++ b/src/lib/core/common-behaviors/tabindex.ts @@ -18,13 +18,12 @@ export interface HasTabIndex { export function mixinTabIndex>(base: T, defaultTabIndex = 0) : Constructor & T { return class extends base { - private _tabIndex: number; + private _tabIndex: number = defaultTabIndex; - get tabIndex(): number { return this.disabled ? -1 : this._tabIndex || defaultTabIndex; } + get tabIndex(): number { return this.disabled ? -1 : this._tabIndex; } set tabIndex(value: number) { - if (typeof value !== 'undefined') { - this._tabIndex = value; - } + // If the specified tabIndex value is null or undefined, fall back to the default value. + this._tabIndex = value != null ? value : defaultTabIndex; } constructor(...args: any[]) {