Skip to content

Commit

Permalink
fix(sidenav): throw error when sidenav has 2 sidenavs on the same sid…
Browse files Browse the repository at this point in the history
…e at the same time (#3369)
  • Loading branch information
mmalerba authored and kara committed Mar 3, 2017
1 parent bdbda71 commit 324da5b
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 52 deletions.
9 changes: 6 additions & 3 deletions src/demo-app/sidenav/sidenav-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,13 @@ <h2>Sidenav Already Opened</h2>
<h2>Dynamic Alignment Sidenav</h2>

<md-sidenav-container class="demo-sidenav-container">
<md-sidenav #dynamicAlignSidenav mode="push" [align]="side">Drawer</md-sidenav>
<md-sidenav #dynamicAlignSidenav1 mode="side" [align]="invert ? 'end' : 'start'">Start</md-sidenav>
<md-sidenav #dynamicAlignSidenav2 mode="side" [align]="invert ? 'start' : 'end'">End</md-sidenav>

<div class="demo-sidenav-content">
<button (click)="dynamicAlignSidenav.toggle()">Toggle sidenav</button>
<button (click)="side = (side == 'start') ? 'end' : 'start'">Change sides</button>
<button (click)="dynamicAlignSidenav1.toggle(); dynamicAlignSidenav2.toggle()">
Toggle sidenavs
</button>
<button (click)="invert = !invert">Change sides</button>
</div>
</md-sidenav-container>
2 changes: 1 addition & 1 deletion src/demo-app/sidenav/sidenav-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ import {Component, ViewEncapsulation} from '@angular/core';
encapsulation: ViewEncapsulation.None,
})
export class SidenavDemo {
side = 'start';
invert = false;
}
4 changes: 0 additions & 4 deletions src/lib/sidenav/sidenav.scss
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,3 @@
}
}
}

.mat-sidenav-invalid {
display: none;
}
18 changes: 10 additions & 8 deletions src/lib/sidenav/sidenav.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});

Expand Down
45 changes: 9 additions & 36 deletions src/lib/sidenav/sidenav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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; }
Expand Down Expand Up @@ -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<MdSidenavToggleResult> {
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 ||
Expand Down Expand Up @@ -410,25 +392,20 @@ 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. */
private _setContainerClass(sidenav: MdSidenav, bool: boolean): void {
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;
Expand All @@ -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;
}
Expand All @@ -462,8 +437,6 @@ export class MdSidenavContainer implements AfterContentInit {
this._left = this._end;
this._right = this._start;
}

this._setDrawersValid(true);
}

_onBackdropClicked() {
Expand Down

0 comments on commit 324da5b

Please sign in to comment.