fix(drawer): infinite loop when two-way opened binding is toggled mid-animation (#8872)

As of https://github.com/angular/material2/commit/004e0fef2fe5172b6e6ed2c4d368d21dab051355 the drawer supports a two-way binding on the `opened` property, however because the `openedChange` emits at the end of the animation and two-way bindings invoke their setter which then trigger an animation which in turn invokes `openedChange`, there is the potential for an infinite loop if the user changes the `opened` state mid-animation. These changes avoid the issue by turning the `openedChanged` into an async stream and emitting the current value at the end of the animation, rather than determining the value based on the animation state.

Fixes #8869.
This commit is contained in:
Kristiyan Kostadinov
2017-12-13 22:52:03 +01:00
committed by Andrew Seguin
parent 50967b6f7c
commit 7c84490df9
2 changed files with 40 additions and 6 deletions
+34 -1
View File
@@ -8,7 +8,7 @@ import {
} from '@angular/core/testing';
import {Component, ElementRef, ViewChild} from '@angular/core';
import {By} from '@angular/platform-browser';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations';
import {MatDrawer, MatSidenavModule, MatDrawerContainer} from './index';
import {A11yModule} from '@angular/cdk/a11y';
import {PlatformModule} from '@angular/cdk/platform';
@@ -139,6 +139,7 @@ describe('MatDrawer', () => {
expect(testComponent.openCount).toBe(0);
expect(testComponent.closeCount).toBe(0);
tick();
fixture.debugElement.query(By.css('.close')).nativeElement.click();
fixture.detectChanges();
@@ -368,6 +369,37 @@ describe('MatDrawer', () => {
expect(fixture.componentInstance.isOpen).toBe(true);
}));
it('should not throw when a two-way binding is toggled quickly while animating',
fakeAsync(() => {
TestBed
.resetTestingModule()
.configureTestingModule({
imports: [MatSidenavModule, BrowserAnimationsModule],
declarations: [DrawerOpenBinding],
})
.compileComponents();
const fixture = TestBed.createComponent(DrawerOpenBinding);
fixture.detectChanges();
// Note that we need actual timeouts and the `BrowserAnimationsModule`
// in order to test it correctly.
setTimeout(() => {
fixture.componentInstance.isOpen = !fixture.componentInstance.isOpen;
expect(() => fixture.detectChanges()).not.toThrow();
setTimeout(() => {
fixture.componentInstance.isOpen = !fixture.componentInstance.isOpen;
expect(() => fixture.detectChanges()).not.toThrow();
}, 1);
tick(1);
}, 1);
tick(1);
}));
});
describe('focus trapping behavior', () => {
@@ -512,6 +544,7 @@ describe('MatDrawerContainer', () => {
fixture.componentInstance.renderDrawer = false;
fixture.detectChanges();
tick();
expect(parseInt(contentElement.style.marginLeft)).toBeLessThan(initialMargin);
}));
+6 -5
View File
@@ -195,7 +195,9 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
_animationState: 'open-instant' | 'open' | 'void' = 'void';
/** Event emitted when the drawer open state is changed. */
@Output() openedChange: EventEmitter<boolean> = new EventEmitter<boolean>();
@Output() openedChange: EventEmitter<boolean> =
// Note this has to be async in order to avoid some issues with two-bindings (see #8872).
new EventEmitter<boolean>(/* isAsync */true);
/** Event emitted when the drawer has been opened. */
@Output('opened')
@@ -402,10 +404,9 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
_onAnimationEnd(event: AnimationEvent) {
const {fromState, toState} = event;
if (toState.indexOf('open') === 0 && fromState === 'void') {
this.openedChange.emit(true);
} else if (toState === 'void' && fromState.indexOf('open') === 0) {
this.openedChange.emit(false);
if ((toState.indexOf('open') === 0 && fromState === 'void') ||
(toState === 'void' && fromState.indexOf('open') === 0)) {
this.openedChange.emit(this._opened);
}
}