From c7e2d4ec7367d6d42f19f7b3082d281b3fa6502e Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 13 Dec 2017 22:49:02 +0100 Subject: [PATCH] fix(chips): event propagation not stopped by remove button (#8772) Fixes clicking on the chip remove button propagating up and potentially triggering its parent control. This is usually handled by the chip itself, however clicking on the button could remove the chip before it has the chance to stop propagation. Fixes #8771. --- src/lib/chips/chip-remove.spec.ts | 29 ++++++++++++++++++++--------- src/lib/chips/chip.ts | 9 +++++++-- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/lib/chips/chip-remove.spec.ts b/src/lib/chips/chip-remove.spec.ts index b8fabef214d..fa953b91a71 100644 --- a/src/lib/chips/chip-remove.spec.ts +++ b/src/lib/chips/chip-remove.spec.ts @@ -1,7 +1,8 @@ import {Component, DebugElement} from '@angular/core'; import {By} from '@angular/platform-browser'; -import {async, ComponentFixture, TestBed} from '@angular/core/testing'; +import {fakeAsync, ComponentFixture, TestBed} from '@angular/core/testing'; import {MatChip, MatChipsModule} from './index'; +import {dispatchFakeEvent} from '@angular/cdk/testing'; describe('Chip Remove', () => { let fixture: ComponentFixture; @@ -9,7 +10,7 @@ describe('Chip Remove', () => { let chipDebugElement: DebugElement; let chipNativeElement: HTMLElement; - beforeEach(async(() => { + beforeEach(fakeAsync(() => { TestBed.configureTestingModule({ imports: [MatChipsModule], declarations: [ @@ -18,9 +19,6 @@ describe('Chip Remove', () => { }); TestBed.compileComponents(); - })); - - beforeEach(async(() => { fixture = TestBed.createComponent(TestChip); testChip = fixture.debugElement.componentInstance; fixture.detectChanges(); @@ -30,14 +28,14 @@ describe('Chip Remove', () => { })); describe('basic behavior', () => { - it('should applies the `mat-chip-remove` CSS class', () => { - let hrefElement = chipNativeElement.querySelector('a')!; + it('should apply the `mat-chip-remove` CSS class', () => { + const hrefElement = chipNativeElement.querySelector('a')!; expect(hrefElement.classList).toContain('mat-chip-remove'); }); - it('should emits (remove) on click', () => { - let hrefElement = chipNativeElement.querySelector('a')!; + it('should emit (remove) on click', () => { + const hrefElement = chipNativeElement.querySelector('a')!; testChip.removable = true; fixture.detectChanges(); @@ -48,6 +46,19 @@ describe('Chip Remove', () => { expect(testChip.didRemove).toHaveBeenCalled(); }); + + it('should prevent the default click action', () => { + const hrefElement = chipNativeElement.querySelector('a')!; + + testChip.removable = true; + fixture.detectChanges(); + + const event = dispatchFakeEvent(hrefElement, 'click'); + fixture.detectChanges(); + + expect(event.defaultPrevented).toBe(true); + }); + }); }); diff --git a/src/lib/chips/chip.ts b/src/lib/chips/chip.ts index 2cbb6248c00..564dc153a42 100644 --- a/src/lib/chips/chip.ts +++ b/src/lib/chips/chip.ts @@ -308,7 +308,7 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes selector: '[matChipRemove]', host: { 'class': 'mat-chip-remove', - '(click)': '_handleClick()', + '(click)': '_handleClick($event)', } }) export class MatChipRemove { @@ -316,9 +316,14 @@ export class MatChipRemove { } /** Calls the parent chip's public `remove()` method if applicable. */ - _handleClick(): void { + _handleClick(event: MouseEvent): void { if (this._parentChip.removable) { this._parentChip.remove(); + + // Note: the parent chip does something similar, however since we're removing it, + // its event handler will be unbound before it has had the chance to fire. + event.preventDefault(); + event.stopPropagation(); } } }