From 23f7a546467e85c6e050979309491d7811908ec5 Mon Sep 17 00:00:00 2001 From: Matthew Brandly Date: Thu, 25 Jul 2013 18:28:58 -0400 Subject: [PATCH 1/5] Remove changePage closure that was leaking memory on transition --- src/change.js | 2 +- src/navigate.js | 12 +++--- src/view.js | 104 ++++++++++++++++++++++++------------------------ 3 files changed, 60 insertions(+), 58 deletions(-) diff --git a/src/change.js b/src/change.js index 5ad2565..dbb5c79 100644 --- a/src/change.js +++ b/src/change.js @@ -17,7 +17,7 @@ angular.module('ajoslin.mobile-navigate') var OUT_CLASS = "out"; var REVERSE_CLASS = "reverse"; var DONE_CLASS = "done"; - var ANIMATION_END = "animationName" in document.documentElement.style ? "animationend" : "webkitAnimationEnd"; + var ANIMATION_END = "webkitAnimationEnd"; this.setTransitionPreset = function(transitionName, inClass, outClass) { inClass = inClass || ''; diff --git a/src/navigate.js b/src/navigate.js index 376a149..0d4d8a8 100644 --- a/src/navigate.js +++ b/src/navigate.js @@ -31,15 +31,15 @@ angular.module('ajoslin.mobile-navigate') _onceTransition = trans; }; } - + function navigate(destination, source, isBack) { $rootScope.$broadcast('$pageTransitionStart', destination, source, isBack); nav.current = nav.next; } - /* + /* * Will listen for a route change success and call the selected callback - * Only one listen is ever active, so if you press for example + * Only one listen is ever active, so if you press for example * /link1 then press back before /link1 is done, it will go listen for the back */ nav.onRouteSuccess = null; @@ -72,7 +72,7 @@ angular.module('ajoslin.mobile-navigate') $location.path(path); //Wait for successful route change before actually doing stuff nav.onRouteSuccess = function($event, next, last) { - nav.current && navHistory.push(nav.current); + nav.current && navHistory.push(nav.current.path()); nav.next = new Page(path, transition || (next.$$route && next.$$route.transition), isReverse); navigate(nav.next, nav.current, false); }; @@ -84,10 +84,10 @@ angular.module('ajoslin.mobile-navigate') nav.back = function() { if (navHistory.length > 0) { var previous = navHistory[navHistory.length-1]; - $location.path(previous.path()); + $location.path(previous); nav.onRouteSuccess = function() { navHistory.pop(); - nav.next = previous; + nav.next = new Page(previous); navigate(nav.next, nav.current, true); }; return true; diff --git a/src/view.js b/src/view.js index 9a2ed9c..f94a1a3 100644 --- a/src/view.js +++ b/src/view.js @@ -2,10 +2,10 @@ angular.module('ajoslin.mobile-navigate') .directive('mobileView', ['$rootScope', '$compile', '$controller', '$route', '$change', '$q', function($rootScope, $compile, $controller, $route, $change, $q) { - function link(scope, viewElement, attrs) { + function link(scope, viewElement, attrs) { //Insert page into dom function insertPage(page) { - var current = $route.current, + var current = $route.current, locals = current && current.locals; page.element = angular.element(document.createElement("div")); @@ -27,64 +27,66 @@ function($rootScope, $compile, $controller, $route, $change, $q) { return page; } + var transitionListener = scope.$on('$pageTransitionStart', function ($event, dest, source, reverse) { + var current = $route.current ? $route.current.$$route : {}; + var transition = reverse ? source.transition() : dest.transition(); - var currentTrans; - scope.$on('$pageTransitionStart', function ($event, dest, source, reverse) { - function changePage() { - var current = $route.current && $route.current.$$route || {}; - var transition = reverse ? source.transition() : dest.transition(); + if (source && source.element) { + var siblings = source.element.parent().children(); + for (var index = 0; index < siblings.length; index++) { + if (source.element[0] !== siblings[index]) { + angular.element(siblings[index]).remove(); + } + } + } - insertPage(dest); + insertPage(dest); - //If the page is marked as reverse, reverse the direction - if (dest.reverse() || current.reverse) { - reverse = !reverse; - } + //If the page is marked as reverse, reverse the direction + if (dest.reverse() || current.reverse) { + reverse = !reverse; + } - function doTransition() { - - var promise = $change(dest.element, (source ? source.element : null), - transition, reverse); - - promise.then(function() { - if (source) { - $rootScope.$broadcast('$pageTransitionSuccess', dest, source); - source.scope.$destroy(); - source.element.remove(); - source = undefined; - } - }); - - return promise; - } + function doTransition() { - //Set next element to display: none, then wait until transition is - //ready, then show it again. - dest.element.css('display', 'none'); - - //Allow a deferTransition expression, which is allowed to return a promise. - //The next page will be inserted, but not transitioned in until the promise - //is fulfilled. - var deferTransitionPromise = scope.$eval(attrs.deferTransition) || $q.when(); - deferTransitionPromise.cancel = function() { - cancelled = true; - //Undo display none from waiting for transition - dest.element.css('display', ''); - }; - - var cancelled = false; - deferTransitionPromise.then(function() { - if (!cancelled) { - //Undo display none from waiting for transition - dest.element.css('display', ''); - return doTransition(); + var promise = $change(dest.element, (source ? source.element : null), + transition, reverse); + + promise.then(function() { + if (source) { + $rootScope.$broadcast('$pageTransitionSuccess', dest, source); + source.scope.$destroy(); + source.scope = undefined; + source.element.remove(); + source.element = undefined; + source = undefined; + promise = null; } }); - return deferTransitionPromise; + return promise; } - currentTrans && currentTrans.cancel(); - currentTrans = changePage(dest, source, reverse); + + //Set next element to display: none, then wait until transition is + //ready, then show it again. + dest.element.css('display', 'none'); + + //Allow a deferTransition expression, which is allowed to return a promise. + //The next page will be inserted, but not transitioned in until the promise + //is fulfilled. + var deferTransitionPromise = scope.$eval(attrs.deferTransition) || $q.when(); + + deferTransitionPromise.then(function() { + //Undo display none from waiting for transition + dest.element.css('display', ''); + deferTransitionPromise = null; + return doTransition(); + }); + }); + + scope.$on('$destroy', function(){ + transitionListener(); + element.remove(); }); } return { From 6f79b7cd032644f0c8cd5ba2f916178c1df62be0 Mon Sep 17 00:00:00 2001 From: Matthew Brandly Date: Fri, 26 Jul 2013 12:41:22 -0400 Subject: [PATCH 2/5] Inject $sniffer for vendor prefix to support Gecko --- src/change.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/change.js b/src/change.js index dbb5c79..3e7cbc5 100644 --- a/src/change.js +++ b/src/change.js @@ -4,7 +4,7 @@ */ angular.module('ajoslin.mobile-navigate') -.provider('$change', function() { +.provider('$change', ['$sniffer', function($sniffer) { var transitionPresets = { //[nextClass, prevClass] //Modal: new page pops up, old page sits there until new page is over it 'modal': ['modal', ''], @@ -17,7 +17,9 @@ angular.module('ajoslin.mobile-navigate') var OUT_CLASS = "out"; var REVERSE_CLASS = "reverse"; var DONE_CLASS = "done"; - var ANIMATION_END = "webkitAnimationEnd"; + var ANIMATION_END = $sniffer.vendorPrefix ? + $sniffer.vendorPrefix.toLowerCase() + 'AnimationEnd' : + 'animationend'; this.setTransitionPreset = function(transitionName, inClass, outClass) { inClass = inClass || ''; @@ -106,4 +108,4 @@ angular.module('ajoslin.mobile-navigate') return deferred.promise; }; }]; -}); +}]); From 5e7603788ef036143f3c2c9f1af1451d8a9dacd8 Mon Sep 17 00:00:00 2001 From: Matthew Brandly Date: Fri, 26 Jul 2013 12:44:34 -0400 Subject: [PATCH 3/5] Destroy sibling's scope before removing element --- src/view.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/view.js b/src/view.js index f94a1a3..500c986 100644 --- a/src/view.js +++ b/src/view.js @@ -35,6 +35,7 @@ function($rootScope, $compile, $controller, $route, $change, $q) { var siblings = source.element.parent().children(); for (var index = 0; index < siblings.length; index++) { if (source.element[0] !== siblings[index]) { + angular.element(siblings[index]).scope().$destroy(); angular.element(siblings[index]).remove(); } } From 8ea8ce1b613b85231778b438c1d56fcee4d7e809 Mon Sep 17 00:00:00 2001 From: Matthew Brandly Date: Fri, 26 Jul 2013 12:51:31 -0400 Subject: [PATCH 4/5] Update Angular in demo to 1.1.5 --- demo/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demo/index.html b/demo/index.html index 7819c3b..b834de5 100644 --- a/demo/index.html +++ b/demo/index.html @@ -8,7 +8,7 @@ Angular Mobile Nav - + From 64d6a563f640255cca94149bae13bba79237f11a Mon Sep 17 00:00:00 2001 From: Matthew Brandly Date: Sat, 27 Jul 2013 10:02:35 -0400 Subject: [PATCH 5/5] Inject $sniffer correctly. Move ANIMATION_END into this.$get --- src/change.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/change.js b/src/change.js index 3e7cbc5..2154cd6 100644 --- a/src/change.js +++ b/src/change.js @@ -4,7 +4,7 @@ */ angular.module('ajoslin.mobile-navigate') -.provider('$change', ['$sniffer', function($sniffer) { +.provider('$change', function() { var transitionPresets = { //[nextClass, prevClass] //Modal: new page pops up, old page sits there until new page is over it 'modal': ['modal', ''], @@ -17,9 +17,6 @@ angular.module('ajoslin.mobile-navigate') var OUT_CLASS = "out"; var REVERSE_CLASS = "reverse"; var DONE_CLASS = "done"; - var ANIMATION_END = $sniffer.vendorPrefix ? - $sniffer.vendorPrefix.toLowerCase() + 'AnimationEnd' : - 'animationend'; this.setTransitionPreset = function(transitionName, inClass, outClass) { inClass = inClass || ''; @@ -30,7 +27,10 @@ angular.module('ajoslin.mobile-navigate') defaultOptions = angular.extend(defaultOptions, opts || {}); }; - this.$get = ['$q', '$rootScope', function($q, $rootScope) { + this.$get = ['$q', '$rootScope', '$sniffer', function($q, $rootScope, $sniffer) { + var ANIMATION_END = $sniffer.vendorPrefix ? + $sniffer.vendorPrefix.toLowerCase() + 'AnimationEnd' : + 'animationend'; return function change(next, prev, transType, reverse, options) { options = angular.extend(options || {}, defaultOptions); @@ -108,4 +108,4 @@ angular.module('ajoslin.mobile-navigate') return deferred.promise; }; }]; -}]); +});