From 0d80a7732caa36f5c34279e8870cdd219bacc0f2 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 2 Aug 2017 00:05:49 +0200 Subject: [PATCH] fix(sidenav): container not reacting to changes to sidenavs added after init (#6161) Fixes the sidenav container not reacting to sidenavs that were added after initialization. Fixes #6127. --- src/lib/sidenav/sidenav.spec.ts | 38 ++++++++++++++++++++++++++++++--- src/lib/sidenav/sidenav.ts | 34 +++++++++++++++-------------- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/lib/sidenav/sidenav.spec.ts b/src/lib/sidenav/sidenav.spec.ts index 55e8f31dd6c..ee2bf7ce540 100644 --- a/src/lib/sidenav/sidenav.spec.ts +++ b/src/lib/sidenav/sidenav.spec.ts @@ -313,7 +313,7 @@ describe('MdSidenavContainer', () => { beforeEach(async(() => { TestBed.configureTestingModule({ imports: [MdSidenavModule, A11yModule, PlatformModule, NoopAnimationsModule], - declarations: [SidenavContainerTwoSidenavTestApp], + declarations: [SidenavContainerTwoSidenavTestApp, SidenavDelayed], }); TestBed.compileComponents(); @@ -342,6 +342,26 @@ describe('MdSidenavContainer', () => { expect(sidenavs.every(sidenav => sidenav.componentInstance.opened)).toBe(false); })); + + it('should animate the content when a sidenav is added at a later point', fakeAsync(() => { + const fixture = TestBed.createComponent(SidenavDelayed); + + fixture.detectChanges(); + + const contentElement = fixture.debugElement.nativeElement.querySelector('.mat-sidenav-content'); + + expect(parseInt(contentElement.style.marginLeft)).toBe(0); + + fixture.componentInstance.showSidenav = true; + fixture.detectChanges(); + + fixture.componentInstance.sidenav.open(); + fixture.detectChanges(); + tick(); + fixture.detectChanges(); + + expect(parseInt(contentElement.style.marginLeft)).toBeGreaterThan(0); + })); }); @@ -358,8 +378,7 @@ class SidenavContainerNoSidenavTestApp { } `, }) class SidenavContainerTwoSidenavTestApp { - @ViewChild(MdSidenavContainer) - sidenavContainer: MdSidenavContainer; + @ViewChild(MdSidenavContainer) sidenavContainer: MdSidenavContainer; } /** Test component that contains an MdSidenavContainer and one MdSidenav. */ @@ -441,3 +460,16 @@ class SidenavDynamicAlign { class SidenavWitFocusableElements { mode: string = 'over'; } + + +@Component({ + template: ` + + Sidenav + + `, +}) +class SidenavDelayed { + @ViewChild(MdSidenav) sidenav: MdSidenav; + showSidenav = false; +} diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts index 884985a3d53..47eb18bf772 100644 --- a/src/lib/sidenav/sidenav.ts +++ b/src/lib/sidenav/sidenav.ts @@ -28,8 +28,9 @@ import {animate, state, style, transition, trigger, AnimationEvent} from '@angul import {Directionality, coerceBooleanProperty} from '../core'; import {FocusTrapFactory, FocusTrap} from '../core/a11y/focus-trap'; import {ESCAPE} from '../core/keyboard/keycodes'; -import {first} from '../core/rxjs/index'; +import {first, takeUntil, startWith} from '../core/rxjs/index'; import {DOCUMENT} from '@angular/platform-browser'; +import {merge} from 'rxjs/observable/merge'; /** Throws an exception when two MdSidenav are matching the same side. */ @@ -336,13 +337,13 @@ export class MdSidenavContainer implements AfterContentInit { } ngAfterContentInit() { - // On changes, assert on consistency. - this._sidenavs.changes.subscribe(() => this._validateDrawers()); - this._sidenavs.forEach((sidenav: MdSidenav) => { - this._watchSidenavToggle(sidenav); - this._watchSidenavAlign(sidenav); + startWith.call(this._sidenavs.changes, null).subscribe(() => { + this._validateDrawers(); + this._sidenavs.forEach((sidenav: MdSidenav) => { + this._watchSidenavToggle(sidenav); + this._watchSidenavAlign(sidenav); + }); }); - this._validateDrawers(); } /** Calls `open` of both start and end sidenavs */ @@ -361,16 +362,17 @@ export class MdSidenavContainer implements AfterContentInit { * is properly hidden. */ private _watchSidenavToggle(sidenav: MdSidenav): void { - sidenav._animationStarted.subscribe(() => { - // Set the transition class on the container so that the animations occur. This should not - // be set initially because animations should only be triggered via a change in state. - this._renderer.addClass(this._element.nativeElement, 'mat-sidenav-transition'); - this._changeDetectorRef.markForCheck(); - }); + takeUntil.call(sidenav._animationStarted, this._sidenavs.changes) + .subscribe(() => { + // Set the transition class on the container so that the animations occur. This should not + // be set initially because animations should only be triggered via a change in state. + this._renderer.addClass(this._element.nativeElement, 'mat-sidenav-transition'); + this._changeDetectorRef.markForCheck(); + }); if (sidenav.mode !== 'side') { - sidenav.onOpen.subscribe(() => this._setContainerClass(true)); - sidenav.onClose.subscribe(() => this._setContainerClass(false)); + takeUntil.call(merge(sidenav.onOpen, sidenav.onClose), this._sidenavs.changes).subscribe(() => + this._setContainerClass(sidenav.opened)); } } @@ -384,7 +386,7 @@ export class MdSidenavContainer implements AfterContentInit { } // NOTE: We need to wait for the microtask queue to be empty before validating, // since both drawers may be swapping sides at the same time. - sidenav.onAlignChanged.subscribe(() => + takeUntil.call(sidenav.onAlignChanged, this._sidenavs.changes).subscribe(() => first.call(this._ngZone.onMicrotaskEmpty).subscribe(() => this._validateDrawers())); }