Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(material-experimental/mdc-chips): decouple removal from animation #21636

Merged
merged 2 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 3 additions & 15 deletions src/material-experimental/mdc-chips/chip-grid.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
TAB
} from '@angular/cdk/keycodes';
import {
createFakeEvent,
createKeyboardEvent,
dispatchEvent,
dispatchFakeEvent,
Expand Down Expand Up @@ -216,7 +215,7 @@ describe('MDC-based MatChipGrid', () => {
expect(chipGridInstance.focus).toHaveBeenCalled();
});

it('should move focus to the last chip when the focused chip was deleted inside a' +
it('should move focus to the last chip when the focused chip was deleted inside a ' +
'component with animations', fakeAsync(() => {
fixture.destroy();
TestBed.resetTestingModule();
Expand Down Expand Up @@ -602,15 +601,13 @@ describe('MDC-based MatChipGrid', () => {

describe('with chip remove', () => {
let chipGrid: MatChipGrid;
let chipElements: DebugElement[];
let chipRemoveDebugElements: DebugElement[];

beforeEach(() => {
fixture = createComponent(ChipGridWithRemove);
fixture.detectChanges();

chipGrid = fixture.debugElement.query(By.directive(MatChipGrid))!.componentInstance;
chipElements = fixture.debugElement.queryAll(By.directive(MatChipRow));
chipRemoveDebugElements = fixture.debugElement.queryAll(By.directive(MatChipRemove));
chips = chipGrid._chips;
});
Expand All @@ -623,12 +620,6 @@ describe('MDC-based MatChipGrid', () => {
dispatchMouseEvent(chipRemoveDebugElements[2].nativeElement, 'click');
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipElements[2].nativeElement.dispatchEvent(fakeEvent);

fixture.detectChanges();

expect(chips.toArray()[2].value).not.toBe(2, 'Expected the third chip to be removed.');
expect(chipGrid._keyManager.activeRowIndex).toBe(2);
});
Expand Down Expand Up @@ -771,9 +762,10 @@ describe('MDC-based MatChipGrid', () => {

dispatchFakeEvent(nativeChips[0], 'focusout');
fixture.detectChanges();
tick();
fixture.detectChanges();
zone.simulateZoneExit();
fixture.detectChanges();
tick();
expect(formField.classList).not.toContain('mat-focused');
}));

Expand All @@ -787,10 +779,6 @@ describe('MDC-based MatChipGrid', () => {
chip.focus();
dispatchKeyboardEvent(chip, 'keydown', BACKSPACE);
fixture.detectChanges();
const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chip.dispatchEvent(fakeEvent);
fixture.detectChanges();
tick();
});

Expand Down
2 changes: 1 addition & 1 deletion src/material-experimental/mdc-chips/chip-grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ export class MatChipGrid extends _MatChipGridMixinBase implements AfterContentIn
const newChipIndex = Math.min(this._lastDestroyedChipIndex, this._chips.length - 1);
this._keyManager.setActiveCell({
row: newChipIndex,
column: this._keyManager.activeColumnIndex
column: Math.max(this._keyManager.activeColumnIndex, 0)
});
} else {
this.focus();
Expand Down
21 changes: 0 additions & 21 deletions src/material-experimental/mdc-chips/chip-remove.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
createFakeEvent,
dispatchKeyboardEvent,
createKeyboardEvent,
dispatchEvent,
Expand Down Expand Up @@ -55,18 +54,6 @@ describe('MDC-based Chip Remove', () => {
expect(buttonElement.hasAttribute('type')).toBe(false);
});

it('should start MDC exit animation on click', () => {
let buttonElement = chipNativeElement.querySelector('button')!;

testChip.removable = true;
fixture.detectChanges();

buttonElement.click();
fixture.detectChanges();

expect(chipNativeElement.classList.contains('mdc-chip--exit')).toBe(true);
});

it('should emit (removed) event when exit animation is complete', () => {
let buttonElement = chipNativeElement.querySelector('button')!;

Expand All @@ -77,10 +64,6 @@ describe('MDC-based Chip Remove', () => {
buttonElement.click();
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(testChip.didRemove).toHaveBeenCalled();
});

Expand Down Expand Up @@ -164,10 +147,6 @@ describe('MDC-based Chip Remove', () => {
dispatchKeyboardEvent(buttonElement, 'keydown', TAB);
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(testChip.didRemove).not.toHaveBeenCalled();
});

Expand Down
25 changes: 0 additions & 25 deletions src/material-experimental/mdc-chips/chip-row.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {Directionality} from '@angular/cdk/bidi';
import {BACKSPACE, DELETE, RIGHT_ARROW, ENTER} from '@angular/cdk/keycodes';
import {
createKeyboardEvent,
createFakeEvent,
dispatchEvent,
dispatchFakeEvent,
} from '@angular/cdk/testing/private';
Expand Down Expand Up @@ -97,10 +96,6 @@ describe('MDC-based Row Chips', () => {
chipInstance.remove();
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(testComponent.chipRemove).toHaveBeenCalledWith({chip: chipInstance});
});

Expand All @@ -127,10 +122,6 @@ describe('MDC-based Row Chips', () => {
chipInstance._keydown(DELETE_EVENT);
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(testComponent.chipRemove).toHaveBeenCalled();
});

Expand All @@ -142,10 +133,6 @@ describe('MDC-based Row Chips', () => {
chipInstance._keydown(BACKSPACE_EVENT);
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(testComponent.chipRemove).toHaveBeenCalled();
});

Expand All @@ -157,10 +144,6 @@ describe('MDC-based Row Chips', () => {
removeIconInstance.interaction.next(ARROW_KEY_EVENT);
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(testComponent.chipRemove).not.toHaveBeenCalled();
});
});
Expand All @@ -179,10 +162,6 @@ describe('MDC-based Row Chips', () => {
chipInstance._keydown(DELETE_EVENT);
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(testComponent.chipRemove).not.toHaveBeenCalled();
});

Expand All @@ -195,10 +174,6 @@ describe('MDC-based Row Chips', () => {
chipInstance._keydown(BACKSPACE_EVENT);
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(testComponent.chipRemove).not.toHaveBeenCalled();
});
});
Expand Down
22 changes: 17 additions & 5 deletions src/material-experimental/mdc-chips/chip-row.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ export class MatChipRow extends MatChip implements AfterContentInit, AfterViewIn
/** The focusable grid cells for this row. Implemented as part of GridKeyManagerRow. */
cells!: HTMLElement[];

/**
* Timeout used to give some time between `focusin` and `focusout`
* in order to determine whether focus has left the chip.
*/
private _focusoutTimeout: number | null;

constructor(
@Inject(DOCUMENT) private readonly _document: any,
changeDetectorRef: ChangeDetectorRef,
Expand Down Expand Up @@ -153,19 +159,25 @@ export class MatChipRow extends MatChip implements AfterContentInit, AfterViewIn
* to the other gridcell.
*/
_focusout(event: FocusEvent) {
this._hasFocusInternal = false;
if (this._focusoutTimeout) {
clearTimeout(this._focusoutTimeout);
}

// Wait to see if focus moves to the other gridcell
setTimeout(() => {
if (this._hasFocus()) {
return;
}
this._focusoutTimeout = setTimeout(() => {
this._hasFocusInternal = false;
this._onBlur.next({chip: this});
this._handleInteraction(event);
});
}

/** Records that the chip has focus when one of the gridcells is focused. */
_focusin(event: FocusEvent) {
if (this._focusoutTimeout) {
clearTimeout(this._focusoutTimeout);
this._focusoutTimeout = null;
}

this._hasFocusInternal = true;
this._handleInteraction(event);
}
Expand Down
16 changes: 0 additions & 16 deletions src/material-experimental/mdc-chips/chip.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {Directionality} from '@angular/cdk/bidi';
import {createFakeEvent} from '@angular/cdk/testing/private';
import {Component, DebugElement, ViewChild} from '@angular/core';
import {waitForAsync, ComponentFixture, TestBed} from '@angular/core/testing';
import {MatRipple} from '@angular/material-experimental/mdc-core';
Expand Down Expand Up @@ -135,24 +134,9 @@ describe('MDC-based MatChip', () => {
chipInstance.remove();
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(testComponent.chipRemove).toHaveBeenCalledWith({chip: chipInstance});
});

it('should make the chip non-focusable when it is removed', () => {
chipInstance.remove();
fixture.detectChanges();

const fakeEvent = createFakeEvent('transitionend');
(fakeEvent as any).propertyName = 'width';
chipNativeElement.dispatchEvent(fakeEvent);

expect(chipNativeElement.style.display).toBe('none');
});

it('should be able to disable ripples with the `[rippleDisabled]` input', () => {
expect(chipRippleInstance.disabled).toBe(false, 'Expected chip ripples to be enabled.');

Expand Down
37 changes: 8 additions & 29 deletions src/material-experimental/mdc-chips/chip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,6 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
}
protected _highlighted: boolean = false;

/** Emitted when the user interacts with the remove icon. */
@Output() removeIconInteraction = new EventEmitter<string>();

/** Emitted when the user interacts with the chip. */
@Output() interaction = new EventEmitter<string>();

Expand All @@ -231,9 +228,6 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
/** The unstyled chip selector for this component. */
protected basicChipAttrName = 'mat-basic-chip';

/** Subject that emits when the component has been destroyed. */
protected _destroyed = new Subject<void>();

/** The chip's leading icon. */
@ContentChild(MAT_CHIP_AVATAR) leadingIcon: MatChipAvatar;

Expand Down Expand Up @@ -276,17 +270,8 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
// input.
},
notifyNavigation: () => this._notifyNavigation(),
notifyTrailingIconInteraction: () =>
this.removeIconInteraction.emit(this.id),
notifyRemoval:
() => {
this.removed.emit({chip: this});

// When MDC removes a chip it just transitions it to `width: 0px`
// which means that it's still in the DOM and it's still focusable.
// Make it `display: none` so users can't tab into it.
this._elementRef.nativeElement.style.display = 'none';
},
notifyTrailingIconInteraction: () => {},
notifyRemoval: () => this.remove(),
notifyEditStart:
() => {
this._onEditStart();
Expand Down Expand Up @@ -375,24 +360,17 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte

ngOnDestroy() {
this.destroyed.emit({chip: this});
this._destroyed.next();
this._destroyed.complete();
this._chipFoundation.destroy();
}

/** Sets up the remove icon chip foundation, and subscribes to remove icon events. */
_initRemoveIcon() {
private _initRemoveIcon() {
if (this.removeIcon) {
this._chipFoundation.setShouldRemoveOnTrailingIconClick(true);
this._listenToRemoveIconInteraction();
this.removeIcon.disabled = this.disabled;
}
}

/** Handles interaction with the remove icon. */
_listenToRemoveIconInteraction() {
this.removeIcon.interaction
.pipe(takeUntil(this._destroyed))
this.removeIcon.interaction
.pipe(takeUntil(this.destroyed))
.subscribe(event => {
// The MDC chip foundation calls stopPropagation() for any trailing icon interaction
// event, even ones it doesn't handle, so we want to avoid passing it keyboard events
Expand All @@ -405,7 +383,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
return;
}

this._chipFoundation.handleTrailingActionInteraction();
this.remove();

if (isKeyboardEvent && !hasModifierKey(event as KeyboardEvent)) {
const keyCode = (event as KeyboardEvent).keyCode;
Expand All @@ -416,6 +394,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
}
}
});
}
}

/**
Expand All @@ -425,7 +404,7 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
*/
remove(): void {
if (this.removable) {
this._chipFoundation.beginExit();
this.removed.emit({chip: this});
}
}

Expand Down
8 changes: 1 addition & 7 deletions src/material-experimental/mdc-chips/chips.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,8 @@
cursor: default;

&._mat-animation-noopable {
// MDC's chip removal works by toggling a class on the chip, waiting for its transitions
// to finish and emitting the remove event at the end. The problem is that if our animations
// were disabled via the `NoopAnimationsModule`, the element won't have a transition and
// `transitionend` won't fire. We work around the issue by assigning a very short transition.
transition-duration: 1ms;

// Disables the chip enter animation.
animation: none;
transition: none;

.mdc-chip__checkmark-svg {
transition: none;
Expand Down
3 changes: 0 additions & 3 deletions src/material-experimental/mdc-chips/testing/chip-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ export class MatChipHarness extends ComponentHarness {
async remove(): Promise<void> {
const hostEl = await this.host();
await hostEl.sendKeys(TestKey.DELETE);

// @breaking-change 12.0.0 Remove non-null assertion from `dispatchEvent`.
await hostEl.dispatchEvent!('transitionend', {propertyName: 'width'});
}

/**
Expand Down