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

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 1 commit
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
42 changes: 17 additions & 25 deletions src/lib/sidenav/sidenav.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,27 +129,23 @@ describe('MdSidenav', () => {
let sidenav: MdSidenav = fixture.debugElement
.query(By.directive(MdSidenav)).componentInstance;

let openCalled = false;
let openCancelled = false;
let closeCalled = false;
let openFinished: boolean;
let closeFinished: boolean;

sidenav.open().then(() => {
openCalled = true;
}, () => {
openCancelled = true;
sidenav.open().then((finished) => {
openFinished = finished;
});

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

endSidenavTransition(fixture);
tick();

expect(openCalled).toBe(false);
expect(openCancelled).toBe(true);
expect(closeCalled).toBe(true);
expect(openFinished).toBe(false);
expect(closeFinished).toBe(true);
tick();
}));

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

let closeCalled = false;
let closeCancelled = false;
let openCalled = false;
let closeFinished: boolean;
let openFinished: boolean;

// 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((finished) => {
closeFinished = finished;
});
// We do not call transition end, open directly.
sidenav.open().then(() => {
openCalled = true;
sidenav.open().then((finished) => {
openFinished = finished;
});

endSidenavTransition(fixture);
tick();

expect(closeCalled).toBe(false);
expect(closeCancelled).toBe(true);
expect(openCalled).toBe(true);
expect(closeFinished).toBe(false);
expect(openFinished).toBe(true);
tick();
}));

Expand Down Expand Up @@ -219,7 +211,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
90 changes: 31 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 @@ -106,6 +106,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<boolean> = null;

/**
* The current toggle animation promise resolution function.
* `null` if no animation is in progress.
*/
private _toggleAnimationPromiseResolve: (value: 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 +124,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._toggleAnimationPromiseResolve(true);
this._toggleAnimationPromise = this._toggleAnimationPromiseResolve = null;
}
}

Expand All @@ -134,15 +143,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<boolean> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than emitting a boolean directly, can we create an event object and include this information? That lets us add additional information to the event in the future.

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<boolean> {
return this.toggle(false);
}

Expand All @@ -151,47 +160,31 @@ 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<boolean> {
if (!this.valid) { return Promise.resolve(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(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._toggleAnimationPromiseResolve(false);
}
this._toggleAnimationPromise = new Promise<boolean>(resolve => {
this._toggleAnimationPromiseResolve = resolve;
});
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 +194,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._toggleAnimationPromiseResolve(true);
this._toggleAnimationPromise = this._toggleAnimationPromiseResolve = 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 +238,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