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.

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

feat(core): introduce fixture.whenRenderingDone for testing #16732

Merged
merged 1 commit into from May 12, 2017

Conversation

Projects
None yet
5 participants
@matsko
Member

matsko commented May 11, 2017

No description provided.

@googlebot googlebot added the cla: yes label May 11, 2017

@matsko matsko requested a review from tbosch May 11, 2017

@tbosch

tbosch approved these changes May 11, 2017

@@ -16,6 +16,7 @@ export declare class ComponentFixture<T> {
destroy(): void;
detectChanges(checkNoChanges?: boolean): void;
isStable(): boolean;
whenRenderingDone(): Promise<any>;

This comment has been minimized.

@IgorMinar

IgorMinar May 12, 2017

Member

is this the right name? when I look at whenStable and whenRenderingDone how should I know which one to choose without reading the api docs?

Maybe whenAnimated() or whenRendered()? "rendered" is however ambiguous because rendering is a just a snapshot of a animation, so at any point during animation the UI is rendered, it's just not fully animated.

Are there any other related api additions that could help us pick the right name? Looking at the overall api surface provides the best guidance.

@IgorMinar

IgorMinar May 12, 2017

Member

is this the right name? when I look at whenStable and whenRenderingDone how should I know which one to choose without reading the api docs?

Maybe whenAnimated() or whenRendered()? "rendered" is however ambiguous because rendering is a just a snapshot of a animation, so at any point during animation the UI is rendered, it's just not fully animated.

Are there any other related api additions that could help us pick the right name? Looking at the overall api surface provides the best guidance.

This comment has been minimized.

@IgorMinar

IgorMinar May 12, 2017

Member

discussed this more with @matsko and I'm fine with the name. all the alternatives are not great.

@IgorMinar

IgorMinar May 12, 2017

Member

discussed this more with @matsko and I'm fine with the name. all the alternatives are not great.

@jasonaden jasonaden merged commit 38c524d into angular:master May 12, 2017

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
cla/google All necessary CLAs are signed
code-review/pullapprove Approved by all reviewer groups.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@matsko matsko deleted the matsko:whenRenderingDone branch May 12, 2017

asnowwolf added a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017

juleskremer added a commit to juleskremer/angular that referenced this pull request Aug 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment