From 4833467d86f3484159dd7de46a8a38c38ab44178 Mon Sep 17 00:00:00 2001 From: Adam Bradley Date: Sun, 16 Nov 2014 00:25:25 -0600 Subject: [PATCH] fix($ionicHistory): pop duplicate view registers --- js/angular/controller/navViewController.js | 3 +- js/angular/service/history.js | 59 ++++++--- js/angular/service/viewSwitcher.js | 125 ++++++++++-------- test/unit/angular/service/history.unit.js | 30 ++++- .../unit/angular/service/viewSwitcher.unit.js | 18 +-- 5 files changed, 150 insertions(+), 85 deletions(-) diff --git a/js/angular/controller/navViewController.js b/js/angular/controller/navViewController.js index 3367ec9335..18c8b26a7f 100644 --- a/js/angular/controller/navViewController.js +++ b/js/angular/controller/navViewController.js @@ -33,8 +33,7 @@ function($scope, $element, $attrs, $ionicNavBarDelegate, $ionicHistory, $ionicVi self.register = function(viewLocals) { // register that a view is coming in and get info on how it should transition - var isAbstractView = viewLocals && viewLocals.$$state && viewLocals.$$state.self && viewLocals.$$state.self.abstract; - var registerData = $ionicHistory.register($scope, isAbstractView); + var registerData = $ionicHistory.register($scope, viewLocals); // update which direction self.update(registerData); diff --git a/js/angular/service/history.js b/js/angular/service/history.js index fcc5064d51..90f15864d7 100644 --- a/js/angular/service/history.js +++ b/js/angular/service/history.js @@ -21,7 +21,8 @@ IonicModule '$state', '$location', '$window', -function($rootScope, $state, $location, $window) { + '$ionicViewSwitcher', +function($rootScope, $state, $location, $window, $ionicViewSwitcher) { // history actions while navigating views var ACTION_INITIAL_VIEW = 'initialView'; @@ -169,7 +170,7 @@ function($rootScope, $state, $location, $window) { return { - register: function(parentScope, isAbstractView) { + register: function(parentScope, viewLocals) { var currentStateId = getCurrentStateId(), hist = getHistory(parentScope), @@ -180,15 +181,8 @@ function($rootScope, $state, $location, $window) { action = null, direction = DIRECTION_NONE, historyId = hist.historyId, - tmp, x; - - if (isAbstractView) { - // abstract states should not register themselves in the history stack - return { - action: 'abstractView', - direction: DIRECTION_NONE - }; - } + url = $location.url(), + tmp, x, ele; if (lastStateId !== currentStateId) { lastStateId = currentStateId; @@ -284,6 +278,17 @@ function($rootScope, $state, $location, $window) { } else { + // create an element from the viewLocals template + ele = $ionicViewSwitcher.createViewEle(viewLocals); + if (this.isAbstractEle(ele)) { + console.log('VIEW', 'abstractView', DIRECTION_NONE, viewHistory.currentView); + return { + action: 'abstractView', + direction: DIRECTION_NONE, + ele: ele + }; + } + // set a new unique viewId viewId = ionic.Utils.nextUid(); @@ -349,7 +354,7 @@ function($rootScope, $state, $location, $window) { stateId: currentStateId, stateName: this.currentStateName(), stateParams: getCurrentStateParams(), - url: $location.url() + url: url }); // add the new view to this history's stack @@ -375,9 +380,24 @@ function($rootScope, $state, $location, $window) { setNavViews(viewId); - hist.cursor = viewHistory.currentView.index; + if (viewHistory.backView && historyId == viewHistory.backView.historyId && currentStateId == viewHistory.backView.stateId && url == viewHistory.backView.url) { + for (x = 0; x < hist.stack.length; x++) { + if (hist.stack[x].viewId == viewId) { + action = 'dupNav'; + direction = DIRECTION_NONE; + hist.stack[x - 1].forwardViewId = viewHistory.forwardView = null; + viewHistory.currentView.index = viewHistory.backView.index; + viewHistory.currentView.backViewId = viewHistory.backView.backViewId; + viewHistory.backView = getBackView(viewHistory.backView); + hist.stack.splice(x, 1); + break; + } + } + } - console.log('VIEW:', viewId, (viewHistory.views[viewId] && viewHistory.views[viewId].url), ' history:', historyId, ' action:', action, ' direction:', direction, ' ishistoryRoot:', viewHistory.currentView.index === 0); + console.log('VIEW', action, direction, viewHistory.currentView); + + hist.cursor = viewHistory.currentView.index; return { viewId: viewId, @@ -385,7 +405,8 @@ function($rootScope, $state, $location, $window) { direction: direction, historyId: historyId, showBack: !!(viewHistory.backView && viewHistory.backView.historyId === viewHistory.currentView.historyId), - isHistoryRoot: (viewHistory.currentView.index === 0) + isHistoryRoot: (viewHistory.currentView.index === 0), + ele: ele }; }, @@ -586,10 +607,18 @@ function($rootScope, $state, $location, $window) { } } return nextViewOptions; + }, + + isAbstractEle: function(ele) { + return !!(ele && (isAbstractTag(ele) || isAbstractTag(ele.children()))); } }; + function isAbstractTag(ele) { + return ele && ele.length && /ion-side-menus|ion-tabs/i.test(ele[0].tagName); + } + }]) .run([ diff --git a/js/angular/service/viewSwitcher.js b/js/angular/service/viewSwitcher.js index 01439853d4..483bffbe7d 100644 --- a/js/angular/service/viewSwitcher.js +++ b/js/angular/service/viewSwitcher.js @@ -16,6 +16,7 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon var TRANSITIONEND_EVENT = 'webkitTransitionEnd transitionend'; var DATA_NO_CACHE = '$noCache'; + var DATA_DESTROY_ELE = '$destroyEle'; var DATA_ELE_IDENTIFIER = '$eleId'; var DATA_ACTIVE_ELE_IDENTIFIER = '$activeEleId'; var DATA_VIEW_ACCESSED = '$accessed'; @@ -33,20 +34,7 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon ionic.transition = ionic.transition || {}; ionic.transition.isActive = false; var isActiveTimer; - - - function createViewElement(viewLocals) { - var containerEle = $document[0].createElement('div'); - if (viewLocals && viewLocals.$template) { - containerEle.innerHTML = viewLocals.$template; - if (containerEle.children.length === 1) { - containerEle.children[0].classList.add('pane'); - return jqLite(containerEle.children[0]); - } - } - containerEle.className = "pane"; - return jqLite(containerEle); - } + var cachedAttr = ionic.DomUtil.cachedAttr; function getViewElementIdentifier(locals, view) { if (viewState(locals).abstract) return viewState(locals).name; @@ -70,8 +58,8 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon var state = viewState(viewLocals); enteringView = enteringView || {}; - var transition = nextTransition || ionic.DomUtil.cachedAttr(enteringEle, 'view-transition') || state.viewTransition || $ionicConfig.views.transition() || 'ios'; - direction = nextDirection || ionic.DomUtil.cachedAttr(enteringEle, 'view-direction') || state.viewDirection || direction || 'none'; + var transition = nextTransition || cachedAttr(enteringEle, 'view-transition') || state.viewTransition || $ionicConfig.views.transition() || 'ios'; + direction = nextDirection || cachedAttr(enteringEle, 'view-direction') || state.viewDirection || direction || 'none'; var shouldAnimate = (transition !== 'none' && direction !== 'none'); return { @@ -87,11 +75,21 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon } function navViewAttr(ele, value) { - ionic.DomUtil.cachedAttr(ele, NAV_VIEW_ATTR, value); + cachedAttr(ele, NAV_VIEW_ATTR, value); } function historyCursorAttr(ele, value) { - ionic.DomUtil.cachedAttr(ele, HISTORY_CURSOR_ATTR, value); + cachedAttr(ele, HISTORY_CURSOR_ATTR, value); + } + + function destroyViewEle(ele) { + // we found an element that should be removed + // destroy its scope, then remove the element + if (ele) { + var viewScope = ele.scope(); + viewScope && viewScope.$destroy(); + ele.remove(); + } } @@ -110,14 +108,14 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon ionicViewSwitcher.isTransitioning(true); $ionicClickBlock.show(); - switcher.loadViewElements(); + switcher.loadViewElements(registerData); switcher.render(registerData, function() { callback && callback(); }); }, - loadViewElements: function() { + loadViewElements: function(registerData) { var viewEle, viewElements = navViewElement.children(); var enteringEleIdentifier = getViewElementIdentifier(viewLocals, enteringView); var navViewActiveEleId = navViewElement.data(DATA_ACTIVE_ELE_IDENTIFIER); @@ -127,7 +125,14 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon if (viewEle.data(DATA_ELE_IDENTIFIER) === enteringEleIdentifier) { // we found an existing element in the DOM that should be entering the view - enteringEle = viewEle; + if (viewEle.data(DATA_NO_CACHE)) { + // the existing element should not be cached, don't use it + viewEle.data(DATA_ELE_IDENTIFIER, enteringEleIdentifier + ionic.Utils.nextUid()); + viewEle.data(DATA_DESTROY_ELE, true); + + } else { + enteringEle = viewEle; + } } else if (viewEle.data(DATA_ELE_IDENTIFIER) === navViewActiveEleId) { leavingEle = viewEle; @@ -141,13 +146,15 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon if (!alreadyInDom) { // still no existing element to use // create it using existing template/scope/locals - enteringEle = createViewElement(viewLocals); + enteringEle = registerData.ele || ionicViewSwitcher.createViewEle(viewLocals); // existing elements in the DOM are looked up by their state name and state id enteringEle.data(DATA_ELE_IDENTIFIER, enteringEleIdentifier); } navViewElement.data(DATA_ACTIVE_ELE_IDENTIFIER, enteringEleIdentifier); + + registerData.ele = null; }, render: function(registerData, callback) { @@ -199,8 +206,8 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon var transData = getTransitionData(viewLocals, enteringEle, direction, enteringView, showBack); transData.transitionId = transitionId; - ionic.DomUtil.cachedAttr(enteringEle.parent(), 'nav-view-transition', transData.transition); - ionic.DomUtil.cachedAttr(enteringEle.parent(), 'nav-view-direction', transData.direction); + cachedAttr(enteringEle.parent(), 'nav-view-transition', transData.transition); + cachedAttr(enteringEle.parent(), 'nav-view-direction', transData.direction); // cancel any previous transition complete fallbacks $timeout.cancel(enteringEle.data(DATA_FALLBACK_TIMER)); @@ -293,45 +300,40 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon }, cleanup: function(transData) { - var viewElements = navViewElement.children(); - var viewElementsLength = viewElements.length; - var x, viewElement, removableEle; - // check if any views should be removed if (leavingEle && transData.direction == 'back' && !$ionicConfig.views.forwardCache()) { // if they just navigated back we can destroy the forward view // do not remove forward views if cacheForwardViews config is true - removableEle = leavingEle; + destroyViewEle(leavingEle); + } - } else if (leavingEle && leavingEle.data(DATA_NO_CACHE)) { - // remove if the leaving element has DATA_NO_CACHE===false - removableEle = leavingEle; + var viewElements = navViewElement.children(); + var viewElementsLength = viewElements.length; + var x, viewElement; + var removeOldestAccess = (viewElementsLength - 1) > $ionicConfig.views.maxCache(); + var removableEle; + var oldestAccess = Date.now(); - } else if ((viewElementsLength - 1) > $ionicConfig.views.maxCache()) { - // check to see if we have more cached views than we should - // the total number of child elements has exceeded how many to keep in the DOM - var oldestAccess = Date.now(); + for (x = 0; x < viewElementsLength; x++) { + viewElement = viewElements.eq(x); - for (x=0; x < viewElementsLength; x++) { - viewElement = viewElements.eq(x); + if (removeOldestAccess && viewElement.data(DATA_VIEW_ACCESSED) < oldestAccess) { + // remember what was the oldest element to be accessed so it can be destroyed + oldestAccess = viewElement.data(DATA_VIEW_ACCESSED); + removableEle = viewElements.eq(x); - if (viewElement.data(DATA_VIEW_ACCESSED) < oldestAccess) { - // remove the element that was the oldest to be accessed - oldestAccess = viewElement.data(DATA_VIEW_ACCESSED); - removableEle = viewElements.eq(x); - } + } else if (viewElement.data(DATA_DESTROY_ELE) && cachedAttr(viewElement) != VIEW_STATUS_ACTIVE) { + destroyViewEle(viewElement); } } - if (removableEle) { - // we found an element that should be removed - // destroy its scope, then remove the element - var viewScope = removableEle.scope(); - viewScope && viewScope.$destroy(); - removableEle.remove(); - } + destroyViewEle(removableEle); ionic.Utils.disconnectScope(leavingEle && leavingEle.scope()); + + if (enteringEle.data(DATA_NO_CACHE)) { + enteringEle.data(DATA_DESTROY_ELE, true); + } }, enteringEle: function() { return enteringEle; }, @@ -354,10 +356,10 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon if (viewElement.data(DATA_ELE_IDENTIFIER) === navViewActiveEleId) { navViewAttr(viewElement, VIEW_STATUS_ACTIVE); - isHistoryRoot = ionic.DomUtil.cachedAttr(viewElement, HISTORY_CURSOR_ATTR) === HISTORY_ROOT; + isHistoryRoot = cachedAttr(viewElement, HISTORY_CURSOR_ATTR) === HISTORY_ROOT; - } else if (ionic.DomUtil.cachedAttr(viewElement, NAV_VIEW_ATTR) === 'leaving' || - (ionic.DomUtil.cachedAttr(viewElement, NAV_VIEW_ATTR) === VIEW_STATUS_ACTIVE && viewElement.data(DATA_ELE_IDENTIFIER) !== navViewActiveEleId)) { + } else if (cachedAttr(viewElement, NAV_VIEW_ATTR) === 'leaving' || + (cachedAttr(viewElement, NAV_VIEW_ATTR) === VIEW_STATUS_ACTIVE && viewElement.data(DATA_ELE_IDENTIFIER) !== navViewActiveEleId)) { navViewAttr(viewElement, VIEW_STATUS_CACHED); } } @@ -366,8 +368,8 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon for (x=0; x < viewElementsLength; x++) { viewElement = viewElements.eq(x); - if (ionic.DomUtil.cachedAttr(viewElement, HISTORY_CURSOR_ATTR) === HISTORY_ROOT && - ionic.DomUtil.cachedAttr(viewElement, NAV_VIEW_ATTR) !== VIEW_STATUS_ACTIVE) { + if (cachedAttr(viewElement, HISTORY_CURSOR_ATTR) === HISTORY_ROOT && + cachedAttr(viewElement, NAV_VIEW_ATTR) !== VIEW_STATUS_ACTIVE) { historyCursorAttr(viewElement, HISTORY_AFTER_ROOT); } } @@ -399,6 +401,19 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon } } return ionic.transition.isActive; + }, + + createViewEle: function(viewLocals) { + var containerEle = $document[0].createElement('div'); + if (viewLocals && viewLocals.$template) { + containerEle.innerHTML = viewLocals.$template; + if (containerEle.children.length === 1) { + containerEle.children[0].classList.add('pane'); + return jqLite(containerEle.children[0]); + } + } + containerEle.className = "pane"; + return jqLite(containerEle); } }; diff --git a/test/unit/angular/service/history.unit.js b/test/unit/angular/service/history.unit.js index 7ff3ab7386..cb4c4eeea6 100644 --- a/test/unit/angular/service/history.unit.js +++ b/test/unit/angular/service/history.unit.js @@ -1033,11 +1033,33 @@ describe('Ionic History', function() { expect( ionicHistory.nextViewOptions({}) ).toEqual({}); })); - it('should be an abstract view', inject(function($document) { - var reg = ionicHistory.register({}, false); - expect(reg.action).not.toEqual('abstractView'); + it('should should find ion-tabs as an abstract element', inject(function($ionicHistory, $document) { + var ele = angular.element(''); + expect($ionicHistory.isAbstractEle(ele)).toBe(true); - reg = ionicHistory.register({}, true); + ele = angular.element(''); + expect($ionicHistory.isAbstractEle(ele)).toBe(false); + })); + + it('should should find ion-side-menus as an abstract element', inject(function($ionicHistory, $document) { + var ele = angular.element(''); + expect($ionicHistory.isAbstractEle(ele)).toBe(true); + + ele = angular.element(''); + expect($ionicHistory.isAbstractEle(ele)).toBe(false); + })); + + it('should should find first child thats an ion-tabs as an abstract element', inject(function($ionicHistory, $document) { + var div = angular.element('
'); + var ionTabs = angular.element(''); + div.append(ionTabs); + expect($ionicHistory.isAbstractEle(div)).toBe(true); + })); + + it('should be an abstract view', inject(function($document) { + var reg = ionicHistory.register({}, { + $template: '' + }); expect(reg.action).toEqual('abstractView'); })); diff --git a/test/unit/angular/service/viewSwitcher.unit.js b/test/unit/angular/service/viewSwitcher.unit.js index 1fae6f2296..4f84b284d3 100644 --- a/test/unit/angular/service/viewSwitcher.unit.js +++ b/test/unit/angular/service/viewSwitcher.unit.js @@ -93,14 +93,14 @@ describe('Ionic View Switcher', function() { it('should get an empty entering element with an empty navViewElement', inject(function($ionicViewSwitcher) { var navViewElement = angular.element('
'); var switcher = $ionicViewSwitcher.create(null, navViewElement, {}, {}); - switcher.loadViewElements(); + switcher.loadViewElements({}); expect(switcher.enteringEle().length).toBe(1); })); it('should not get a leaving element with an empty navViewElement', inject(function($ionicViewSwitcher) { var navViewElement = angular.element('
'); var switcher = $ionicViewSwitcher.create(null, navViewElement, {}, {}); - switcher.loadViewElements(); + switcher.loadViewElements({}); expect(switcher.leavingEle()).toBeUndefined(); })); @@ -113,7 +113,7 @@ describe('Ionic View Switcher', function() { stateId: 'STATE_ID' }; var switcher = $ionicViewSwitcher.create(null, navViewElement, viewLocals, enteringView); - switcher.loadViewElements(); + switcher.loadViewElements({}); expect(switcher.enteringEle().hasClass('locals-template')).toBe(true); expect(switcher.enteringEle().data('$eleId')).toBe('STATE_ID'); })); @@ -132,7 +132,7 @@ describe('Ionic View Switcher', function() { stateId: 'STATE_ID' }; var switcher = $ionicViewSwitcher.create($rootScope, navViewElement, viewLocals, enteringView); - switcher.loadViewElements(); + switcher.loadViewElements({}); switcher.render({}); expect(switcher.enteringEle().data('$noCache')).toBe(true); })); @@ -146,7 +146,7 @@ describe('Ionic View Switcher', function() { stateId: 'STATE_ID' }; var switcher = $ionicViewSwitcher.create($rootScope, navViewElement, viewLocals, enteringView); - switcher.loadViewElements(); + switcher.loadViewElements({}); switcher.render({}); expect(switcher.enteringEle().data('$noCache')).toBe(true); })); @@ -162,7 +162,7 @@ describe('Ionic View Switcher', function() { viewId: 'VIEW_ID' }; var switcher = $ionicViewSwitcher.create(null, navViewElement, {}, enteringView); - switcher.loadViewElements(); + switcher.loadViewElements({}); expect(switcher.enteringEle().hasClass('existing')).toBe(true); expect(switcher.enteringEle().data('$eleId')).toBe('STATE_ID'); })); @@ -177,7 +177,7 @@ describe('Ionic View Switcher', function() { viewId: 'VIEW_ID' }; var switcher = $ionicViewSwitcher.create(null, navViewElement, {}, enteringView); - switcher.loadViewElements(); + switcher.loadViewElements({}); expect(switcher.enteringEle().hasClass('existing')).toBe(true); expect(switcher.enteringEle().data('$eleId')).toBe('VIEW_ID'); })); @@ -201,7 +201,7 @@ describe('Ionic View Switcher', function() { viewId: 'VIEW_ID' }; var switcher = $ionicViewSwitcher.create(null, navViewElement, viewLocals, enteringView); - switcher.loadViewElements(); + switcher.loadViewElements({}); expect(switcher.enteringEle().hasClass('existing')).toBe(true); expect(switcher.enteringEle().data('$eleId')).toBe('ABSTRACT_STATE'); })); @@ -210,7 +210,7 @@ describe('Ionic View Switcher', function() { var navViewElement = angular.element('
'); var switcher = $ionicViewSwitcher.create($rootScope, navViewElement, {}); - switcher.loadViewElements(); + switcher.loadViewElements({}); switcher.render(function(){}); expect(switcher.enteringEle().length).toBe(1); expect(switcher.enteringEle().attr('nav-view')).toBe('stage');