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

feat($animateCss): add support for temporary styles via cleanupStyles #12930

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/ng/animateCss.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ var $CoreAnimateCssProvider = function() {
};

return function(element, options) {
// there is no point in applying the styles since
// there is no animation that goes on at all in
// this version of $animateCss.
if (options.cleanupStyles) {
options.from = options.to = null;
}

if (options.from) {
element.css(options.from);
options.from = null;
Expand Down
40 changes: 38 additions & 2 deletions src/ngAnimate/animateCss.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ var ANIMATE_TIMER_KEY = '$$animateCss';
* * `staggerIndex` - The numeric index representing the stagger item (e.g. a value of 5 is equal to the sixth item in the stagger; therefore when a
* * `stagger` option value of `0.1` is used then there will be a stagger delay of `600ms`)
* * `applyClassesEarly` - Whether or not the classes being added or removed will be used when detecting the animation. This is set by `$animate` when enter/leave/move animations are fired to ensure that the CSS classes are resolved in time. (Note that this will prevent any transitions from occuring on the classes being added and removed.)
* * `cleanupStyles` - Whether or not the provided `from` and `to` styles will be removed once the animation is closed. This is useful for when the styles are used purely for the sake of the animation and do not have a lasting visual effect on the element (e.g. a colapse and open animation). By default this value is set to `false`.
Copy link
Member

Choose a reason for hiding this comment

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

Too long lines :(

*
* @return {object} an object with start and end methods and details about the animation.
*
Expand Down Expand Up @@ -324,6 +325,23 @@ function createLocalCacheLookup() {
};
}

function registerRestorableStyles(backup, node, properties) {
forEach(properties, function(prop) {
// we do not reassign an already present style value since
// if we detect the style property value again we may be
// detecting styles that were added via the `from` styles.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move this first sentence into a comment above the function declaration since it is about what this function is doing in general, whereas the following sentences are about the implementation issue within the function.

// We make use of `isDefined` here since an empty string
// or null value (which is what getPropertyValue will return
// for a non-existing style) will still be marked as a valid
// value for the style (a falsy value implies that the style
// is to be removed at the animation). If we had a simple
Copy link
Member

Choose a reason for hiding this comment

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

at the animation --> at the animation end ??

// or statement then it would not be enough to catch that.
Copy link
Member

Choose a reason for hiding this comment

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

or --> "or"/'or'/or/OR/...

backup[prop] = isDefined(backup[prop])
? backup[prop]
: node.style.getPropertyValue(prop);
});
}

var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
var gcsLookup = createLocalCacheLookup();
var gcsStaggerLookup = createLocalCacheLookup();
Expand Down Expand Up @@ -424,6 +442,7 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
}

return function init(element, options) {
var restoreStyles = {};
var node = getDomNode(element);
if (!node
|| !node.parentNode
Expand Down Expand Up @@ -625,7 +644,12 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
stagger.animationDuration === 0;
}

applyAnimationFromStyles(element, options);
if (options.from) {
if (options.cleanupStyles) {
registerRestorableStyles(restoreStyles, node, Object.keys(options.from));
}
applyAnimationFromStyles(element, options);
}

if (flags.blockTransition || flags.blockKeyframeAnimation) {
applyBlocking(maxDuration);
Expand Down Expand Up @@ -692,6 +716,13 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
applyAnimationClasses(element, options);
applyAnimationStyles(element, options);

if (Object.keys(restoreStyles).length) {
forEach(restoreStyles, function(value, prop) {
value ? node.style.setProperty(prop, value)
: node.style.removeProperty(prop);
});
}

// the reason why we have this option is to allow a synchronous closing callback
// that is fired as SOON as the animation ends (when the CSS is removed) or if
// the animation never takes off at all. A good example is a leave animation since
Expand Down Expand Up @@ -886,7 +917,12 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
}

element.on(events.join(' '), onAnimationProgress);
applyAnimationToStyles(element, options);
if (options.to) {
if (options.cleanupStyles) {
registerRestorableStyles(restoreStyles, node, Object.keys(options.to));
}
applyAnimationToStyles(element, options);
}
}

function onAnimationExpired() {
Expand Down
32 changes: 32 additions & 0 deletions test/ng/animateCssSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,38 @@ describe("$animateCss", function() {
expect(cancelSpy).toHaveBeenCalled();
expect(doneSpy).not.toHaveBeenCalled();
}));

it("should not bother applying the provided [from] and [to] styles to the element if [cleanupStyles] is present",
inject(function($animateCss, $rootScope) {

var animator = $animateCss(element, {
cleanupStyles: true,
from: { width: '100px' },
to: { width: '900px', height: '1000px' }
});

assertStyleIsEmpty(element, 'width');
assertStyleIsEmpty(element, 'height');

var runner = animator.start();

assertStyleIsEmpty(element, 'width');
assertStyleIsEmpty(element, 'height');

triggerRAF();

assertStyleIsEmpty(element, 'width');
assertStyleIsEmpty(element, 'height');

runner.end();

assertStyleIsEmpty(element, 'width');
assertStyleIsEmpty(element, 'height');

function assertStyleIsEmpty(element, prop) {
expect(element[0].style.getPropertyValue(prop)).toBeFalsy();
}
}));
});

});
65 changes: 65 additions & 0 deletions test/ngAnimate/animateCssSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2786,6 +2786,71 @@ describe("ngAnimate $animateCss", function() {
}));
});

describe("[cleanupStyles]", function() {
it("should cleanup [from] and [to] styles that have been applied for the animation when true",
inject(function($animateCss) {

var runner = $animateCss(element, {
duration: 1,
from: { background: 'gold' },
to: { color: 'brown' },
cleanupStyles: true
}).start();

assertStyleIsPresent(element, 'background', true);
assertStyleIsPresent(element, 'color', false);

triggerAnimationStartFrame();

assertStyleIsPresent(element, 'background', true);
assertStyleIsPresent(element, 'color', true);

runner.end();

assertStyleIsPresent(element, 'background', false);
assertStyleIsPresent(element, 'color', false);

function assertStyleIsPresent(element, style, bool) {
expect(element[0].style[style])[bool ? 'toBeTruthy' : 'toBeFalsy']();
}
}));

it("should restore existing overidden styles already on present on the element when true",
Copy link
Member

Choose a reason for hiding this comment

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

on present --> present

inject(function($animateCss) {

element.css("height", "100px");
element.css("width", "111px");

var runner = $animateCss(element, {
duration: 1,
from: { height: '200px', 'font-size':'66px' },
to: { height: '300px', 'font-size': '99px', width: '222px' },
cleanupStyles: true
}).start();

assertStyle(element, 'height', "200px");
assertStyle(element, 'font-size', "66px");
assertStyle(element, 'width', "111px");

triggerAnimationStartFrame();

assertStyle(element, 'height', "300px");
assertStyle(element, 'width', "222px");
assertStyle(element, 'font-size', "99px");

runner.end();

assertStyle(element, 'width', "111px");
assertStyle(element, 'height', "100px");

expect(element[0].style.getPropertyValue("font-size")).not.toBe("66px");

function assertStyle(element, prop, value) {
expect(element[0].style.getPropertyValue(prop)).toBe(value);
}
}));
});

Copy link
Member

Choose a reason for hiding this comment

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

Single quotes everywhere for consistency, anyone ? 😆

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 - I blame @matsko's VIM setup

Copy link
Member

Choose a reason for hiding this comment

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

It must be a really elaborate setup, producing stuff like assertStyle(element, 'height', "100px") 😛 :

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You guys are so mean :(

Copy link
Contributor

Choose a reason for hiding this comment

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

:)

it('should round up long elapsedTime values to close off a CSS3 animation',
inject(function($animateCss) {

Expand Down