refactor: $animate - add $animateClassToggler to stop race conditions

Addresses #1100.

Fixes race conditions where $animate.{removeClass,hideClass} are
called simultaneously, in addition fixes any blinking that coulld be
caused by showing an element just attached to the dom.
This commit is contained in:
Andy Joslin
2014-04-10 10:34:14 -06:00
parent f7849cb846
commit e75e20dc9d
8 changed files with 166 additions and 50 deletions

View File

@@ -0,0 +1,55 @@
/**
* Stop race conditions with toggling an ngAnimate on an element rapidly,
* due to the asynchronous nature of addClass and removeClass
*
* Additionally, only toggle classes on requestAnimationFrame, to stop flickers on
* iOS and older android for elements that were just attached to the DOM.
*/
angular.module('ionic')
.factory('$animateClassToggler', [
'$animate',
'$rootScope',
function($animate, $rootScope) {
return function(element, className) {
var self = {
_nextOperation: null,
_animating: false,
_toggle: toggle,
_animate: animate,
_animationDone: animationDone,
addClass: function() { self._toggle(true); },
removeClass: function() { self._toggle(false); }
};
return self;
function toggle(hasClass) {
var operation = hasClass ? 'addClass' : 'removeClass';
if (self._animating) {
self._nextOperation = operation;
} else {
self._animate(operation);
}
}
function animate(operation) {
self._animating = true;
ionic.requestAnimationFrame(function() {
$rootScope.$evalAsync(function() {
$animate[operation](element, className, self._animationDone);
});
});
}
function animationDone() {
self._animating = false;
if (self._nextOperation) {
self._animate(self._nextOperation);
self._nextOperation = null;
}
}
};
}]);

View File

@@ -6,32 +6,30 @@ angular.module('ionic')
.factory('$ionicBackdrop', [
'$animate',
'$document',
function($animate, $document) {
'$animateClassToggler',
function($animate, $document, $animateClassToggler) {
var el;
var el = angular.element('<div class="backdrop ng-hide">');
var backdropHolds = 0;
var toggler = $animateClassToggler(el, 'ng-hide');
$document[0].body.appendChild(el[0]);
return {
retain: retain,
release: release,
_getElement: getElement
// exposed for testing
_element: el
};
function getElement() {
if (!el) {
el = angular.element('<div class="backdrop ng-hide">');
$document[0].body.appendChild(el[0]);
}
return el;
}
function retain() {
if ( (++backdropHolds) === 1 ) {
$animate.removeClass(getElement(), 'ng-hide');
toggler.removeClass();
}
}
function release() {
if ( (--backdropHolds) === 0 ) {
$animate.addClass(getElement(), 'ng-hide');
toggler.addClass();
}
}
}]);

View File

@@ -41,7 +41,8 @@ angular.module('ionic.service.loading', [])
'$q',
'$log',
'$compile',
function($animate, $document, $ionicTemplateLoader, $ionicBackdrop, $timeout, $q, $log, $compile) {
'$animateClassToggler',
function($animate, $document, $ionicTemplateLoader, $ionicBackdrop, $timeout, $q, $log, $compile, $animateClassToggler) {
var loaderInstance;
//default value
@@ -81,6 +82,9 @@ function($animate, $document, $ionicTemplateLoader, $ionicBackdrop, $timeout, $q
appendTo: $document[0].body
})
.then(function(loader) {
var toggler = $animateClassToggler(loader.element, 'ng-hide');
loader.show = function(options) {
var self = this;
var templatePromise = options.templateUrl ?
@@ -88,6 +92,7 @@ function($animate, $document, $ionicTemplateLoader, $ionicBackdrop, $timeout, $q
//options.content: deprecated
$q.when(options.template || options.content || '');
if (!this.isShown) {
//options.showBackdrop: deprecated
this.hasBackdrop = !options.noBackdrop && options.showBackdrop !== false;
@@ -111,26 +116,16 @@ function($animate, $document, $ionicTemplateLoader, $ionicBackdrop, $timeout, $q
}
});
toggler.removeClass();
ionic.DomUtil.centerElementByMarginTwice(this.element[0]);
this.isShown = true;
ionic.requestAnimationFrame(function() {
if (self.isShown) {
$animate.removeClass(self.element, 'ng-hide');
//Fix for ios: if we center the element twice, it always gets
//position right. Otherwise, it doesn't
ionic.DomUtil.centerElementByMargin(self.element[0]);
//One frame after it's visible, position it
ionic.requestAnimationFrame(function() {
ionic.DomUtil.centerElementByMargin(self.element[0]);
});
}
});
};
loader.hide = function() {
if (this.isShown) {
if (this.hasBackdrop) {
$ionicBackdrop.release();
}
$animate.addClass(this.element, 'ng-hide');
toggler.addClass();
}
$timeout.cancel(this.durationTimeout);
this.isShown = false;

View File

@@ -327,14 +327,8 @@ function($animate, $ionicTemplateLoader, $ionicBackdrop, $log, $q, $timeout, $ro
self.element.removeClass('popup-hidden');
self.element.addClass('popup-showing active');
ionic.DomUtil.centerElementByMarginTwice(self.element[0]);
focusLastButton(self.element);
//Fix for ios: if we center the element twice, it always gets
//position right. Otherwise, it doesn't
ionic.DomUtil.centerElementByMargin(self.element[0]);
//One frame after it's visible, position it
ionic.requestAnimationFrame(function() {
ionic.DomUtil.centerElementByMargin(self.element[0]);
});
});
};
self.hide = function(callback) {

View File

@@ -1,38 +1,34 @@
describe('$ionicBackdrop service', function() {
beforeEach(module('ionic'));
beforeEach(function() {
beforeEach(inject(function($animate) {
ionic.requestAnimationFrame = function(cb) { cb(); };
});
it('should create backdrop first time then reuse', inject(function($ionicBackdrop) {
var el = $ionicBackdrop._getElement();
var el2 = $ionicBackdrop._getElement();
expect(el.hasClass('backdrop')).toBe(true);
expect(el.hasClass('ng-hide')).toBe(true);
expect(el[0]).toBe(el2[0]);
}));
it('should remove ngHide on retain', inject(function($ionicBackdrop) {
var el = $ionicBackdrop._getElement();
it('should remove ngHide on retain', inject(function($ionicBackdrop, $timeout) {
var el = $ionicBackdrop._element;
expect(el.hasClass('ng-hide')).toBe(true);
$ionicBackdrop.retain();
$timeout.flush();
expect(el.hasClass('ng-hide')).toBe(false);
}));
it('should add ngHide on retain', inject(function($ionicBackdrop) {
var el = $ionicBackdrop._getElement();
it('should add ngHide on retain', inject(function($ionicBackdrop, $timeout) {
var el = $ionicBackdrop._element;
expect(el.hasClass('ng-hide')).toBe(true);
$ionicBackdrop.retain();
$timeout.flush();
expect(el.hasClass('ng-hide')).toBe(false);
$ionicBackdrop.release();
$timeout.flush();
expect(el.hasClass('ng-hide')).toBe(true);
}));
it('should require equal releases and retains', inject(function($ionicBackdrop) {
var el = $ionicBackdrop._getElement();
it('should require equal releases and retains', inject(function($ionicBackdrop, $timeout) {
var el = $ionicBackdrop._element;
expect(el.hasClass('ng-hide')).toBe(true);
$ionicBackdrop.retain();
$timeout.flush();
expect(el.hasClass('ng-hide')).toBe(false);
$ionicBackdrop.retain();
expect(el.hasClass('ng-hide')).toBe(false);
@@ -43,6 +39,7 @@ describe('$ionicBackdrop service', function() {
$ionicBackdrop.release();
expect(el.hasClass('ng-hide')).toBe(false);
$ionicBackdrop.release();
$timeout.flush();
expect(el.hasClass('ng-hide')).toBe(true);
}));
});

View File

@@ -0,0 +1,66 @@
describe('$animateClassToggler service', function() {
beforeEach(module('ionic'));
var toggler, el;
beforeEach(inject(function($animateClassToggler) {
el = angular.element('<div>');
toggler = $animateClassToggler(el, 'foo');
ionic.requestAnimationFrame = function(cb) { cb(); };
}));
function flush() {
inject(function($timeout) {
$timeout.flush();
});
}
it('addClass should go to toggle', function() {
spyOn(toggler, '_toggle');
toggler.addClass();
expect(toggler._toggle).toHaveBeenCalledWith(true);
});
it('removeClass should go to toggle', function() {
spyOn(toggler, '_toggle');
toggler.removeClass();
expect(toggler._toggle).toHaveBeenCalledWith(false);
});
it('toggle should add class and remove class', function() {
toggler._toggle(true);
flush();
expect(el.hasClass('foo')).toBe(true);
toggler._toggle(false);
flush();
expect(el.hasClass('foo')).toBe(false);
});
it('toggle should set nextOperation if animating', function() {
toggler._animating = true;
toggler._toggle(true);
expect(toggler._nextOperation).toBe('addClass');
toggler._toggle(false);
expect(toggler._nextOperation).toBe('removeClass');
});
it('animate should set animating to true and animationDone when done', inject(function($rootScope, $animate) {
spyOn($rootScope, '$evalAsync').andCallThrough();
spyOn($animate, 'addClass');
toggler._animate('addClass');
expect(toggler._animating).toBe(true);
expect($rootScope.$evalAsync).toHaveBeenCalled();
flush();
expect($animate.addClass).toHaveBeenCalledWith(el, 'foo', toggler._animationDone);
}));
it('animationDone should use nextOperation and unset animating if no more', function() {
spyOn(toggler, '_animate').andCallThrough();
toggler._animating = true;
toggler._nextOperation = 'addClass';
toggler._animationDone();
expect(toggler._nextOperation).toBeFalsy();
expect(toggler._animate).toHaveBeenCalled();
flush();
expect(el.hasClass('foo')).toBe(true);
expect(toggler._animating).toBe(false);
});
});

View File

@@ -41,11 +41,12 @@ describe('$ionicLoading service', function() {
expect(loader.durationTimeout).toBeTruthy();
expect(loader.durationTimeout.$$timeoutId).toBeTruthy();
}));
it('should remove ng-hide after raf', inject(function($ionicLoading) {
it('should remove ng-hide', inject(function($ionicLoading, $timeout) {
var loader = TestUtil.unwrapPromise($ionicLoading._getLoader());
ionic.requestAnimationFrame = function(cb) { cb(); };
expect(loader.element.hasClass('ng-hide')).toBe(true);
loader.show({});
$timeout.flush();
expect(loader.element.hasClass('ng-hide')).toBe(false);
}));
it('should isShown = true', inject(function($ionicLoading) {
@@ -137,7 +138,7 @@ describe('$ionicLoading service', function() {
it('show should only removeClass after raf is still isShown', inject(function($ionicLoading) {
var loader = TestUtil.unwrapPromise($ionicLoading._getLoader());
var rafCallback;
ionic.requestAnimationFrame = function(cb) {
ionic.requestAnimationFrame = function(cb) {
rafCallback = cb;
};
loader.show({});

View File

@@ -187,6 +187,16 @@
el.style.marginLeft = (-el.offsetWidth) / 2 + 'px';
el.style.marginTop = (-el.offsetHeight) / 2 + 'px';
},
//Center twice, after raf, to fix a bug with ios and showing elements
//that have just been attached to the DOM.
centerElementByMarginTwice: function(el) {
ionic.requestAnimationFrame(function() {
ionic.DomUtil.centerElementByMargin(el);
ionic.requestAnimationFrame(function() {
ionic.DomUtil.centerElementByMargin(el);
});
});
},
/**
* @ngdoc method