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.
angular · Jan 29, 2020 · bbb92a7 · bbb92a7
1 parent ea17135
commit bbb92a7
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 29 deletions.
diff --git 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>(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: `<mat-basic-chip>{{name}}</mat-basic-chip>`
+  template: `<mat-basic-chip>Hello</mat-basic-chip>`
 })
 class BasicChip {
 }
+
+@Component({
+  template: `<mat-basic-chip tabindex="3">Hello</mat-basic-chip>`
+})
+class BasicChipWithStaticTabindex {
+}
+
+@Component({
+  template: `<mat-basic-chip [tabIndex]="tabindex">Hello</mat-basic-chip>`
+})
+class BasicChipWithBoundTabindex {
+  tabindex = 12;
+}
diff --git 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>(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: `<mat-basic-chip>{{name}}</mat-basic-chip>`
+  template: `<mat-basic-chip>Hello</mat-basic-chip>`
 })
 class BasicChip {
 }
+
+@Component({
+  template: `<mat-basic-chip tabindex="3">Hello</mat-basic-chip>`
+})
+class BasicChipWithStaticTabindex {
+}
+
+@Component({
+  template: `<mat-basic-chip [tabIndex]="tabindex">Hello</mat-basic-chip>`
+})
+class BasicChipWithBoundTabindex {
+  tabindex = 12;
+}
diff --git 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
@@ -1,6 +1,6 @@
 export declare const MAT_CHIPS_DEFAULT_OPTIONS: InjectionToken<MatChipsDefaultOptions>;
 
-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<HTMLElement>;
@@ -29,7 +29,7 @@ export declare class MatChip extends _MatChipMixinBase implements FocusableOptio
     trailingIcon: MatChipTrailingIcon;
     get value(): any;
     set value(value: any);
-    constructor(_elementRef: ElementRef<HTMLElement>, _ngZone: NgZone, platform: Platform, globalRippleOptions: RippleGlobalOptions | null, animationMode?: string, _changeDetectorRef?: ChangeDetectorRef | undefined);
+    constructor(_elementRef: ElementRef<HTMLElement>, _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<MatChip, "mat-basic-chip, [mat-basic-chip], mat-chip, [mat-chip]", ["matChip"], { "color": "color"; "disabled": "disabled"; "disableRipple": "disableRipple"; "selected": "selected"; "value": "value"; "selectable": "selectable"; "removable": "removable"; }, { "selectionChange": "selectionChange"; "destroyed": "destroyed"; "removed": "removed"; }, ["avatar", "trailingIcon", "removeIcon"]>;
+    static ɵdir: i0.ɵɵDirectiveDefWithMeta<MatChip, "mat-basic-chip, [mat-basic-chip], mat-chip, [mat-chip]", ["matChip"], { "color": "color"; "disabled": "disabled"; "disableRipple": "disableRipple"; "tabIndex": "tabIndex"; "selected": "selected"; "value": "value"; "selectable": "selectable"; "removable": "removable"; }, { "selectionChange": "selectionChange"; "destroyed": "destroyed"; "removed": "removed"; }, ["avatar", "trailingIcon", "removeIcon"]>;
     static ɵfac: i0.ɵɵFactoryDef<MatChip>;
 }