Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(ngAnimate): Change event to ng-enter - #11833

Closed
stovenator wants to merge 1 commit into
angular:masterfrom
stovenator:patch-1
Closed

stovenator wants to merge 1 commit into
angular:masterfrom
stovenator:patch-1

Conversation

@stovenator

Copy link
Copy Markdown

The event needs to map to the css class, or else the css class won't fire

Review on Reviewable

The event needs to map to the css class, or else the css class won't fire
@googlebot

Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@stovenator

Copy link
Copy Markdown
Author

I signed it!

@googlebot

Copy link
Copy Markdown

CLAs look good, thanks!

@petebacondarwin

Copy link
Copy Markdown
Contributor

@matsko - can you check this is correct and merge?

@gkalpak

gkalpak commented Jun 30, 2015

Copy link
Copy Markdown
Member

This doesn't seem correct (unless the documentation is wrong).
But there is definitely something that needs fixing.

@petebacondarwin

Copy link
Copy Markdown
Contributor

It seem to me that the original wording is correct. See https://github.com/angular/angular.js/blob/master/src/ngAnimate/animateCss.js#L185
In other words, the event is mapped to the "DOM event", and that the class is then generated from this.

@gkalpak

gkalpak commented Jun 30, 2015

Copy link
Copy Markdown
Member

The problem is that in practice, it doesn't work as described in the docs (but I think there is another issue about this).

@petebacondarwin

Copy link
Copy Markdown
Contributor

Can you provide a demo of this not working?
This line of code here seems to follow what the docs say:
https://github.com/angular/angular.js/blob/master/src/ngAnimate/animateCss.js#L528

@gkalpak

gkalpak commented Jul 1, 2015

Copy link
Copy Markdown
Member

I am pretty sure there was another issue (where I had commented) that showed the issue with enter and enter-active being added (without the ng- prefix). But I can't find it.

@matsko has pushed some fixes that may have taken care of the issue. I'll check and create a demo if it hasn't been fixed on master.

@gkalpak

gkalpak commented Jul 1, 2015

Copy link
Copy Markdown
Member

I found it: the issue is #12202 (which is not directily related, but lead me to noticing the unexpected(?) behaviour).

Here is the demo, I created for that issue.
(I would expect ng-enter/ng-enter-active to be added, but enter/enter-active are added instead. The same happens with the current snapshot.)

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants