From b4e4055a06408f6e69a8f9f689b8dc4d515c85be Mon Sep 17 00:00:00 2001 From: Adam Bradley Date: Wed, 19 Nov 2014 11:38:14 -0600 Subject: [PATCH] refactor(backButton): separate show/enable logic Previously the showBack property was setting if the back button should or should not be enabled, and it was also used if the back button should be hidden or not for the view. Changed it so there are now two concepts, where showBack is visual only, and enableBack determines if it should show according to $ionicHistory and navigation info. --- js/angular/controller/headerBarController.js | 24 +++++++++++++++---- js/angular/controller/navBarController.js | 18 ++++++++++---- js/angular/controller/navViewController.js | 12 +++++++--- js/angular/controller/viewController.js | 5 ++-- js/angular/directive/menuToggle.js | 2 +- js/angular/service/history.js | 2 +- js/angular/service/navBarDelegate.js | 2 +- js/angular/service/viewSwitcher.js | 8 +++---- test/unit/angular/directive/view.unit.js | 23 +++++++----------- test/unit/angular/service/history.unit.js | 6 ++--- .../unit/angular/service/viewSwitcher.unit.js | 8 +++---- 11 files changed, 68 insertions(+), 42 deletions(-) diff --git a/js/angular/controller/headerBarController.js b/js/angular/controller/headerBarController.js index 759364516b..b4221fd578 100644 --- a/js/angular/controller/headerBarController.js +++ b/js/angular/controller/headerBarController.js @@ -21,7 +21,8 @@ function($scope, $element, $attrs, $q, $ionicConfig, $ionicHistory) { var titleLeft = 0; var titleRight = 0; var titleCss = ''; - var isBackShown; + var isBackEnabled = false; + var isBackShown = false; var titleTextWidth = 0; @@ -40,13 +41,25 @@ function($scope, $element, $attrs, $q, $ionicConfig, $ionicHistory) { }; + self.enableBack = function(shouldEnable) { + // whether or not the back button show be visible, according + // to the navigation and history + if (arguments.length && shouldEnable !== isBackEnabled) { + var backBtnEle = getEle(BACK_BUTTON); + backBtnEle && backBtnEle.classList[ shouldEnable ? 'remove' : 'add' ](HIDE); + isBackEnabled = shouldEnable; + } + return isBackEnabled; + }; + + self.showBack = function(shouldShow) { + // different from enableBack() because this will always have the back + // visually hidden if false, even if the history says it should show if (arguments.length && shouldShow !== isBackShown) { var backBtnEle = getEle(BACK_BUTTON); - if (backBtnEle) { - backBtnEle.classList[ shouldShow ? 'remove' : 'add' ](HIDE); - isBackShown = shouldShow; - } + if (backBtnEle) backBtnEle.style.display = (shouldShow ? '' : 'none'); + isBackShown = shouldShow; } return isBackShown; }; @@ -109,6 +122,7 @@ function($scope, $element, $attrs, $q, $ionicConfig, $ionicHistory) { defaultTitleEle.classList.remove(HIDE); } } + self.showBack(true); }; diff --git a/js/angular/controller/navBarController.js b/js/angular/controller/navBarController.js index 53b90d9654..12b5670ef2 100644 --- a/js/angular/controller/navBarController.js +++ b/js/angular/controller/navBarController.js @@ -77,6 +77,9 @@ function($scope, $element, $attrs, $compile, $timeout, $ionicNavBarDelegate, $io var headerBarInstance = { isActive: isActive, + enableBack: function(shouldEnable) { + headerBarCtrl.enableBack(shouldEnable); + }, showBack: function(shouldShow) { headerBarCtrl.showBack(shouldShow); }, @@ -209,6 +212,7 @@ function($scope, $element, $attrs, $compile, $timeout, $ionicNavBarDelegate, $io var leavingHeaderBar = self.isInitialized ? getOnScreenHeaderBar() : null; // update if the entering header should show the back button or not + self.enableBackButton(viewData.enableBack, enteringHeaderBar); self.showBackButton(viewData.showBack, enteringHeaderBar); // update the entering header bar's title @@ -326,11 +330,17 @@ function($scope, $element, $attrs, $compile, $timeout, $ionicNavBarDelegate, $io }; - self.showBackButton = function(show, headerBar) { + self.enableBackButton = function(shouldEnable, headerBar) { headerBar = headerBar || getOnScreenHeaderBar(); - headerBar && headerBar.showBack(show); - $scope.$isBackButtonShown = !!show; - return !!show; + headerBar && headerBar.enableBack(shouldEnable); + }; + + + self.showBackButton = function(shouldShow, headerBar) { + headerBar = headerBar || getOnScreenHeaderBar(); + headerBar && headerBar.showBack(shouldShow); + $scope.$isBackButtonShown = !!shouldShow; + return !!shouldShow; }; diff --git a/js/angular/controller/navViewController.js b/js/angular/controller/navViewController.js index bb393da0d9..ebdc93aec3 100644 --- a/js/angular/controller/navViewController.js +++ b/js/angular/controller/navViewController.js @@ -89,7 +89,7 @@ function($scope, $element, $attrs, $ionicNavBarDelegate, $ionicHistory, $ionicVi // the view is now compiled, in the dom and linked, now lets transition the views. // this uses a callback incase THIS nav-view has a nested nav-view, and after the NESTED // nav-view links, the NESTED nav-view would update which direction THIS nav-view should use - switcher.transition(self.direction(), registerData.showBack); + switcher.transition(self.direction(), registerData.enableBack); }); }; @@ -111,9 +111,15 @@ function($scope, $element, $attrs, $ionicNavBarDelegate, $ionicHistory, $ionicVi }; - self.showBackButton = function(val) { + self.enableBackButton = function(shouldEnable) { var associatedNavBarCtrl = getAssociatedNavBarCtrl(); - associatedNavBarCtrl && associatedNavBarCtrl.showBackButton(val); + associatedNavBarCtrl && associatedNavBarCtrl.enableBackButton(shouldEnable); + }; + + + self.showBackButton = function(shouldShow) { + var associatedNavBarCtrl = getAssociatedNavBarCtrl(); + associatedNavBarCtrl && associatedNavBarCtrl.showBackButton(shouldShow); }; diff --git a/js/angular/controller/viewController.js b/js/angular/controller/viewController.js index 3be14b5a80..b1223d1fbe 100644 --- a/js/angular/controller/viewController.js +++ b/js/angular/controller/viewController.js @@ -54,7 +54,8 @@ function($scope, $element, $attrs, $compile, $ionicViewSwitcher) { transition: transData.transition, transitionId: transData.transitionId, shouldAnimate: transData.shouldAnimate, - showBack: transData.showBack && !attrTrue('hideBackButton'), + enableBack: transData.enableBack, + showBack: !attrTrue('hideBackButton'), buttons: buttons, navBarDelegate: navBarDelegateHandle || null, showNavBar: !attrTrue('hideNavBar'), @@ -111,7 +112,7 @@ function($scope, $element, $attrs, $compile, $ionicViewSwitcher) { function attrTrue(key) { - return $attrs[key] == 'true' || $attrs[key] === ''; + return !!$scope.$eval($attrs[key]); } diff --git a/js/angular/directive/menuToggle.js b/js/angular/directive/menuToggle.js index 9079311cb1..41aee9bfb8 100644 --- a/js/angular/directive/menuToggle.js +++ b/js/angular/directive/menuToggle.js @@ -26,7 +26,7 @@ IonicModule restrict: 'AC', link: function($scope, $element, $attr) { $scope.$on('$ionicView.beforeEnter', function(ev, viewData) { - if (viewData.showBack) { + if (viewData.enableBack) { var sideMenuCtrl = $element.inheritedData('$ionSideMenusController'); if (!sideMenuCtrl.enableMenuWithBackViews()) { $element.addClass('hide'); diff --git a/js/angular/service/history.js b/js/angular/service/history.js index 15f3fdd3fe..08ed93bada 100644 --- a/js/angular/service/history.js +++ b/js/angular/service/history.js @@ -406,7 +406,7 @@ function($rootScope, $state, $location, $window, $ionicViewSwitcher) { action: action, direction: direction, historyId: historyId, - showBack: !!(viewHistory.backView && viewHistory.backView.historyId === viewHistory.currentView.historyId), + enableBack: !!(viewHistory.backView && viewHistory.backView.historyId === viewHistory.currentView.historyId), isHistoryRoot: (viewHistory.currentView.index === 0), ele: ele }; diff --git a/js/angular/service/navBarDelegate.js b/js/angular/service/navBarDelegate.js index 60ddbeadde..503b7c850f 100644 --- a/js/angular/service/navBarDelegate.js +++ b/js/angular/service/navBarDelegate.js @@ -40,7 +40,7 @@ IonicModule * @name $ionicNavBarDelegate#showBackButton * @description * Set/get whether the {@link ionic.directive:ionNavBackButton} is shown - * (if it exists). + * (if it exists and there is a previous view that can be navigated to). * @param {boolean=} show Whether to show the back button. * @returns {boolean} Whether the back button is shown. */ diff --git a/js/angular/service/viewSwitcher.js b/js/angular/service/viewSwitcher.js index 6f315a0866..c2cac698ea 100644 --- a/js/angular/service/viewSwitcher.js +++ b/js/angular/service/viewSwitcher.js @@ -46,7 +46,7 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon return locals && locals.$$state && locals.$$state.self || {}; } - function getTransitionData(viewLocals, enteringEle, direction, showBack, view) { + function getTransitionData(viewLocals, enteringEle, direction, enableBack, view) { // Priority // 1) attribute directive on the button/link to this view // 2) entering element's attribute @@ -63,7 +63,7 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon transition: transition, direction: direction, shouldAnimate: (transition !== 'none' && direction !== 'none'), - showBack: !!showBack + enableBack: !!enableBack }); } @@ -207,8 +207,8 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon $timeout(callback, 16); }, - transition: function(direction, showBack) { - var enteringData = getTransitionData(viewLocals, enteringEle, direction, showBack, enteringView); + transition: function(direction, enableBack) { + var enteringData = getTransitionData(viewLocals, enteringEle, direction, enableBack, enteringView); var leavingData = extend(extend({}, enteringData), getViewData(leavingView)); enteringData.transitionId = leavingData.transitionId = transitionId; enteringData.fromCache = !!alreadyInDom; diff --git a/test/unit/angular/directive/view.unit.js b/test/unit/angular/directive/view.unit.js index 1f32d21aca..110f5e12c4 100644 --- a/test/unit/angular/directive/view.unit.js +++ b/test/unit/angular/directive/view.unit.js @@ -13,6 +13,7 @@ describe('ionView directive', function() { beforeEnter: function(d) { beforeEnterData = d; }, title: jasmine.createSpy('title'), showBar: jasmine.createSpy('showBar'), + enableBackButton: jasmine.createSpy('enableBackButton'), showBackButton: jasmine.createSpy('showBackButton') }); content && el.html(content); @@ -47,19 +48,13 @@ describe('ionView directive', function() { expect(el.html()).toBe('some html'); }); - it('should call ionNavViewController.beforeEnter with showNavBar=false and hide-nav-bar=true attr', inject(function($rootScope) { + it('should call ionNavViewController.beforeEnter with showNavBar=false w/ hide-nav-bar="true" attr', inject(function($rootScope) { var el = setup('hide-nav-bar="true"'); $rootScope.$broadcast('$ionicView.beforeEnter', {}); expect( beforeEnterData.showNavBar ).toBe(false); })); - it('should call ionNavViewController.beforeEnter with showNavBar=false and hide-nav-bar="" attr', inject(function($rootScope) { - var el = setup('hide-nav-bar'); - $rootScope.$broadcast('$ionicView.beforeEnter', {}); - expect( beforeEnterData.showNavBar ).toBe(false); - })); - - it('should call ionNavViewController.beforeEnter with showNavBar=true and hide-nav-bar=false attr', inject(function($rootScope) { + it('should call ionNavViewController.beforeEnter with showNavBar=true and hide-nav-bar="false" attr', inject(function($rootScope) { var el = setup('hide-nav-bar="false"'); $rootScope.$broadcast('$ionicView.beforeEnter', {}); expect( beforeEnterData.showNavBar ).toBe(true); @@ -83,27 +78,27 @@ describe('ionView directive', function() { expect( beforeEnterData.title ).toBe('my title'); })); - it('should not showBack with hide-back-button attr', inject(function($rootScope) { + it('should not enableBack with hide-back-button attr', inject(function($rootScope) { var el = setup('hide-back-button="true"'); $rootScope.$broadcast('$ionicView.beforeEnter', { - showBack: true + enableBack: true }); expect( beforeEnterData.showBack ).toBe(false); $rootScope.shouldShowBack = false; var el = setup('hide-back-button="shouldShowBack"'); $rootScope.$broadcast('$ionicView.beforeEnter', { - showBack: true + enableBack: true }); expect( beforeEnterData.showBack ).toBe(false); })); - it('should showBack without hide-back-button but no showBack from transition', inject(function($rootScope) { + it('should enableBack without hide-back-button but no enableBack from transition', inject(function($rootScope) { var el = setup(); $rootScope.$broadcast('$ionicView.beforeEnter', { - showBack: false + enableBack: false }); - expect( beforeEnterData.showBack ).toBe(false); + expect( beforeEnterData.enableBack ).toBe(false); })); it('should be receive delegateHandle from child ionNavBar', inject(function($rootScope) { diff --git a/test/unit/angular/service/history.unit.js b/test/unit/angular/service/history.unit.js index 885849fb79..71d349f84c 100644 --- a/test/unit/angular/service/history.unit.js +++ b/test/unit/angular/service/history.unit.js @@ -1005,7 +1005,7 @@ describe('Ionic History', function() { rootScope.$apply(); var infoReg = ionicHistory.register({}, false); expect(infoReg.direction).toEqual('none'); - expect(infoReg.showBack).toEqual(true); + expect(infoReg.enableBack).toEqual(true); expect(ionicHistory.viewHistory().histories[infoReg.historyId].stack.length).toEqual(2); expect(ionicHistory.viewHistory().backView.viewId).toBe(homeReg.viewId); })); @@ -1021,7 +1021,7 @@ describe('Ionic History', function() { $state.go('info'); rootScope.$apply(); var infoReg = ionicHistory.register({}, false); - expect(infoReg.showBack).toEqual(false); + expect(infoReg.enableBack).toEqual(false); expect(infoReg.direction).toEqual('forward'); expect(ionicHistory.viewHistory().histories[infoReg.historyId].stack.length).toEqual(2); expect(ionicHistory.viewHistory().backView).toBe(null); @@ -1038,7 +1038,7 @@ describe('Ionic History', function() { $state.go('info'); rootScope.$apply(); var infoReg = ionicHistory.register({}, false); - expect(infoReg.showBack).toEqual(false); + expect(infoReg.enableBack).toEqual(false); expect(infoReg.direction).toEqual('forward'); expect(ionicHistory.viewHistory().histories[infoReg.historyId].stack.length).toEqual(1); expect(ionicHistory.viewHistory().backView).toBe(null); diff --git a/test/unit/angular/service/viewSwitcher.unit.js b/test/unit/angular/service/viewSwitcher.unit.js index 4082e45493..590f53baaf 100644 --- a/test/unit/angular/service/viewSwitcher.unit.js +++ b/test/unit/angular/service/viewSwitcher.unit.js @@ -79,15 +79,15 @@ describe('Ionic View Switcher', function() { expect(d.direction).toEqual('forward'); })); - it('should set showBack when the view data sets it', inject(function($ionicViewSwitcher) { + it('should set enableBack when the view data sets it', inject(function($ionicViewSwitcher) { var d = $ionicViewSwitcher.getTransitionData(null, null, null, true); - expect(d.showBack).toEqual(true); + expect(d.enableBack).toEqual(true); d = $ionicViewSwitcher.getTransitionData(null, null, null, false); - expect(d.showBack).toEqual(false); + expect(d.enableBack).toEqual(false); d = $ionicViewSwitcher.getTransitionData(null, null, null, null); - expect(d.showBack).toEqual(false); + expect(d.enableBack).toEqual(false); })); it('should get an empty entering element with an empty navViewElement', inject(function($ionicViewSwitcher) {