From bbb92a7e2c0ae5845971069e2be58873831f8111 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 29 Jan 2020 07:59:26 +0100 Subject: [PATCH] fix(chips): unable to set custom tabindex on chip (#17699) I noticed this as I was going to recommend to someone on GitHub to set a tabindex on a `mat-chip`. As things are setup right now, the `mat-chip` will override whatever `tabindex` is set on it. Note that this isn't an issue on the MDC chip. These changes fix the current `mat-chip` and add tests to the MDC based one on so it doesn't regress. --- .../mdc-chips/chip.spec.ts | 59 ++++++++++++++++--- src/material/chips/chip.spec.ts | 55 +++++++++++++---- src/material/chips/chip.ts | 19 ++++-- tools/public_api_guard/material/chips.d.ts | 6 +- 4 files changed, 110 insertions(+), 29 deletions(-) diff --git a/src/material-experimental/mdc-chips/chip.spec.ts b/src/material-experimental/mdc-chips/chip.spec.ts index d4c9b7125913..f5ed6a56612a 100644 --- a/src/material-experimental/mdc-chips/chip.spec.ts +++ b/src/material-experimental/mdc-chips/chip.spec.ts @@ -21,7 +21,12 @@ describe('MDC-based MatChip', () => { globalRippleOptions = {}; TestBed.configureTestingModule({ imports: [MatChipsModule], - declarations: [BasicChip, SingleChip], + declarations: [ + BasicChip, + SingleChip, + BasicChipWithStaticTabindex, + BasicChipWithBoundTabindex, + ], providers: [ {provide: MAT_RIPPLE_GLOBAL_OPTIONS, useFactory: () => globalRippleOptions}, {provide: Directionality, useFactory: () => ({ @@ -35,18 +40,41 @@ describe('MDC-based MatChip', () => { })); describe('MatBasicChip', () => { - - beforeEach(() => { + it('adds the `mat-mdc-basic-chip` class', () => { fixture = TestBed.createComponent(BasicChip); fixture.detectChanges(); - chipDebugElement = fixture.debugElement.query(By.directive(MatChip))!; - chipNativeElement = chipDebugElement.nativeElement; - chipInstance = chipDebugElement.injector.get(MatChip); + const chip = fixture.nativeElement.querySelector('mat-basic-chip'); + expect(chip.classList).toContain('mat-mdc-basic-chip'); }); - it('adds the `mat-mdc-basic-chip` class', () => { - expect(chipNativeElement.classList).toContain('mat-mdc-basic-chip'); + it('should be able to set a static tabindex', () => { + fixture = TestBed.createComponent(BasicChipWithStaticTabindex); + fixture.detectChanges(); + + const chip = fixture.nativeElement.querySelector('mat-basic-chip'); + expect(chip.getAttribute('tabindex')).toBe('3'); + }); + + it('should be able to set a static tabindex', () => { + fixture = TestBed.createComponent(BasicChipWithStaticTabindex); + fixture.detectChanges(); + + const chip = fixture.nativeElement.querySelector('mat-basic-chip'); + expect(chip.getAttribute('tabindex')).toBe('3'); + }); + + it('should be able to set a dynamic tabindex', () => { + fixture = TestBed.createComponent(BasicChipWithBoundTabindex); + fixture.detectChanges(); + + const chip = fixture.nativeElement.querySelector('mat-basic-chip'); + expect(chip.getAttribute('tabindex')).toBe('12'); + + fixture.componentInstance.tabindex = 15; + fixture.detectChanges(); + + expect(chip.getAttribute('tabindex')).toBe('15'); }); }); @@ -184,7 +212,20 @@ class SingleChip { } @Component({ - template: `{{name}}` + template: `Hello` }) class BasicChip { } + +@Component({ + template: `Hello` +}) +class BasicChipWithStaticTabindex { +} + +@Component({ + template: `Hello` +}) +class BasicChipWithBoundTabindex { + tabindex = 12; +} diff --git a/src/material/chips/chip.spec.ts b/src/material/chips/chip.spec.ts index bb1ab9368b51..d89cdeda9c73 100644 --- a/src/material/chips/chip.spec.ts +++ b/src/material/chips/chip.spec.ts @@ -15,14 +15,18 @@ describe('MatChip', () => { let chipNativeElement: HTMLElement; let chipInstance: MatChip; let globalRippleOptions: RippleGlobalOptions; - let dir = 'ltr'; beforeEach(async(() => { globalRippleOptions = {}; TestBed.configureTestingModule({ imports: [MatChipsModule], - declarations: [BasicChip, SingleChip], + declarations: [ + BasicChip, + SingleChip, + BasicChipWithStaticTabindex, + BasicChipWithBoundTabindex, + ], providers: [ {provide: MAT_RIPPLE_GLOBAL_OPTIONS, useFactory: () => globalRippleOptions}, {provide: Directionality, useFactory: () => ({ @@ -36,22 +40,38 @@ describe('MatChip', () => { })); describe('MatBasicChip', () => { - - beforeEach(() => { + it('adds the `mat-basic-chip` class', () => { fixture = TestBed.createComponent(BasicChip); fixture.detectChanges(); - chipDebugElement = fixture.debugElement.query(By.directive(MatChip))!; - chipNativeElement = chipDebugElement.nativeElement; - chipInstance = chipDebugElement.injector.get(MatChip); + const chip = fixture.nativeElement.querySelector('mat-basic-chip'); + expect(chip.classList).toContain('mat-chip'); + expect(chip.classList).toContain('mat-basic-chip'); }); - it('adds the `mat-basic-chip` class', () => { - expect(chipNativeElement.classList).toContain('mat-chip'); - expect(chipNativeElement.classList).toContain('mat-basic-chip'); + it('should be able to set a static tabindex', () => { + fixture = TestBed.createComponent(BasicChipWithStaticTabindex); + fixture.detectChanges(); + + const chip = fixture.nativeElement.querySelector('mat-basic-chip'); + expect(chip.getAttribute('tabindex')).toBe('3'); + }); + + it('should be able to set a dynamic tabindex', () => { + fixture = TestBed.createComponent(BasicChipWithBoundTabindex); + fixture.detectChanges(); + + const chip = fixture.nativeElement.querySelector('mat-basic-chip'); + expect(chip.getAttribute('tabindex')).toBe('12'); + + fixture.componentInstance.tabindex = 15; + fixture.detectChanges(); + + expect(chip.getAttribute('tabindex')).toBe('15'); }); }); + describe('MatChip', () => { let testComponent: SingleChip; @@ -426,7 +446,20 @@ class SingleChip { } @Component({ - template: `{{name}}` + template: `Hello` }) class BasicChip { } + +@Component({ + template: `Hello` +}) +class BasicChipWithStaticTabindex { +} + +@Component({ + template: `Hello` +}) +class BasicChipWithBoundTabindex { + tabindex = 12; +} diff --git a/src/material/chips/chip.ts b/src/material/chips/chip.ts index c4ac757e1437..c55e267752e2 100644 --- a/src/material/chips/chip.ts +++ b/src/material/chips/chip.ts @@ -23,6 +23,7 @@ import { Optional, Output, ChangeDetectorRef, + Attribute, } from '@angular/core'; import { CanColor, @@ -31,6 +32,9 @@ import { CanDisableCtor, CanDisableRipple, CanDisableRippleCtor, + HasTabIndex, + HasTabIndexCtor, + mixinTabIndex, MAT_RIPPLE_GLOBAL_OPTIONS, mixinColor, mixinDisabled, @@ -69,8 +73,9 @@ class MatChipBase { constructor(public _elementRef: ElementRef) {} } -const _MatChipMixinBase: CanColorCtor & CanDisableRippleCtor & CanDisableCtor & typeof MatChipBase = - mixinColor(mixinDisableRipple(mixinDisabled(MatChipBase)), 'primary'); +const _MatChipMixinBase: CanColorCtor & CanDisableRippleCtor & CanDisableCtor & + HasTabIndexCtor & typeof MatChipBase = + mixinTabIndex(mixinColor(mixinDisableRipple(mixinDisabled(MatChipBase)), 'primary'), -1); /** * Dummy directive to add CSS class to chip avatar. @@ -97,11 +102,11 @@ export class MatChipTrailingIcon {} */ @Directive({ selector: `mat-basic-chip, [mat-basic-chip], mat-chip, [mat-chip]`, - inputs: ['color', 'disabled', 'disableRipple'], + inputs: ['color', 'disabled', 'disableRipple', 'tabIndex'], exportAs: 'matChip', host: { 'class': 'mat-chip', - '[attr.tabindex]': 'disabled ? null : -1', + '[attr.tabindex]': 'disabled ? null : tabIndex', 'role': 'option', '[class.mat-chip-selected]': 'selected', '[class.mat-chip-with-avatar]': 'avatar', @@ -118,7 +123,7 @@ export class MatChipTrailingIcon {} }, }) export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDestroy, CanColor, - CanDisable, CanDisableRipple, RippleTarget { + CanDisable, CanDisableRipple, RippleTarget, HasTabIndex { /** Reference to the RippleRenderer for the chip. */ private _chipRipple: RippleRenderer; @@ -238,7 +243,8 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes // @breaking-change 8.0.0 `animationMode` parameter to become required. @Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string, // @breaking-change 9.0.0 `_changeDetectorRef` parameter to become required. - private _changeDetectorRef?: ChangeDetectorRef) { + private _changeDetectorRef?: ChangeDetectorRef, + @Attribute('tabindex') tabIndex?: string) { super(_elementRef); this._addHostClassName(); @@ -247,6 +253,7 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes this._chipRipple.setupTriggerEvents(_elementRef); this.rippleConfig = globalRippleOptions || {}; this._animationsDisabled = animationMode === 'NoopAnimations'; + this.tabIndex = tabIndex != null ? (parseInt(tabIndex) || -1) : -1; } _addHostClassName() { diff --git a/tools/public_api_guard/material/chips.d.ts b/tools/public_api_guard/material/chips.d.ts index 63cbec89d131..bd91cc143ec4 100644 --- a/tools/public_api_guard/material/chips.d.ts +++ b/tools/public_api_guard/material/chips.d.ts @@ -1,6 +1,6 @@ export declare const MAT_CHIPS_DEFAULT_OPTIONS: InjectionToken; -export declare class MatChip extends _MatChipMixinBase implements FocusableOption, OnDestroy, CanColor, CanDisable, CanDisableRipple, RippleTarget { +export declare class MatChip extends _MatChipMixinBase implements FocusableOption, OnDestroy, CanColor, CanDisable, CanDisableRipple, RippleTarget, HasTabIndex { _animationsDisabled: boolean; _chipListMultiple: boolean; _elementRef: ElementRef; @@ -29,7 +29,7 @@ export declare class MatChip extends _MatChipMixinBase implements FocusableOptio trailingIcon: MatChipTrailingIcon; get value(): any; set value(value: any); - constructor(_elementRef: ElementRef, _ngZone: NgZone, platform: Platform, globalRippleOptions: RippleGlobalOptions | null, animationMode?: string, _changeDetectorRef?: ChangeDetectorRef | undefined); + constructor(_elementRef: ElementRef, _ngZone: NgZone, platform: Platform, globalRippleOptions: RippleGlobalOptions | null, animationMode?: string, _changeDetectorRef?: ChangeDetectorRef | undefined, tabIndex?: string); _addHostClassName(): void; _blur(): void; _handleClick(event: Event): void; @@ -46,7 +46,7 @@ export declare class MatChip extends _MatChipMixinBase implements FocusableOptio static ngAcceptInputType_removable: BooleanInput; static ngAcceptInputType_selectable: BooleanInput; static ngAcceptInputType_selected: BooleanInput; - static ɵdir: i0.ɵɵDirectiveDefWithMeta; + static ɵdir: i0.ɵɵDirectiveDefWithMeta; static ɵfac: i0.ɵɵFactoryDef; }