Skip to content

Commit

Permalink
fix(chips): focus not being restored correctly on chip removal when i…
Browse files Browse the repository at this point in the history
…nside component with animations (#12416)

Fixes the chip list losing its focus position if a chip is deleted while it's inside a component with animations.

Fixes #12374.
  • Loading branch information
crisbeto authored and jelbourn committed Aug 29, 2018
1 parent 2349166 commit d08d8bc
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 8 deletions.
80 changes: 75 additions & 5 deletions src/lib/chips/chip-list.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import {FocusKeyManager} from '@angular/cdk/a11y';
import {Directionality, Direction} from '@angular/cdk/bidi';
import {BACKSPACE, DELETE, ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE, TAB} from '@angular/cdk/keycodes';
import {createKeyboardEvent, dispatchFakeEvent, dispatchKeyboardEvent} from '@angular/cdk/testing';
import {
createKeyboardEvent,
dispatchFakeEvent,
dispatchKeyboardEvent,
MockNgZone,
} from '@angular/cdk/testing';
import {
Component,
DebugElement,
Expand All @@ -10,16 +15,18 @@ import {
ViewChildren,
Type,
Provider,
NgZone,
} from '@angular/core';
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
import {FormControl, FormsModule, NgForm, ReactiveFormsModule, Validators} from '@angular/forms';
import {MatFormFieldModule} from '@angular/material/form-field';
import {By} from '@angular/platform-browser';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {NoopAnimationsModule, BrowserAnimationsModule} from '@angular/platform-browser/animations';
import {MatInputModule} from '../input/index';
import {MatChip} from './chip';
import {MatChipInputEvent} from './chip-input';
import {MatChipList, MatChipsModule} from './index';
import {trigger, transition, style, animate} from '@angular/animations';


describe('MatChipList', () => {
Expand All @@ -30,6 +37,7 @@ describe('MatChipList', () => {
let testComponent: StandardChipList;
let chips: QueryList<any>;
let manager: FocusKeyManager<MatChip>;
let zone: MockNgZone;

describe('StandardChipList', () => {
describe('basic behaviors', () => {
Expand Down Expand Up @@ -189,6 +197,7 @@ describe('MatChipList', () => {
// Focus and blur the middle item
midItem.focus();
midItem._blur();
zone.simulateZoneExit();

// Destroy the middle item
testComponent.remove = 2;
Expand All @@ -197,6 +206,32 @@ describe('MatChipList', () => {
// Should not have focus
expect(chipListInstance._keyManager.activeItemIndex).toEqual(-1);
});

it('should move focus to the last chip when the focused chip was deleted inside a' +
'component with animations', fakeAsync(() => {
fixture.destroy();
TestBed.resetTestingModule();
fixture = createComponent(StandardChipListWithAnimations, [], BrowserAnimationsModule);
fixture.detectChanges();

chipListDebugElement = fixture.debugElement.query(By.directive(MatChipList));
chipListNativeElement = chipListDebugElement.nativeElement;
chipListInstance = chipListDebugElement.componentInstance;
testComponent = fixture.debugElement.componentInstance;
chips = chipListInstance.chips;

chips.last.focus();
fixture.detectChanges();

expect(chipListInstance._keyManager.activeItemIndex).toBe(chips.length - 1);

dispatchKeyboardEvent(chips.last._elementRef.nativeElement, 'keydown', BACKSPACE);
fixture.detectChanges();
tick(500);

expect(chipListInstance._keyManager.activeItemIndex).toBe(chips.length - 1);
}));

});
});

Expand Down Expand Up @@ -1053,18 +1088,23 @@ describe('MatChipList', () => {
});
});

function createComponent<T>(component: Type<T>, providers: Provider[] = []): ComponentFixture<T> {
function createComponent<T>(component: Type<T>, providers: Provider[] = [], animationsModule:
Type<NoopAnimationsModule> | Type<BrowserAnimationsModule> = NoopAnimationsModule):
ComponentFixture<T> {
TestBed.configureTestingModule({
imports: [
FormsModule,
ReactiveFormsModule,
MatChipsModule,
MatFormFieldModule,
MatInputModule,
NoopAnimationsModule,
animationsModule,
],
declarations: [component],
providers
providers: [
{provide: NgZone, useFactory: () => zone = new MockNgZone()},
...providers
]
}).compileComponents();

return TestBed.createComponent<T>(component);
Expand Down Expand Up @@ -1328,3 +1368,33 @@ class ChipListWithFormErrorMessages {
@ViewChild('form') form: NgForm;
formControl = new FormControl('', Validators.required);
}


@Component({
template: `
<mat-chip-list>
<mat-chip *ngFor="let i of numbers" (removed)="remove(i)">{{i}}</mat-chip>
</mat-chip-list>`,
animations: [
// For the case we're testing this animation doesn't
// have to be used anywhere, it just has to be defined.
trigger('dummyAnimation', [
transition(':leave', [
style({opacity: 0}),
animate('500ms', style({opacity: 1}))
])
])
]
})
class StandardChipListWithAnimations {
numbers = [0, 1, 2, 3, 4];

remove(item: number): void {
const index = this.numbers.indexOf(item);

if (index > -1) {
this.numbers.splice(index, 1);
}
}
}

15 changes: 12 additions & 3 deletions src/lib/chips/chip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
RippleTarget
} from '@angular/material/core';
import {Subject} from 'rxjs';
import {take} from 'rxjs/operators';


/** Represents an event fired on an individual `mat-chip`. */
Expand Down Expand Up @@ -218,14 +219,14 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
}

constructor(public _elementRef: ElementRef,
ngZone: NgZone,
private _ngZone: NgZone,
platform: Platform,
@Optional() @Inject(MAT_RIPPLE_GLOBAL_OPTIONS) globalOptions: RippleGlobalOptions) {
super(_elementRef);

this._addHostClassName();

this._chipRipple = new RippleRenderer(this, ngZone, _elementRef, platform);
this._chipRipple = new RippleRenderer(this, _ngZone, _elementRef, platform);
this._chipRipple.setupTriggerEvents(_elementRef.nativeElement);

if (globalOptions) {
Expand Down Expand Up @@ -359,7 +360,15 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
}

_blur(): void {
this._hasFocus = false;
// When animations are enabled, Angular may end up removing the chip from the DOM a little
// earlier than usual, causing it to be blurred and throwing off the logic in the chip list
// that moves focus not the next item. To work around the issue, we defer marking the chip
// as not focused until the next time the zone stabilizes.
this._ngZone.onStable
.asObservable()
.pipe(take(1))
.subscribe(() => this._hasFocus = false);

this._onBlur.next({chip: this});
}
}
Expand Down

0 comments on commit d08d8bc

Please sign in to comment.