From 4009fb965dc5ce157c5bccaa6a879e29ef218712 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 1 Mar 2016 13:54:39 -0800 Subject: [PATCH] fix(): remove all async/waiting from the sidenav specs. --- .../sidenav/sidenav-transitions.scss | 27 ++ src/components/sidenav/sidenav.scss | 10 - src/components/sidenav/sidenav.spec.ts | 415 ++++++++---------- src/components/sidenav/sidenav.ts | 16 +- test/browser-providers.ts | 2 +- 5 files changed, 226 insertions(+), 244 deletions(-) create mode 100644 src/components/sidenav/sidenav-transitions.scss diff --git a/src/components/sidenav/sidenav-transitions.scss b/src/components/sidenav/sidenav-transitions.scss new file mode 100644 index 00000000000..691171a5702 --- /dev/null +++ b/src/components/sidenav/sidenav-transitions.scss @@ -0,0 +1,27 @@ +/** + * We separate transitions to be able to disable them in unit tests, by simply not loading this file. + */ +@import "variables"; + + +:host { + transition: margin-left $swift-ease-out-duration $swift-ease-out-timing-function, + margin-right $swift-ease-out-duration $swift-ease-out-timing-function; + + & > .md-sidenav-backdrop { + &.md-sidenav-shown { + transition: background-color $swift-ease-out-duration $swift-ease-out-timing-function; + } + } + + & > md-content { + transition: margin-left $swift-ease-out-duration $swift-ease-out-timing-function, + margin-right $swift-ease-out-duration $swift-ease-out-timing-function, + left $swift-ease-out-duration $swift-ease-out-timing-function, + right $swift-ease-out-duration $swift-ease-out-timing-function; + } + + > md-sidenav { + transition: transform $swift-ease-out-duration $swift-ease-out-timing-function; + } +} diff --git a/src/components/sidenav/sidenav.scss b/src/components/sidenav/sidenav.scss index 86cc986646d..b1836d695a4 100644 --- a/src/components/sidenav/sidenav.scss +++ b/src/components/sidenav/sidenav.scss @@ -58,9 +58,6 @@ $md-sidenav-push-background-color: md-color($md-background, dialog) !default; // Hide the sidenavs when they're closed. overflow-x: hidden; - transition: margin-left $swift-ease-out-duration $swift-ease-out-timing-function, - margin-right $swift-ease-out-duration $swift-ease-out-timing-function; - & > .md-sidenav-backdrop { position: absolute; top: 0; @@ -83,7 +80,6 @@ $md-sidenav-push-background-color: md-color($md-background, dialog) !default; &.md-sidenav-shown { visibility: visible; background-color: $md-sidenav-backdrop-color; - transition: background-color $swift-ease-out-duration $swift-ease-out-timing-function; } } @@ -91,10 +87,6 @@ $md-sidenav-push-background-color: md-color($md-background, dialog) !default; @include md-stacking-context(); display: block; - transition: margin-left $swift-ease-out-duration $swift-ease-out-timing-function, - margin-right $swift-ease-out-duration $swift-ease-out-timing-function, - left $swift-ease-out-duration $swift-ease-out-timing-function, - right $swift-ease-out-duration $swift-ease-out-timing-function; } > md-sidenav { @@ -108,8 +100,6 @@ $md-sidenav-push-background-color: md-color($md-background, dialog) !default; background-color: $md-sidenav-background-color; - transition: transform $swift-ease-out-duration $swift-ease-out-timing-function; - @include md-sidenav-transition(0, -100%); &.md-sidenav-push { diff --git a/src/components/sidenav/sidenav.spec.ts b/src/components/sidenav/sidenav.spec.ts index c2c8c533f1e..84ec2a56d54 100644 --- a/src/components/sidenav/sidenav.spec.ts +++ b/src/components/sidenav/sidenav.spec.ts @@ -1,10 +1,12 @@ import { - inject, - injectAsync, ComponentFixture, TestComponentBuilder, - beforeEachProviders, + fakeAsync, + inject, + injectAsync, + tick } from 'angular2/testing'; +import {XHR} from 'angular2/src/compiler/xhr'; import { it, iit, @@ -16,7 +18,11 @@ import { import { getComputedStyle } from '../../core/facade/html'; -import {provide, Component, DebugElement} from 'angular2/core'; +import { + Component, + ViewMetadata +} from 'angular2/core'; + import {By} from 'angular2/platform/browser'; import {Predicate} from 'angular2/src/facade/collection'; import {PromiseWrapper} from 'angular2/src/facade/promise'; @@ -24,262 +30,202 @@ import {TimerWrapper} from 'angular2/src/facade/async'; import {MdSidenav, MdSidenavLayout, MD_SIDENAV_DIRECTIVES} from './sidenav'; -function wait(msec: number) { - let completer = PromiseWrapper.completer(); - TimerWrapper.setTimeout(() => { - completer.resolve(null); - }, msec); - return completer.promise; +function fakeAsyncAdaptor(fn: () => void) { + return inject([], fakeAsync(fn)); } -function waitOnEvent(fixture: ComponentFixture, - by: Predicate, - propertyName: string) { - fixture.detectChanges(); +/** + * Create a ComponentFixture from the builder. This takes a template and a style for sidenav. + */ +function createFixture(builder: TestComponentBuilder, template: string, style: string): ComponentFixture { + let fixture: ComponentFixture = null; + // Remove the styles (which remove the animations/transitions). + builder + .overrideView(MdSidenavLayout, new ViewMetadata({ + template: template, + styles: [style], + directives: [MdSidenav], + })) + .createAsync(BasicTestApp).then((f: ComponentFixture) => { fixture = f; }); + tick(); - // Wait for the animation end. - let completer = PromiseWrapper.completer(); - let component: any = fixture.debugElement.query(by).componentInstance; - component[propertyName].subscribe(() => { - completer.resolve(null); + return fixture; +} + + +function endSidenavTransition(fixture: ComponentFixture) { + let sidenav: any = fixture.debugElement.query(By.directive(MdSidenav)).componentInstance; + sidenav.onTransitionEnd({ + target: (sidenav).elementRef_.nativeElement, + propertyName: 'transform' }); - return completer.promise; + fixture.detectChanges(); } export function main() { describe('MdSidenav', () => { + let template: string; + let style: string; let builder: TestComponentBuilder; - beforeEach(inject([TestComponentBuilder], (tcb: TestComponentBuilder) => { + /** + * We need to get the template and styles for the sidenav in an Async test. FakeAsync would block indefinitely + * on the XHR if we were to create the component async-ly. See https://github.com/angular/angular/issues/5601. + * We do some style verification so styles have to match. But we remove the transitions so we only + * set the regular `sidenav.css` styling. + */ + beforeEach(injectAsync([TestComponentBuilder, XHR], (tcb: TestComponentBuilder, xhr: XHR) => { builder = tcb; + + return Promise.all([ + xhr.get('./components/sidenav/sidenav.html').then((t) => { template = t; }), + xhr.get('./components/sidenav/sidenav.css').then((css) => { style = css; }) + ]).catch((err: any) => { + console.error(err); + }); })); describe('methods', () => { - it('should be able to open and close', (done: any) => { - let testComponent: BasicTestApp; - let fixture: ComponentFixture; + it('should be able to open and close', fakeAsyncAdaptor(() => { + let fixture = createFixture(builder, template, style); - return builder.createAsync(BasicTestApp) - .then((f) => { - fixture = f; - testComponent = fixture.debugElement.componentInstance; - fixture.detectChanges(); - return wait(1); - }).then((_: any) => { - let openButtonElement = fixture.debugElement.query(By.css('.open')); - openButtonElement.nativeElement.click(); - fixture.detectChanges(); - return wait(1); - }).then((_: any) => { - expect(testComponent.openStartCount).toBe(1); - expect(testComponent.openCount).toBe(0); - }) - .then((_: any) => { return waitOnEvent(fixture, By.directive(MdSidenav), 'onOpen'); }) - .then((_: any) => { - expect(testComponent.openStartCount).toBe(1); - expect(testComponent.openCount).toBe(1); - expect(testComponent.closeStartCount).toBe(0); - expect(testComponent.closeCount).toBe(0); + let testComponent: BasicTestApp = fixture.debugElement.componentInstance; + let openButtonElement = fixture.debugElement.query(By.css('.open')); + openButtonElement.nativeElement.click(); + fixture.detectChanges(); + tick(); - let sidenavElement = fixture.debugElement.query(By.css('md-sidenav')); - let sidenavBackdropElement = fixture.debugElement.query(By.css('.md-sidenav-backdrop')); - expect(getComputedStyle(sidenavElement.nativeElement).visibility).toEqual('visible'); - expect(getComputedStyle(sidenavBackdropElement.nativeElement).visibility).toEqual('visible'); + expect(testComponent.openStartCount).toBe(1); + expect(testComponent.openCount).toBe(0); - // Close it. - let closeButtonElement = fixture.debugElement.query(By.css('.close')); - closeButtonElement.nativeElement.click(); - fixture.detectChanges(); - return wait(1); - }) - .then((_: any) => wait(1)) - .then((_: any) => { - expect(testComponent.openStartCount).toBe(1); - expect(testComponent.openCount).toBe(1); - expect(testComponent.closeStartCount).toBe(1); - expect(testComponent.closeCount).toBe(0); - }) - .then((_: any) => { return waitOnEvent(fixture, By.directive(MdSidenav), 'onClose'); }) - .then((_: any) => fixture.detectChanges()) - .then((_: any) => { - expect(testComponent.openStartCount).toBe(1); - expect(testComponent.openCount).toBe(1); - expect(testComponent.closeStartCount).toBe(1); - expect(testComponent.closeCount).toBe(1); + endSidenavTransition(fixture); + tick(); - let sidenavElement = fixture.debugElement.query(By.css('md-sidenav')); - let sidenavBackdropElement = fixture.debugElement.query(By.css('.md-sidenav-backdrop')); - expect(getComputedStyle(sidenavElement.nativeElement).visibility).toEqual('hidden'); - expect(getComputedStyle(sidenavBackdropElement.nativeElement).visibility).toEqual('hidden'); - }) - .then((_: any) => { done(); }); - }, 8000); + expect(testComponent.openStartCount).toBe(1); + expect(testComponent.openCount).toBe(1); + expect(testComponent.closeStartCount).toBe(0); + expect(testComponent.closeCount).toBe(0); - it('open() and close() return a promise that resolves after the animation ended', - (done: any) => { - let fixture: ComponentFixture; - let sidenav: MdSidenav; + let sidenavElement = fixture.debugElement.query(By.css('md-sidenav')); + let sidenavBackdropElement = fixture.debugElement.query(By.css('.md-sidenav-backdrop')); + expect(getComputedStyle(sidenavElement.nativeElement).visibility).toEqual('visible'); + expect(getComputedStyle(sidenavBackdropElement.nativeElement).visibility).toEqual('visible'); - let promise: Promise; - let called: boolean = false; + // Close it. + let closeButtonElement = fixture.debugElement.query(By.css('.close')); + closeButtonElement.nativeElement.click(); + fixture.detectChanges(); + tick(); - return builder.createAsync(BasicTestApp) - .then((f) => { - fixture = f; - sidenav = fixture.debugElement.query(By.directive(MdSidenav)).componentInstance; + expect(testComponent.openStartCount).toBe(1); + expect(testComponent.openCount).toBe(1); + expect(testComponent.closeStartCount).toBe(1); + expect(testComponent.closeCount).toBe(0); - promise = sidenav.open(); - promise.then((_: any) => called = true); - }) - .then((_: any) => { return wait(1); }) - .then((_: any) => fixture.detectChanges()) - .then((_: any) => { - expect(called).toBe(false); - }) - .then((_: any) => { return promise; }) - .then((_: any) => { expect(called).toBe(true); }) - .then((_: any) => { - // Close it now. - called = false; - promise = sidenav.close(); - promise.then((_: any) => called = true); - }) - .then((_: any) => { return wait(1); }) - .then((_: any) => fixture.detectChanges()) - .then((_: any) => { - expect(called).toBe(false); - }) - .then((_: any) => { return promise; }) - .then((_: any) => { expect(called).toBe(true); }) - .then((_: any) => { done(); }); - }, 8000); + endSidenavTransition(fixture); + tick(); - it('open/close() twice returns the same promise', (done: any) => { - let fixture: ComponentFixture; - let sidenav: MdSidenav; + expect(testComponent.openStartCount).toBe(1); + expect(testComponent.openCount).toBe(1); + expect(testComponent.closeStartCount).toBe(1); + expect(testComponent.closeCount).toBe(1); - let promise: Promise; + expect(getComputedStyle(sidenavElement.nativeElement).visibility).toEqual('hidden'); + expect(getComputedStyle(sidenavBackdropElement.nativeElement).visibility).toEqual('hidden'); + })); - return builder.createAsync(BasicTestApp) - .then((f) => { - fixture = f; - sidenav = fixture.debugElement.query(By.directive(MdSidenav)).componentInstance; + it('open() and close() return a promise that resolves after the animation ended', fakeAsyncAdaptor(() => { + let fixture = createFixture(builder, template, style); + let sidenav: MdSidenav = fixture.debugElement.query(By.directive(MdSidenav)).componentInstance; + let called: boolean = false; - promise = sidenav.open(); - expect(sidenav.open()).toBe(promise); - return wait(1); - }) - .then((_: any) => { return wait(1); }) - .then((_: any) => { - fixture.detectChanges(); - return promise; - }) - .then((_: any) => { return wait(1); }) - .then((_: any) => { - promise = sidenav.close(); - expect(sidenav.close()).toBe(promise); - }) - .then((_: any) => { done(); }); - }); + sidenav.open().then((_: any) => { + called = true + }); - it('open() then close() cancel animations when called too fast', - (done: any) => { - let fixture: ComponentFixture; - let sidenav: MdSidenav; + expect(called).toBe(false); + endSidenavTransition(fixture); + tick(); + expect(called).toBe(true); - let closePromise: Promise; - let openCalled: boolean = false; - let openCancelled: boolean = false; - let closeCalled: boolean = false; + called = false; + sidenav.close().then((_: any) => { + called = true + }); - return builder.createAsync(BasicTestApp) - .then((f) => { - fixture = f; - sidenav = fixture.debugElement.query(By.directive(MdSidenav)).componentInstance; + expect(called).toBe(false); + endSidenavTransition(fixture); + tick(); + expect(called).toBe(true); - sidenav.open().then((_: any) => { - openCalled = true; - }, - () => { - openCancelled = true; - }); - }) - .then((_: any) => { return wait(1); }) - .then((_: any) => fixture.detectChanges()) - // We need to wait for the browser to start the transition. - .then((_: any) => { return wait(50); }) - .then((_: any) => { - closePromise = sidenav.close().then((_: any) => { - closeCalled = true; - }); - return wait(1); - }) - .then((_: any) => { - fixture.detectChanges(); - return closePromise; - }) - .then((_: any) => { - expect(openCalled).toBe(false); - expect(openCancelled).toBe(true); - expect(closeCalled).toBe(true); - }) - .then((_: any) => { done(); }); - }, 8000); + })); - it('close() then open() cancel animations when called too fast', - (done: any) => { - let fixture: ComponentFixture; - let sidenav: MdSidenav; + it('open/close() twice returns the same promise', fakeAsyncAdaptor(() => { + let fixture = createFixture(builder, template, style); + let sidenav: MdSidenav = fixture.debugElement.query(By.directive(MdSidenav)).componentInstance; - let openPromise: Promise; - let closeCalled: boolean = false; - let closeCancelled: boolean = false; - let openCalled: boolean = false; + let promise = sidenav.open(); + expect(sidenav.open()).toBe(promise); + fixture.detectChanges(); + tick(); - return builder.createAsync(BasicTestApp) - .then((f) => { - fixture = f; - sidenav = fixture.debugElement.query(By.directive(MdSidenav)).componentInstance; + promise = sidenav.close(); + expect(sidenav.close()).toBe(promise); + tick(); + })); - /** First, open it. */ - openPromise = sidenav.open(); - }) - .then((_: any) => { return wait(1); }) - .then((_: any) => { - fixture.detectChanges(); - return openPromise; - }) - .then((_: any) => { - // Then close and check behavior. - sidenav.close().then((_: any) => { - closeCalled = true; - }, () => { - closeCancelled = true; - }); - }) - .then((_: any) => { return wait(1); }) - .then((_: any) => fixture.detectChanges()) - // We need to wait for the browser to start the transition. - .then((_: any) => { return wait(50); }) - .then((_: any) => { - openPromise = sidenav.open().then((_: any) => { - openCalled = true; - }, done.fail); - return wait(1); - }) - .then((_: any) => { - fixture.detectChanges(); - return openPromise; - }) - .then((_: any) => { - expect(closeCalled).toBe(false); - expect(closeCancelled).toBe(true); - expect(openCalled).toBe(true); - }) - .then((_: any) => { done(); }); - }, 8000); + it('open() then close() cancel animations when called too fast', fakeAsyncAdaptor(() => { + let fixture = createFixture(builder, template, style); + let sidenav: MdSidenav = fixture.debugElement.query(By.directive(MdSidenav)).componentInstance; + + let closePromise: Promise; + let openCalled: boolean = false; + let openCancelled: boolean = false; + let closeCalled: boolean = false; + + sidenav.open().then((_: any) => { openCalled = true; }, () => { openCancelled = true; }); + + // We do not call transition end, close directly. + closePromise = sidenav.close().then((_: any) => { closeCalled = true; }); + + endSidenavTransition(fixture); + tick(); + + expect(openCalled).toBe(false); + expect(openCancelled).toBe(true); + expect(closeCalled).toBe(true); + tick(); + })); + + it('close() then open() cancel animations when called too fast', fakeAsyncAdaptor(() => { + let fixture = createFixture(builder, template, style); + let sidenav: MdSidenav = fixture.debugElement.query(By.directive(MdSidenav)).componentInstance; + + let openPromise: Promise; + let closeCalled: boolean = false; + let closeCancelled: boolean = false; + let openCalled: boolean = false; + + // First, open the sidenav completely. + sidenav.open(); + endSidenavTransition(fixture); + tick(); + + // Then close and check behavior. + sidenav.close().then((_: any) => { closeCalled = true; }, () => { closeCancelled = true; }); + // We do not call transition end, open directly. + sidenav.open().then((_: any) => { openCalled = true; }); + + endSidenavTransition(fixture); + tick(); + + expect(closeCalled).toBe(false); + expect(closeCancelled).toBe(true); + expect(openCalled).toBe(true); + tick(); + })); }); }); } @@ -291,10 +237,10 @@ export function main() { template: ` + (open-start)="openStart()" + (open)="open()" + (close-start)="closeStart()" + (close)="close()"> Content. @@ -307,4 +253,17 @@ class BasicTestApp { openCount: number = 0; closeStartCount: number = 0; closeCount: number = 0; + + openStart() { + this.openStartCount++; + } + open() { + this.openCount++; + } + closeStart() { + this.closeStartCount++; + } + close() { + this.closeCount++; + } } diff --git a/src/components/sidenav/sidenav.ts b/src/components/sidenav/sidenav.ts index bb645482f12..2c78de828f6 100644 --- a/src/components/sidenav/sidenav.ts +++ b/src/components/sidenav/sidenav.ts @@ -11,6 +11,7 @@ import { Output, QueryList, Type, + View, ChangeDetectionStrategy } from 'angular2/core'; import {PromiseWrapper, ObservableWrapper, EventEmitter} from 'angular2/src/facade/async'; @@ -155,7 +156,7 @@ export class MdSidenav { * @param e The event. * @private */ - @HostListener('transitionend', ['$event']) private onTransitionEnd_(e: any) { + @HostListener('transitionend', ['$event']) onTransitionEnd(e: any) { if (e.target == this.elementRef_.nativeElement // Simpler version to check for prefixes. && e.propertyName.endsWith('transform')) { @@ -241,13 +242,18 @@ export class MdSidenav { */ @Component({ selector: 'md-sidenav-layout', - directives: [MdSidenav], - templateUrl: './components/sidenav/sidenav.html', - styleUrls: ['./components/sidenav/sidenav.css'], // Do not use ChangeDetectionStrategy.OnPush. It does not work for this component because // technically it is a sibling of MdSidenav (on the content tree) and isn't updated when MdSidenav // changes its state. }) +@View({ + directives: [MdSidenav], + templateUrl: './components/sidenav/sidenav.html', + styleUrls: [ + './components/sidenav/sidenav.css', + './components/sidenav/sidenav-transitions.css', + ], +}) export class MdSidenavLayout implements AfterContentInit { @ContentChildren(MdSidenav) private sidenavs_: QueryList; @@ -269,10 +275,10 @@ export class MdSidenavLayout implements AfterContentInit { } - /** The sidenav at the start/end alignment, independent of direction. */ private start_: MdSidenav; private end_: MdSidenav; + /** * The sidenav at the left/right. When direction changes, these will change as well. * They're used as aliases for the above to set the left/right style properly. diff --git a/test/browser-providers.ts b/test/browser-providers.ts index a26c403ab1f..adca2beadfd 100644 --- a/test/browser-providers.ts +++ b/test/browser-providers.ts @@ -27,7 +27,7 @@ export type AliasMap = { [name: string]: string[] }; const configuration: { [name: string]: ConfigurationInfo } = { 'Chrome': { unitTest: {target: 'SL', required: true}, e2e: {target: null, required: true}}, 'Firefox': { unitTest: {target: 'SL', required: true}, e2e: {target: null, required: true}}, - 'ChromeBeta': { unitTest: {target: 'SL', required: true}, e2e: {target: null, required: true}}, + 'ChromeBeta': { unitTest: {target: 'SL', required: false}, e2e: {target: null, required: true}}, 'FirefoxBeta': { unitTest: {target: 'SL', required: true}, e2e: {target: null, required: true}}, 'ChromeDev': { unitTest: {target: null, required: true}, e2e: {target: null, required: true}}, 'FirefoxDev': { unitTest: {target: null, required: true}, e2e: {target: null, required: true}},