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.

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(@angular/cli): adds --animation flag to ng new #5786

Closed

Conversation

dave11mj
Copy link
Contributor

Fixes #5785

@dave11mj dave11mj force-pushed the feature-add-app-with-animation branch 5 times, most recently from 9803555 to 2c0c1a8 Compare April 1, 2017 17:49
@dave11mj dave11mj changed the title feat(@angular/cli): adds --animation support to ng new feat(@angular/cli): adds --animation flag to ng new Apr 1, 2017
@sumitarora sumitarora requested a review from Brocco April 1, 2017 22:15
@dave11mj dave11mj force-pushed the feature-add-app-with-animation branch 4 times, most recently from 5b0f6c3 to cd1ab1b Compare April 3, 2017 18:37
@dave11mj dave11mj force-pushed the feature-add-app-with-animation branch from cd1ab1b to 711965d Compare April 12, 2017 15:20
Copy link
Contributor

@Brocco Brocco left a comment

Choose a reason for hiding this comment

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

I like the idea of supporting animations out of the box with the CLI.

I don't think the CLI should generate an use an animation though, so please remove app-fade-in.animation.ts as well as references to it.

Also, BrowserAnimationsModule should be used in place of BrowserModules not in addition to in app.module.ts

Good catch to include updates to the documentation 👍

@dave11mj dave11mj force-pushed the feature-add-app-with-animation branch 3 times, most recently from 7da07d9 to 781fb95 Compare April 17, 2017 19:38
@dave11mj
Copy link
Contributor Author

dave11mj commented Apr 17, 2017

Hey @Brocco !

Thanks for the feedback ~ ^^

Specially for the part about BrowserAnimationsModule since thats really good to know.

BrowserAnimationsModule should be used in place of BrowserModules not in addition

I updated the PR with the changes requested and added an alias of -a to the flag.

Let me know if I missed anything or if there other requests ! :D

@dave11mj dave11mj force-pushed the feature-add-app-with-animation branch from 781fb95 to 0411a0c Compare April 24, 2017 14:37
@dave11mj
Copy link
Contributor Author

dave11mj commented Apr 24, 2017

Hey again @Brocco!

I added BrowserModules back to the commit since it seems intended to be used along with BrowserAnimationsModule according to the official documentation for animations.

image

When I first tested it out using BrowserAnimationsModule in place of BrowserModules it seemed to work since the testing app didn't have much logic. However, after trying it out on a project of mine, I started to get runtime errors on developer tools (webpack / everything else compiles properly). Below is a screenshot to help illustrate the steps to reproduce the bug. The fix seemed to be including the BrowserModules.

image

@filipesilva
Copy link
Contributor

This PR is being superseded by #6144, which adds animations a lot of other changes.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add animation support to ng new command
6 participants