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
wants to merge 1 commit into from

Conversation

stovenator
Copy link

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

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
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@petebacondarwin
Copy link
Contributor

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

@gkalpak
Copy link
Member

gkalpak commented Jun 30, 2015

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

@petebacondarwin
Copy link
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
Copy link
Member

gkalpak commented Jun 30, 2015

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
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
Copy link
Member

gkalpak commented Jul 1, 2015

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
Copy link
Member

gkalpak commented Jul 1, 2015

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