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

fix($animateCss): avoid flicker caused by temporary classes #15472

Closed
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
5 changes: 5 additions & 0 deletions src/ngAnimate/animateCss.js
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,11 @@ var $AnimateCssProvider = ['$animateProvider', /** @this */ function($animatePro
return;
}

if (options.tempClasses) {
$$jqLite.addClass(element, options.tempClasses);
options.tempClasses = null;
}

// even though we only pause keyframe animations here the pause flag
// will still happen when transitions are used. Only the transition will
// not be paused since that is not possible. If the animation ends when
Expand Down
5 changes: 5 additions & 0 deletions src/ngAnimate/animateJs.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ var $$AnimateJsProvider = ['$animateProvider', /** @this */ function($animatePro
return runner;
}

if (options.tempClasses) {
$$jqLite.addClass(element, options.tempClasses);
options.tempClasses = null;
}

runner = new $$AnimateRunner();
var closeActiveAnimations;
var chain = [];
Expand Down
13 changes: 6 additions & 7 deletions src/ngAnimate/animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro
var tempClasses = options.tempClasses;
if (tempClasses) {
classes += ' ' + tempClasses;
options.tempClasses = null;
}

var prepareClassName;
Expand Down Expand Up @@ -185,9 +184,8 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro
toBeSortedAnimations.push({
domNode: getDomNode(animationEntry.from ? animationEntry.from.element : animationEntry.element),
fn: function triggerAnimationStart() {
// it's important that we apply the `ng-animate` CSS class and the
// temporary classes before we do any driver invoking since these
// CSS classes may be required for proper CSS detection.
// It is important that we apply the `ng-animate` CSS class before we do any driver
// invoking since these CSS classes may be required for proper CSS detection.
animationEntry.beforeStart();

var startAnimationFn, closeFn = animationEntry.close;
Expand Down Expand Up @@ -359,10 +357,11 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro
}

function beforeStart() {
// Note: `tempClasses` are not added here, since the actual animation may not start right
// away (`$animateCss` for example forces a reflow first). It is the responsibility of the
// respective animation driver to add them (if necessary).

element.addClass(NG_ANIMATE_CLASSNAME);
if (tempClasses) {
$$jqLite.addClass(element, tempClasses);
}
if (prepareClassName) {
$$jqLite.removeClass(element, prepareClassName);
prepareClassName = null;
Expand Down
20 changes: 20 additions & 0 deletions test/ngAnimate/animateCssSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,26 @@ describe('ngAnimate $animateCss', function() {
expect(element).toHaveClass('super-active');
}));

it('should apply `tempClasses` when starting the animation',
inject(function($animateCss, $document, $rootElement) {

var element = angular.element('<div></div>');
$rootElement.append(element);
$document.find('body').append($rootElement);

$animateCss(element, {
event: 'super',
duration: 1000,
to: fakeStyle,
tempClasses: 'foo'
}).start();

expect(element).not.toHaveClass('foo');

triggerAnimationStartFrame();
expect(element).toHaveClass('foo');
}));

describe('structural animations', function() {
they('should decorate the element with the ng-$prop CSS class',
['enter', 'leave', 'move'], function(event) {
Expand Down
19 changes: 18 additions & 1 deletion test/ngAnimate/animateJsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ describe('ngAnimate $$animateJs', function() {
});
});

it('should always run the provided animation in atleast one RAF frame if defined', function() {
it('should always run the provided animation in at least one RAF frame if defined', function() {
var before, after, endCalled;
module(function($animateProvider) {
$animateProvider.register('.the-end', function() {
Expand Down Expand Up @@ -325,6 +325,23 @@ describe('ngAnimate $$animateJs', function() {
});
});

it('should apply `tempClasses` when starting the animation', function() {
module(function($animateProvider) {
$animateProvider.register('.foo-element', function() {
return {foo: noop};
});
});
inject(function($$animateJs) {
var element = jqLite('<div class="foo-element"></div>');
var animator = $$animateJs(element, 'foo', 'foo-element', {tempClasses: 'temp'});

expect(element).not.toHaveClass('temp');

animator.start();
expect(element).toHaveClass('temp');
});
});

describe('events', function() {
var animations, runAnimation, element, log;
beforeEach(module(function($animateProvider) {
Expand Down
53 changes: 47 additions & 6 deletions test/ngAnimate/animationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -863,9 +863,10 @@ describe('$$animation', function() {
jqLite($document[0].body).append($rootElement);
$rootElement.append(parent);

mockedDriverFn = function(element, method, options, domOperation) {
mockedDriverFn = function(details) {
return {
start: function() {
details.element.addClass(details.options.tempClasses);
runner = new $$AnimateRunner();
return runner;
}
Expand All @@ -874,7 +875,7 @@ describe('$$animation', function() {
};
}));

it('should temporarily assign the provided CSS class for the duration of the animation',
it('should temporarily assign the provided CSS classes for the duration of the animation',
inject(function($rootScope, $$animation) {

parent.append(element);
Expand All @@ -894,6 +895,26 @@ describe('$$animation', function() {
expect(element).not.toHaveClass('fudge');
}));

it('should remove the temporary CSS classes if the animation gets cancelled',
inject(function($$animation, $rootScope) {

parent.append(element);

$$animation(element, 'enter', {
tempClasses: 'temporary fudge'
});
$rootScope.$digest();

expect(element).toHaveClass('temporary');
expect(element).toHaveClass('fudge');

runner.cancel();
$rootScope.$digest();

expect(element).not.toHaveClass('temporary');
expect(element).not.toHaveClass('fudge');
}));

it('should add and remove the ng-animate CSS class when the animation is active',
inject(function($$animation, $rootScope) {

Expand All @@ -910,11 +931,9 @@ describe('$$animation', function() {
}));


it('should apply the `ng-animate` and temporary CSS classes before the driver is invoked', function() {
it('should apply the `ng-animate` CSS class before the driver is invoked', function() {
var capturedElementClasses;

parent.append(element);

module(function($provide) {
$provide.factory('mockedTestDriver', function() {
return function(details) {
Expand All @@ -932,7 +951,29 @@ describe('$$animation', function() {
$rootScope.$digest();

expect(capturedElementClasses).toMatch(/\bng-animate\b/);
expect(capturedElementClasses).toMatch(/\btemp-class-name\b/);
});
});

it('should not apply `tempClasses` before the driver is invoked', function() {
var capturedElementClasses;

module(function($provide) {
$provide.factory('mockedTestDriver', function($$jqLite) {
return function(details) {
capturedElementClasses = details.element.attr('class');
};
});
});

inject(function($$animation, $rootScope) {
parent.append(element);

$$animation(element, 'enter', {
tempClasses: 'temp-class-name'
});
$rootScope.$digest();

expect(capturedElementClasses).not.toMatch(/\btemp-class-name\b/);
});
});

Expand Down
65 changes: 65 additions & 0 deletions test/ngAnimate/integrationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,71 @@ describe('ngAnimate integration tests', function() {
expect(child).not.toHaveClass('blue');
});
});

it('should apply a temporary class only for the actual duration of the animation', function() {
var elementClassList = [];
var hasTempClass = {
beforeCssAnimationStart: null,
afterCssAnimationStart: null,
afterCssAnimationTriggered: null,
afterCssAnimationFinished: null
};

module(function($provide) {
$provide.decorator('$animateCss', function($delegate) {
var decorated = function(element) {
var animator = $delegate.apply(null, arguments);
var startFn = animator.start;

animator.start = function() {
hasTempClass.beforeCssAnimationStart = element.hasClass('temp-class');
var runner = startFn.apply(animator, arguments);
hasTempClass.afterCssAnimationStart = element.hasClass('temp-class');
return runner;
};

return animator;
};

return decorated;
});
});

inject(function($animate, $compile, $rootScope) {
element = jqLite('<div class="animate-me"></div>');
$compile(element)($rootScope);

var className = 'klass';
var parent = jqLite('<div></div>');
html(parent);

parent.append(element);

ss.addRule('.animate-me', 'transition: all 2s;');

var runner = $animate.addClass(element, className, {
tempClasses: 'temp-class'
});

$rootScope.$digest();
$animate.flush();

hasTempClass.afterCssAnimationTriggered = element.hasClass('temp-class');

browserTrigger(element, 'transitionend', {timeStamp: Date.now(), elapsedTime: 2});
$rootScope.$digest();
$animate.flush();

hasTempClass.afterCssAnimationFinished = element.hasClass('temp-class');

expect(hasTempClass).toEqual({
beforeCssAnimationStart: false,
afterCssAnimationStart: false,
afterCssAnimationTriggered: true,
afterCssAnimationFinished: false
});
});
});
});

describe('JS animations', function() {
Expand Down