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

fix(ngAnimate): allow animations on body and root elements #11956

Closed
wants to merge 1 commit into from

Conversation

OlenDavis
Copy link
Contributor

Overview of the Issue:

Animations on <body> and the root element don't work.

Motivation for or Use Case:

I have a class-based animation on the <body> element (though I noticed this would also be an issue for animations on the root element).

Angular Version(s):

v1.4.0-rc.2

Browsers and Operating System:

Chrome Version 45.0.2414.0 canary (64-bit), Mac OS X Yosemite Version 10.10.3 (14D136)

Reproduce the Error:

See this Plunker.

Related Issues:

The only open issue related to animate and body or root I found was #10527.

Suggest a Fix:

All that needed to be fixed was initializing (body|root)ElementDetected to whether the current element matches the body or root element rather than false; otherwise, the animated element is never checked as being within the body and root element and can therefore never be animated.

I believe all that needs to be added to this fix is testing if you agree this is a valid and relevant issue as fixed.

@petebacondarwin
Copy link
Member

@matsko could you take a look at this?
@OlenDavis this change will require a unit test before it can be merged.

@matsko
Copy link
Contributor

matsko commented Jun 9, 2015

@petebacondarwin @OlenDavis here is the completed commit with your tests:

matsko@2d0913e

@OlenDavis
Copy link
Contributor Author

Do you need me to add those tests to my commit? I'm sorry; it's just that I can't actually run the tests or even complete the normal grunt build locally because I get a Warning: stderr maxBuffer exceeded. Use --force to continue. failure every time. It doesn't even make it through the npm install portion of the grunt default task.

@matsko
Copy link
Contributor

matsko commented Jun 15, 2015

No that's fine. The commit I posted can be merged in so long as you're happy with the tests.

@petebacondarwin petebacondarwin modified the milestones: 1.4.1, 1.4.2 Jun 16, 2015
@OlenDavis
Copy link
Contributor Author

I'm unfamiliar with $$AnimateRunner and $$animate, but the tests look great; thank you! And, I'm very glad you went the extra inch and added the check for the HTML/document.documentElement element as well as body.

@petebacondarwin
Copy link
Member

@matsko - do you want to merge this then?

matsko pushed a commit to matsko/angular.js that referenced this pull request Jun 30, 2015
matsko pushed a commit to matsko/angular.js that referenced this pull request Jun 30, 2015
matsko pushed a commit to matsko/angular.js that referenced this pull request Jun 30, 2015
@petebacondarwin petebacondarwin modified the milestones: 1.4.2, 1.4.3 Jul 6, 2015
matsko pushed a commit to matsko/angular.js that referenced this pull request Jul 14, 2015
@petebacondarwin petebacondarwin modified the milestones: 1.4.3, 1.4.4 Jul 16, 2015
@matsko
Copy link
Contributor

matsko commented Jul 17, 2015

This is the final PR that will go in: #12245

matsko pushed a commit to matsko/angular.js that referenced this pull request Jul 17, 2015
@matsko matsko closed this in 44ce9c8 Jul 17, 2015
netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
ggershoni pushed a commit to ggershoni/angular.js that referenced this pull request Sep 29, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants