Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(sidenav): resolve promise as false rather than #1930

Merged
merged 2 commits into from
Nov 30, 2016
Merged

fix(sidenav): resolve promise as false rather than #1930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a2b7a1
fix(sidenav): resolve promise as false rather than
mmalerba Nov 18, 2016
dbf585a
use a result object instead of boolean as promise result
mmalerba Nov 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 22 additions & 26 deletions src/lib/sidenav/sidenav.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {fakeAsync, async, tick, ComponentFixture, TestBed} from '@angular/core/testing';
import {Component} from '@angular/core';
import {By} from '@angular/platform-browser';
import {MdSidenav, MdSidenavModule} from './sidenav';
import {MdSidenav, MdSidenavModule, MdSidenavToggleResult} from './sidenav';


function endSidenavTransition(fixture: ComponentFixture<any>) {
Expand Down Expand Up @@ -129,27 +129,25 @@ describe('MdSidenav', () => {
let sidenav: MdSidenav = fixture.debugElement
.query(By.directive(MdSidenav)).componentInstance;

let openCalled = false;
let openCancelled = false;
let closeCalled = false;
let openResult: MdSidenavToggleResult;
let closeResult: MdSidenavToggleResult;

sidenav.open().then(() => {
openCalled = true;
}, () => {
openCancelled = true;
sidenav.open().then((result) => {
openResult = result;
});

// We do not call transition end, close directly.
sidenav.close().then(() => {
closeCalled = true;
sidenav.close().then((result) => {
closeResult = result;
});

endSidenavTransition(fixture);
tick();

expect(openCalled).toBe(false);
expect(openCancelled).toBe(true);
expect(closeCalled).toBe(true);
expect(openResult.type).toBe('open');
expect(openResult.animationFinished).toBe(false);
expect(closeResult.type).toBe('close');
expect(closeResult.animationFinished).toBe(true);
tick();
}));

Expand All @@ -158,32 +156,30 @@ describe('MdSidenav', () => {
let sidenav: MdSidenav = fixture.debugElement
.query(By.directive(MdSidenav)).componentInstance;

let closeCalled = false;
let closeCancelled = false;
let openCalled = false;
let closeResult: MdSidenavToggleResult;
let openResult: MdSidenavToggleResult;

// First, open the sidenav completely.
sidenav.open();
endSidenavTransition(fixture);
tick();

// Then close and check behavior.
sidenav.close().then(() => {
closeCalled = true;
}, () => {
closeCancelled = true;
sidenav.close().then((result) => {
closeResult = result;
});
// We do not call transition end, open directly.
sidenav.open().then(() => {
openCalled = true;
sidenav.open().then((result) => {
openResult = result;
});

endSidenavTransition(fixture);
tick();

expect(closeCalled).toBe(false);
expect(closeCancelled).toBe(true);
expect(openCalled).toBe(true);
expect(closeResult.type).toBe('close');
expect(closeResult.animationFinished).toBe(false);
expect(openResult.type).toBe('open');
expect(openResult.animationFinished).toBe(true);
tick();
}));

Expand Down Expand Up @@ -219,7 +215,7 @@ describe('MdSidenav', () => {
expect(sidenavEl.classList).not.toContain('md-sidenav-closed');
expect(sidenavEl.classList).toContain('md-sidenav-opened');

expect((testComponent as any)._openPromise).toBeNull();
expect((testComponent as any)._toggleAnimationPromise).toBeNull();
});

it('should remove align attr from DOM', () => {
Expand Down
101 changes: 42 additions & 59 deletions src/lib/sidenav/sidenav.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ export class MdDuplicatedSidenavError extends MdError {
}
}


/** Sidenav toggle promise result. */
export class MdSidenavToggleResult {
constructor(public type: 'open' | 'close', public animationFinished: boolean) {}
}


/**
* <md-sidenav> component.
*
Expand Down Expand Up @@ -106,6 +113,15 @@ export class MdSidenav implements AfterContentInit {
/** Event emitted when the sidenav alignment changes. */
@Output('align-changed') onAlignChanged = new EventEmitter<void>();

/** The current toggle animation promise. `null` if no animation is in progress. */
private _toggleAnimationPromise: Promise<MdSidenavToggleResult> = null;

/**
* The current toggle animation promise resolution function.
* `null` if no animation is in progress.
*/
private _resolveToggleAnimationPromise: (animationFinished: boolean) => void = null;

/**
* @param _elementRef The DOM element reference. Used for transition and width calculation.
* If not available we do not hook on transitions.
Expand All @@ -115,9 +131,9 @@ export class MdSidenav implements AfterContentInit {
ngAfterContentInit() {
// This can happen when the sidenav is set to opened in the template and the transition
// isn't ended.
if (this._openPromise) {
this._openPromiseResolve();
this._openPromise = null;
if (this._toggleAnimationPromise) {
this._resolveToggleAnimationPromise(true);
this._toggleAnimationPromise = this._resolveToggleAnimationPromise = null;
}
}

Expand All @@ -134,15 +150,15 @@ export class MdSidenav implements AfterContentInit {

/** Open this sidenav, and return a Promise that will resolve when it's fully opened (or get
* rejected if it didn't). */
open(): Promise<void> {
open(): Promise<MdSidenavToggleResult> {
return this.toggle(true);
}

/**
* Close this sidenav, and return a Promise that will resolve when it's fully closed (or get
* rejected if it didn't).
*/
close(): Promise<void> {
close(): Promise<MdSidenavToggleResult> {
return this.toggle(false);
}

Expand All @@ -151,47 +167,35 @@ export class MdSidenav implements AfterContentInit {
* close() when it's closed.
* @param isOpen
*/
toggle(isOpen: boolean = !this.opened): Promise<void> {
if (!this.valid) { return Promise.resolve(null); }
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) {
if (!this._transition) {
return Promise.resolve(null);
} else {
return isOpen ? this._openPromise : this._closePromise;
}
return this._toggleAnimationPromise ||
Promise.resolve(new MdSidenavToggleResult(isOpen ? 'open' : 'close', true));
}

this._opened = isOpen;
this._transition = true;

if (isOpen) {
this.onOpenStart.emit();
} else {
this.onCloseStart.emit();
}

if (isOpen) {
if (this._openPromise == null) {
this._openPromise = new Promise<void>((resolve, reject) => {
this._openPromiseResolve = resolve;
this._openPromiseReject = reject;
});
}
return this._openPromise;
} else {
if (this._closePromise == null) {
this._closePromise = new Promise<void>((resolve, reject) => {
this._closePromiseResolve = resolve;
this._closePromiseReject = reject;
});
}
return this._closePromise;
if (this._toggleAnimationPromise) {
this._resolveToggleAnimationPromise(false);
}
this._toggleAnimationPromise = new Promise<MdSidenavToggleResult>(resolve => {
this._resolveToggleAnimationPromise = animationFinished =>
resolve(new MdSidenavToggleResult(isOpen ? 'open' : 'close', animationFinished));
});
return this._toggleAnimationPromise;
}


/**
* When transition has finished, set the internal state for classes and emit the proper event.
* The event passed is actually of type TransitionEvent, but that type is not available in
Expand All @@ -201,43 +205,30 @@ export class MdSidenav implements AfterContentInit {
if (transitionEvent.target == this._elementRef.nativeElement
// Simpler version to check for prefixes.
&& transitionEvent.propertyName.endsWith('transform')) {
this._transition = false;
if (this._opened) {
if (this._openPromise != null) {
this._openPromiseResolve();
}
if (this._closePromise != null) {
this._closePromiseReject();
}

this.onOpen.emit();
} else {
if (this._closePromise != null) {
this._closePromiseResolve();
}
if (this._openPromise != null) {
this._openPromiseReject();
}

this.onClose.emit();
}

this._openPromise = null;
this._closePromise = null;
if (this._toggleAnimationPromise) {
this._resolveToggleAnimationPromise(true);
this._toggleAnimationPromise = this._resolveToggleAnimationPromise = null;
}
}
}

get _isClosing() {
return !this._opened && this._transition;
return !this._opened && !!this._toggleAnimationPromise;
}
get _isOpening() {
return this._opened && this._transition;
return this._opened && !!this._toggleAnimationPromise;
}
get _isClosed() {
return !this._opened && !this._transition;
return !this._opened && !this._toggleAnimationPromise;
}
get _isOpened() {
return this._opened && !this._transition;
return this._opened && !this._toggleAnimationPromise;
}
get _isEnd() {
return this.align == 'end';
Expand All @@ -258,14 +249,6 @@ export class MdSidenav implements AfterContentInit {
}
return 0;
}

private _transition: boolean = false;
private _openPromise: Promise<void>;
private _openPromiseResolve: () => void;
private _openPromiseReject: () => void;
private _closePromise: Promise<void>;
private _closePromiseResolve: () => void;
private _closePromiseReject: () => void;
}

/**
Expand Down