From dd2a0400db78f11c6c7c746be40784d63d119531 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Wed, 1 Mar 2017 15:09:54 -0800 Subject: [PATCH] fix(sidenav): throw error when sidenav has 2 sidenavs on the same side at the same time --- src/demo-app/sidenav/sidenav-demo.html | 9 ++++-- src/demo-app/sidenav/sidenav-demo.ts | 2 +- src/lib/sidenav/sidenav.scss | 4 --- src/lib/sidenav/sidenav.spec.ts | 18 ++++++----- src/lib/sidenav/sidenav.ts | 45 ++++++-------------------- 5 files changed, 26 insertions(+), 52 deletions(-) diff --git a/src/demo-app/sidenav/sidenav-demo.html b/src/demo-app/sidenav/sidenav-demo.html index 3330798e1ce6..3b362b3006d6 100644 --- a/src/demo-app/sidenav/sidenav-demo.html +++ b/src/demo-app/sidenav/sidenav-demo.html @@ -51,10 +51,13 @@

Sidenav Already Opened

Dynamic Alignment Sidenav

- Drawer + Start + End
- - + +
diff --git a/src/demo-app/sidenav/sidenav-demo.ts b/src/demo-app/sidenav/sidenav-demo.ts index ed5b512679d2..5d72a469a5df 100644 --- a/src/demo-app/sidenav/sidenav-demo.ts +++ b/src/demo-app/sidenav/sidenav-demo.ts @@ -9,5 +9,5 @@ import {Component, ViewEncapsulation} from '@angular/core'; encapsulation: ViewEncapsulation.None, }) export class SidenavDemo { - side = 'start'; + invert = false; } diff --git a/src/lib/sidenav/sidenav.scss b/src/lib/sidenav/sidenav.scss index 273299f5fcb0..4f70dff0c874 100644 --- a/src/lib/sidenav/sidenav.scss +++ b/src/lib/sidenav/sidenav.scss @@ -135,7 +135,3 @@ } } } - -.mat-sidenav-invalid { - display: none; -} diff --git a/src/lib/sidenav/sidenav.spec.ts b/src/lib/sidenav/sidenav.spec.ts index d2f3ca68c75d..31a59669134e 100644 --- a/src/lib/sidenav/sidenav.spec.ts +++ b/src/lib/sidenav/sidenav.spec.ts @@ -373,23 +373,25 @@ describe('MdSidenav', () => { .toBe(false, 'Expected sidenav not to have a native align attribute.'); }); - it('should mark sidenavs invalid when multiple have same align', () => { + it('should throw when multiple sidenavs have the same align', () => { const fixture = TestBed.createComponent(SidenavDynamicAlign); fixture.detectChanges(); const testComponent: SidenavDynamicAlign = fixture.debugElement.componentInstance; - const sidenavEl = fixture.debugElement.query(By.css('md-sidenav')).nativeElement; - expect(sidenavEl.classList).not.toContain('mat-sidenav-invalid'); - testComponent.sidenav1Align = 'end'; - fixture.detectChanges(); - expect(sidenavEl.classList).toContain('mat-sidenav-invalid'); + expect(() => fixture.detectChanges()).toThrow(); + }); - testComponent.sidenav2Align = 'start'; + it('should not throw when sidenavs swap sides', () => { + const fixture = TestBed.createComponent(SidenavDynamicAlign); fixture.detectChanges(); - expect(sidenavEl.classList).not.toContain('mat-sidenav-invalid'); + const testComponent: SidenavDynamicAlign = fixture.debugElement.componentInstance; + testComponent.sidenav1Align = 'end'; + testComponent.sidenav2Align = 'start'; + + expect(() => fixture.detectChanges()).not.toThrow(); }); }); diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts index 86f9fcf111d4..c81e797d9224 100644 --- a/src/lib/sidenav/sidenav.ts +++ b/src/lib/sidenav/sidenav.ts @@ -65,7 +65,6 @@ export class MdSidenavToggleResult { '[class.mat-sidenav-over]': '_modeOver', '[class.mat-sidenav-push]': '_modePush', '[class.mat-sidenav-side]': '_modeSide', - '[class.mat-sidenav-invalid]': '!valid', 'tabIndex': '-1' }, changeDetection: ChangeDetectionStrategy.OnPush, @@ -77,19 +76,6 @@ export class MdSidenav implements AfterContentInit, OnDestroy { /** Alignment of the sidenav (direction neutral); whether 'start' or 'end'. */ private _align: 'start' | 'end' = 'start'; - /** Whether this md-sidenav is part of a valid md-sidenav-container configuration. */ - get valid() { return this._valid; } - set valid(value) { - value = coerceBooleanProperty(value); - // When the drawers are not in a valid configuration we close them all until they are in a valid - // configuration again. - if (!value) { - this.close(); - } - this._valid = value; - } - private _valid = true; - /** Direction which the sidenav is aligned in. */ @Input() get align() { return this._align; } @@ -221,10 +207,6 @@ export class MdSidenav implements AfterContentInit, OnDestroy { * @returns Resolves with the result of whether the sidenav was opened or closed. */ toggle(isOpen: boolean = !this.opened): Promise { - if (!this.valid) { - return Promise.resolve(new MdSidenavToggleResult(isOpen ? 'open' : 'close', true)); - } - // Shortcut it if we're already opened. if (isOpen === this.opened) { return this._toggleAnimationPromise || @@ -410,8 +392,13 @@ export class MdSidenavContainer implements AfterContentInit { * changes. */ private _watchSidenavAlign(sidenav: MdSidenav): void { - if (!sidenav) { return; } - sidenav.onAlignChanged.subscribe(() => this._validateDrawers()); + if (!sidenav) { + return; + } + // 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(() => + this._ngZone.onMicrotaskEmpty.first().subscribe(() => this._validateDrawers())); } /** Toggles the 'mat-sidenav-opened' class on the main 'md-sidenav-container' element. */ @@ -419,16 +406,6 @@ export class MdSidenavContainer implements AfterContentInit { this._renderer.setElementClass(this._element.nativeElement, 'mat-sidenav-opened', bool); } - /** Sets the valid state of the drawers. */ - private _setDrawersValid(valid: boolean) { - this._sidenavs.forEach((sidenav) => { - sidenav.valid = valid; - }); - if (!valid) { - this._start = this._end = this._left = this._right = null; - } - } - /** Validate the state of the sidenav children components. */ private _validateDrawers() { this._start = this._end = null; @@ -439,14 +416,12 @@ export class MdSidenavContainer implements AfterContentInit { for (let sidenav of this._sidenavs.toArray()) { if (sidenav.align == 'end') { if (this._end != null) { - this._setDrawersValid(false); - return; + throw new MdDuplicatedSidenavError('end'); } this._end = sidenav; } else { if (this._start != null) { - this._setDrawersValid(false); - return; + throw new MdDuplicatedSidenavError('start'); } this._start = sidenav; } @@ -462,8 +437,6 @@ export class MdSidenavContainer implements AfterContentInit { this._left = this._end; this._right = this._start; } - - this._setDrawersValid(true); } _onBackdropClicked() {