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

Fixes to body element animation #12245

Closed
wants to merge 3 commits into from
Closed

Conversation

matsko
Copy link
Contributor

@matsko matsko commented Jun 30, 2015

Review on Reviewable

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@matsko matsko force-pushed the pr_11956 branch 2 times, most recently from 11e2f11 to 0d940ad Compare June 30, 2015 18:17
@lgalfaso
Copy link
Contributor

I am slightly inclined into making the new $body service an undocumented service. That a side, LGTM

inject(function($animate, $rootScope, $document, $rootElement) {
$animate.enabled(true);

var body = jqLite($document[0].body);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to use the new body service here instead?

@Narretz
Copy link
Contributor

Narretz commented Jul 6, 2015

I also think $body should be $$body.

@Narretz Narretz added this to the 1.4.2 milestone Jul 6, 2015
@petebacondarwin petebacondarwin modified the milestones: 1.4.2, 1.4.3 Jul 6, 2015
@petebacondarwin petebacondarwin modified the milestones: 1.4.3, 1.4.4 Jul 16, 2015
@matsko
Copy link
Contributor Author

matsko commented Jul 17, 2015

@petebacondarwin please review so we can merge this in tomorrow.

@petebacondarwin
Copy link
Member

It looks fine to me. The only comment I have is that since $$body is only used once in the core ng module, we should move the definition of $$body over to the ngAnimate module so we don't need to make angular.js file any bigger.

@matsko
Copy link
Contributor Author

matsko commented Jul 17, 2015

Merged in as 44ce9c8. Excellent work @OlenDavis.

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

6 participants