From d378f5500ab2eef0779338336c6a95656505ebb8 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sat, 2 Nov 2013 09:22:26 +0000 Subject: [PATCH] fix(ngInclude): only run anchorScroll after animation is done We need to wait until animations have added the content to the document before trying to `autoscroll` to anchors that may have been inserted. Fixes #4723 --- src/ng/directive/ngInclude.js | 12 ++-- test/ng/directive/ngIncludeSpec.js | 110 ++++++++++++++++++++++------- 2 files changed, 92 insertions(+), 30 deletions(-) diff --git a/src/ng/directive/ngInclude.js b/src/ng/directive/ngInclude.js index f4b84fcf252a..7ca9a48b383f 100644 --- a/src/ng/directive/ngInclude.js +++ b/src/ng/directive/ngInclude.js @@ -176,6 +176,11 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$compile' }; scope.$watch($sce.parseAsResourceUrl(srcExp), function ngIncludeWatchAction(src) { + var afterAnimation = function() { + if (isDefined(autoScrollExp) && (!autoScrollExp || scope.$eval(autoScrollExp))) { + $anchorScroll(); + } + }; var thisChangeId = ++changeCounter; if (src) { @@ -190,13 +195,8 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$compile' currentElement = clone; currentElement.html(response); - $animate.enter(currentElement, null, $element); + $animate.enter(currentElement, null, $element, afterAnimation); $compile(currentElement.contents())(currentScope); - - if (isDefined(autoScrollExp) && (!autoScrollExp || scope.$eval(autoScrollExp))) { - $anchorScroll(); - } - currentScope.$emit('$includeContentLoaded'); scope.$eval(onloadExp); }); diff --git a/test/ng/directive/ngIncludeSpec.js b/test/ng/directive/ngIncludeSpec.js index b8b0c16bdd1d..beb29da759a8 100644 --- a/test/ng/directive/ngIncludeSpec.js +++ b/test/ng/directive/ngIncludeSpec.js @@ -312,7 +312,7 @@ describe('ngInclude', function() { })); - describe('autoscoll', function() { + describe('autoscroll', function() { var autoScrollSpy; function spyOnAnchorScroll() { @@ -328,33 +328,58 @@ describe('ngInclude', function() { }; } - function changeTplAndValueTo(template, value) { - return function($rootScope, $browser) { - $rootScope.$apply(function() { - $rootScope.tpl = template; - $rootScope.value = value; - }); - }; - } - - beforeEach(module(spyOnAnchorScroll())); + beforeEach(module(spyOnAnchorScroll(), 'mock.animate')); beforeEach(inject( putIntoCache('template.html', 'CONTENT'), putIntoCache('another.html', 'CONTENT'))); - it('should call $anchorScroll if autoscroll attribute is present', inject( compileAndLink('
'), - changeTplAndValueTo('template.html'), function() { + function($rootScope, $animate, $timeout) { + + $rootScope.$apply(function () { + $rootScope.tpl = 'template.html'; + }); + + expect(autoScrollSpy).not.toHaveBeenCalled(); + $animate.flushNext('enter'); + $timeout.flush(); + expect(autoScrollSpy).toHaveBeenCalledOnce(); })); - it('should call $anchorScroll if autoscroll evaluates to true', inject( - compileAndLink('
'), - changeTplAndValueTo('template.html', true), - changeTplAndValueTo('another.html', 'some-string'), - changeTplAndValueTo('template.html', 100), function() { + it('should call $anchorScroll if autoscroll evaluates to true', + inject(function($rootScope, $compile, $animate, $timeout) { + + element = $compile('
')($rootScope); + + $rootScope.$apply(function () { + $rootScope.tpl = 'template.html'; + $rootScope.value = true; + }); + + $animate.flushNext('enter'); + $timeout.flush(); + + $rootScope.$apply(function () { + $rootScope.tpl = 'another.html'; + $rootScope.value = 'some-string'; + }); + + $animate.flushNext('leave'); + $animate.flushNext('enter'); + $timeout.flush(); + + $rootScope.$apply(function() { + $rootScope.tpl = 'template.html'; + $rootScope.value = 100; + }); + + $animate.flushNext('leave'); + $animate.flushNext('enter'); + $timeout.flush(); + expect(autoScrollSpy).toHaveBeenCalled(); expect(autoScrollSpy.callCount).toBe(3); })); @@ -362,18 +387,55 @@ describe('ngInclude', function() { it('should not call $anchorScroll if autoscroll attribute is not present', inject( compileAndLink('
'), - changeTplAndValueTo('template.html'), function() { + function($rootScope, $animate, $timeout) { + + $rootScope.$apply(function () { + $rootScope.tpl = 'template.html'; + }); + + $animate.flushNext('enter'); + $timeout.flush(); expect(autoScrollSpy).not.toHaveBeenCalled(); })); - it('should not call $anchorScroll if autoscroll evaluates to false', inject( - compileAndLink('
'), - changeTplAndValueTo('template.html', false), - changeTplAndValueTo('template.html', undefined), - changeTplAndValueTo('template.html', null), function() { + it('should not call $anchorScroll if autoscroll evaluates to false', + inject(function($rootScope, $compile, $animate, $timeout) { + + element = $compile('
')($rootScope); + + $rootScope.$apply(function () { + $rootScope.tpl = 'template.html'; + $rootScope.value = false; + }); + + $animate.flushNext('enter'); + $timeout.flush(); + + $rootScope.$apply(function () { + $rootScope.tpl = 'template.html'; + $rootScope.value = undefined; + }); + + $rootScope.$apply(function () { + $rootScope.tpl = 'template.html'; + $rootScope.value = null; + }); + expect(autoScrollSpy).not.toHaveBeenCalled(); })); + + it('should only call $anchorScroll after the "enter" animation completes', inject( + compileAndLink('
'), + function($rootScope, $animate, $timeout) { + expect(autoScrollSpy).not.toHaveBeenCalled(); + + $rootScope.$apply("tpl = 'template.html'"); + $animate.flushNext('enter'); + $timeout.flush(); + + expect(autoScrollSpy).toHaveBeenCalledOnce(); + })); }); });