From ee2b76864abfecbb1f8aa53c70339e82d22a23b3 Mon Sep 17 00:00:00 2001 From: Andy Joslin Date: Tue, 1 Apr 2014 08:26:54 -0600 Subject: [PATCH] fix(ionicTabBar): detect if matches state in all cases Addresses #894. --- js/ext/angular/src/directive/ionicTabBar.js | 62 ++++---- .../test/directive/ionicTabBar.unit.js | 147 +++++++++++++++--- 2 files changed, 162 insertions(+), 47 deletions(-) diff --git a/js/ext/angular/src/directive/ionicTabBar.js b/js/ext/angular/src/directive/ionicTabBar.js index 4e1c61c8ce3..6c79ad4e298 100644 --- a/js/ext/angular/src/directive/ionicTabBar.js +++ b/js/ext/angular/src/directive/ionicTabBar.js @@ -214,7 +214,8 @@ angular.module('ionic.ui.tabs', ['ionic.service.view']) controller: 'ionicTabs', compile: function(element, attr) { element.addClass('view'); - //We cannot transclude here because it breaks element.data() inheritance on compile + //We cannot use regular transclude here because it breaks element.data() + //inheritance on compile var innerElement = angular.element('
'); innerElement.append(element.contents()); element.append(innerElement); @@ -246,9 +247,26 @@ angular.module('ionic.ui.tabs', ['ionic.service.view']) }; }]) -.controller('ionicTab', ['$scope', '$ionicViewService', '$rootScope', '$element', -function($scope, $ionicViewService, $rootScope, $element) { +.controller('ionicTab', ['$scope', '$ionicViewService', '$attrs', '$location', '$state', +function($scope, $ionicViewService, $attrs, $location, $state) { this.$scope = $scope; + + //All of these exposed for testing + this.hrefMatchesState = function() { + return $attrs.href && $location.path().indexOf( + $attrs.href.replace(/^#/, '').replace(/\/$/, '') + ) === 0; + }; + this.srefMatchesState = function() { + return $attrs.uiSref && $state.includes( $attrs.uiSref.split('(')[0] ); + }; + this.navNameMatchesState = function() { + return this.navViewName && $ionicViewService.isCurrentStateNavView(this.navViewName); + }; + + this.tabMatchesState = function() { + return this.hrefMatchesState() || this.srefMatchesState() || this.navNameMatchesState(); + }; }]) /** @@ -300,27 +318,20 @@ function($rootScope, $animate, $ionicBind, $compile, $ionicViewService, $state, controller: 'ionicTab', scope: true, compile: function(element, attr) { - //Do we have a navView? - var navView = element[0].querySelector('ion-nav-view') || + var navView = element[0].querySelector('ion-nav-view') || element[0].querySelector('data-ion-nav-view'); var navViewName = navView && navView.getAttribute('name'); - var tabNavItem = angular.element( - element[0].querySelector('ion-tab-nav') || - element[0].querySelector('data-ion-tab-nav') - ).remove(); - //Remove the contents of the element so we can compile them later, if tab is selected + //We don't use regular transclusion because it breaks element inheritance var tabContent = angular.element('
') .append( element.contents().remove() ); + return function link($scope, $element, $attr, ctrls) { var childScope, childElement, tabNavElement; tabsCtrl = ctrls[0], tabCtrl = ctrls[1]; - //Remove title attribute so browser-tooltip does not apear - $element[0].removeAttribute('title'); - $ionicBind($scope, $attr, { animate: '=', onSelect: '&', @@ -337,10 +348,18 @@ function($rootScope, $animate, $ionicBind, $compile, $ionicViewService, $state, tabNavElement.remove(); }); + //Remove title attribute so browser-tooltip does not apear + $element[0].removeAttribute('title'); + if (navViewName) { - $scope.navViewName = navViewName; - $scope.$on('$stateChangeSuccess', selectTabIfMatchesState); - selectTabIfMatchesState(); + tabCtrl.navViewName = navViewName; + } + $scope.$on('$stateChangeSuccess', selectIfMatchesState); + selectIfMatchesState(); + function selectIfMatchesState() { + if (tabCtrl.tabMatchesState()) { + tabsCtrl.select($scope); + } } tabNavElement = angular.element( @@ -371,17 +390,6 @@ function($rootScope, $animate, $ionicBind, $compile, $ionicViewService, $state, } }); - function selectTabIfMatchesState() { - var href = $attr.href.replace(/^#/, ''); - var stateName = $attr.uiSref.split('(')[0]; - - if ($location.path().indexOf(href) === 0 || $state.includes(stateName)) { - // this tab's ui-view is the current one, go to it! - if ($ionicViewService.isCurrentStateNavView($scope.navViewName)) { - tabsCtrl.select($scope); - } - } - } }; } }; diff --git a/js/ext/angular/test/directive/ionicTabBar.unit.js b/js/ext/angular/test/directive/ionicTabBar.unit.js index f30520a9572..84e126036d5 100644 --- a/js/ext/angular/test/directive/ionicTabBar.unit.js +++ b/js/ext/angular/test/directive/ionicTabBar.unit.js @@ -1,4 +1,4 @@ -describe('tabs', function() { +ddescribe('tabs', function() { describe('miscellaneous', function() { beforeEach(module('ionic', function($provide) { @@ -261,12 +261,103 @@ describe('tabs', function() { }); }); - describe('ionTab directive', function() { + describe('ionicTab controller', function() { beforeEach(module('ionic')); - var tabsCtrl, tabsEl, scope; + function setup(attrs) { + var ctrl; + inject(function($controller, $rootScope) { + ctrl = $controller('ionicTab', { + $scope: $rootScope.$new(), + $attrs: attrs || {} + }); + }); + return ctrl; + } + + it('.hrefMatchesState', inject(function($location) { + spyOn($location, 'path').andReturn('/a/b/c'); + var attr = {}; + var ctrl = setup(attr); + + expect(ctrl.hrefMatchesState()).toBeFalsy(); + + attr.href = 'a'; + expect(ctrl.hrefMatchesState()).toBe(false); + + attr.href = '/a'; + expect(ctrl.hrefMatchesState()).toBe(true); + + attr.href = '#/a'; + expect(ctrl.hrefMatchesState()).toBe(true); + + attr.href = '#/a/b/c'; + expect(ctrl.hrefMatchesState()).toBe(true); + + attr.href = '#/a/b/c/'; + expect(ctrl.hrefMatchesState()).toBe(true); + + attr.href = '/a/b/c/'; + expect(ctrl.hrefMatchesState()).toBe(true); + + attr.href = '/a/b/c/d'; + expect(ctrl.hrefMatchesState()).toBe(false); + + attr.href = 'something'; + expect(ctrl.hrefMatchesState()).toBe(false); + })); + + it('.srefMatchesState', inject(function($state) { + spyOn($state, 'includes').andReturn(111); + var attr = {}; + var ctrl = setup(attr); + + expect(ctrl.srefMatchesState()).toBeFalsy(); + expect($state.includes).not.toHaveBeenCalled(); + + //We won't unit test $state.includes, only that it was called + attr.uiSref = 'abc'; + expect(ctrl.srefMatchesState()).toBe(111); + expect($state.includes).toHaveBeenCalledWith('abc'); + + $state.includes.reset(); + attr.uiSref = 'def({ param: "value" })'; + ctrl.srefMatchesState(); + expect($state.includes).toHaveBeenCalledWith('def'); + })); + + it('.navNameMatchesState', inject(function($ionicViewService) { + spyOn($ionicViewService, 'isCurrentStateNavView').andReturn(123); + + var ctrl = setup(); + expect(ctrl.navNameMatchesState()).toBeFalsy(); + + ctrl.navViewName = 'foo'; + expect(ctrl.navNameMatchesState()).toBe(123); + expect($ionicViewService.isCurrentStateNavView).toHaveBeenCalledWith('foo'); + })); + }); + + describe('ionTab directive', function() { + var tabDoesMatch; + beforeEach(module('ionic', function($controllerProvider) { + $controllerProvider.register('ionicTab', function($scope) { + this.$scope = $scope; + this.tabMatchesState = jasmine.createSpy('tabMatchesState') + .andCallFake(function() { + return tabDoesMatch; + }); + }); + })); + + var tabsCtrl, tabsEl, scope, tabEl; function setup(attrs, content) { inject(function($compile, $rootScope) { - tabsEl = angular.element(''+(content||'')+''); + tabsEl = angular.element( + '' + + ''+(content||'')+'' + + '' + ); + tabEl = tabsEl.find('ion-tab'); $compile(tabsEl)($rootScope.$new()); $rootScope.$apply(); @@ -323,32 +414,48 @@ describe('tabs', function() { expect(navItem.parent().length).toBe(0); }); - it('should not set navViewName if no child nav-view', function() { + it('should not set navViewName by default', function() { setup(); - expect(tabsCtrl.tabs[0].navViewName).toBeUndefined(); + expect(tabEl.controller('ionTab').navViewName).toBeUndefined(); }); angular.forEach(['ion-nav-view', 'data-ion-nav-view'], function(directive) { - it('should set navViewName and select when necessary if a child '+directive, inject(function($ionicViewService, $rootScope) { - var isCurrent = false; - spyOn($ionicViewService, 'isCurrentStateNavView').andCallFake(function(name) { - return isCurrent; - }); - + it('should set navViewName if a child '+directive, inject(function($ionicViewService, $rootScope) { setup('', '<' + directive + ' name="banana">'); spyOn(tabsCtrl, 'select'); var tab = tabsCtrl.tabs[0]; - expect(tab.navViewName).toBe('banana'); - expect($ionicViewService.isCurrentStateNavView).toHaveBeenCalledWith('banana'); + expect(tabEl.controller('ionTab').navViewName).toBe('banana'); + })); + }); - $ionicViewService.isCurrentStateNavView.reset(); - isCurrent = true; - $rootScope.$broadcast('$stateChangeSuccess'); + it('should call tabMatchesState on compile and if match select', function() { + setup(); + expect(tabEl.controller('ionTab').tabMatchesState).toHaveBeenCalled(); - expect($ionicViewService.isCurrentStateNavView).toHaveBeenCalledWith('banana'); - expect(tabsCtrl.select).toHaveBeenCalledWith(tab); - })); + tabDoesMatch = true; + setup(); + expect(tabEl.controller('ionTab').tabMatchesState).toHaveBeenCalled(); + }); + + it('should call selectIfMatchesState on $stateChangeSuccess', function() { + setup(); + var tabMatchesState = tabEl.controller('ionTab').tabMatchesState; + + tabMatchesState.reset(); + spyOn(tabsCtrl, 'select'); + tabDoesMatch = false; + + tabEl.scope().$broadcast('$stateChangeSuccess'); + expect(tabMatchesState).toHaveBeenCalled(); + expect(tabsCtrl.select).not.toHaveBeenCalled(); + + tabMatchesState.reset(); + tabDoesMatch = true; + + tabEl.scope().$broadcast('$stateChangeSuccess'); + expect(tabMatchesState).toHaveBeenCalled(); + expect(tabsCtrl.select).toHaveBeenCalledWith(tabEl.scope()); }); it('should transclude on $tabSelected=true', function() {