From 749cd3829ccedacd552abfd4a2c607066f12c0b2 Mon Sep 17 00:00:00 2001 From: Andy Joslin Date: Fri, 14 Feb 2014 14:04:19 -0500 Subject: [PATCH] fix(navBar): animations work properly Starting a couple of versions ago, animations in navbar stopped working. I took this as a chance to fix this, and ddo a refactor to make the code more modular and testable. Lots of manual dom manipulation was offloaded to angular directives, and now we will not have bugs with end-user using interpolated class attribute on their own nav-bar and overriding our own manually added classes. --- config/karma.conf.js | 1 + .../angular/src/directive/ionicViewState.js | 281 +++++++++--------- .../angular/test/directive/ionicView.unit.js | 53 ++-- js/ext/angular/test/dom-trace.js | 5 + js/views/headerBarView.js | 59 ++-- 5 files changed, 204 insertions(+), 195 deletions(-) diff --git a/config/karma.conf.js b/config/karma.conf.js index 81915fffa0..c00d80694f 100644 --- a/config/karma.conf.js +++ b/config/karma.conf.js @@ -29,6 +29,7 @@ module.exports = function(config) { // list of files to exclude exclude: [ + 'js/ext/angular/test/dom-trace.js' ], // test results reporter to use diff --git a/js/ext/angular/src/directive/ionicViewState.js b/js/ext/angular/src/directive/ionicViewState.js index f50e98222c..45222cec61 100644 --- a/js/ext/angular/src/directive/ionicViewState.js +++ b/js/ext/angular/src/directive/ionicViewState.js @@ -17,7 +17,7 @@ * */ -angular.module('ionic.ui.viewState', ['ionic.service.view', 'ionic.service.gesture']) +angular.module('ionic.ui.viewState', ['ionic.service.view', 'ionic.service.gesture', 'ionic.ui.bindHtml']) /** * Our Nav Bar directive which updates as the controller state changes. @@ -25,144 +25,144 @@ angular.module('ionic.ui.viewState', ['ionic.service.view', 'ionic.service.gestu .directive('navBar', ['$ionicViewService', '$rootScope', '$animate', '$compile', function( $ionicViewService, $rootScope, $animate, $compile) { - /** - * Perform an animation between one tab bar state and the next. - * Right now this just animates the titles. - */ - var animate = function($scope, $element, oldTitle, data, cb) { - var title, nTitle, oTitle, titles = $element[0].querySelectorAll('.title'); - - var newTitle = data.title; - if(!oldTitle || oldTitle === newTitle) { - cb(); - return; - } - - // Clone the old title and add a new one so we can show two animating in and out - // add ng-leave and ng-enter during creation to prevent flickering when they are swapped during animation - title = angular.element(titles[0]); - oTitle = $compile('

')($scope); - title.replaceWith(oTitle); - nTitle = $compile('

')($scope); - - var insert = $element[0].firstElementChild || null; - - // Insert the new title - $animate.enter(nTitle, $element, insert && angular.element(insert), function() { - cb(); - }); - - // Remove the old title - $animate.leave(angular.element(oTitle), function() { - }); - }; - return { restrict: 'E', replace: true, scope: { + animation: '@', type: '@', - backButtonType: '@', - backButtonLabel: '@', - backButtonIcon: '@', + backType: '@backButtonType', + backLabel: '@backButtonLabel', + backIcon: '@backButtonIcon', alignTitle: '@' }, - template: '', + controller: function() {}, + template: + '', compile: function(tElement, tAttrs) { - var backBtnEle = tElement[0].querySelector('.back-button'); - if(backBtnEle) { - if(tAttrs.backButtonType) backBtnEle.className += ' ' + tAttrs.backButtonType; - - if(tAttrs.backButtonIcon && tAttrs.backButtonLabel) { - backBtnEle.innerHTML = ' ' + tAttrs.backButtonLabel; - } else if(tAttrs.backButtonLabel) { - backBtnEle.innerHTML = tAttrs.backButtonLabel; - } else if(tAttrs.backButtonIcon) { - backBtnEle.className += ' icon ' + tAttrs.backButtonIcon; - } - } return function link($scope, $element, $attr) { - var canHaveBackButton = !(!tAttrs.backButtonType && !tAttrs.backButtonLabel && !tAttrs.backButtonIcon); - $scope.enableBackButton = canHaveBackButton; + $scope.backButtonEnabled = true; - $scope.isInvisible = true; - $rootScope.$on('viewState.showNavBar', function(e, showNavBar) { - $scope.isInvisible = !showNavBar; - }); + var animationDisabled = false; + $scope.titles = []; - function setBarType(value, oldValue) { - if (oldValue) $element.removeClass(oldValue); - $element.addClass(value); - } + $scope.navBarClass = function() { + return ($scope.type ? ' ' + $scope.type : '') + + ($scope.isReverse ? ' reverse' : '') + + ($scope.isInvisible ? ' invisible' : '') + + (!animationDisabled && $scope.animation ? ' ' + $scope.animation : ''); + }; - // Initialize our header bar view which will handle resizing and aligning our title labels + $scope.isReverse = false; //default + $scope.isInvisible = true; //default + + // Initialize our header bar view which will handle + // resizing and aligning our title labels var hb = new ionic.views.HeaderBar({ el: $element[0], alignTitle: $scope.alignTitle || 'center' }); $scope.headerBarView = hb; - var updateHeaderData = function(data) { - $scope.oldTitle = $scope.currentTitle; - - $scope.currentTitle = (data && data.title ? data.title : ''); - - $scope.leftButtons = data.leftButtons; - $scope.rightButtons = data.rightButtons; - - if(typeof data.hideBackButton !== 'undefined') { - $scope.enableBackButton = data.hideBackButton !== true && canHaveBackButton; - } - - if(data.animate !== false && $attr.animation && data.title && data.navDirection) { - - $element[0].classList.add($attr.animation); - if(data.navDirection === 'back') { - $element[0].classList.add('reverse'); - } else { - $element[0].classList.remove('reverse'); - } - - animate($scope, $element, $scope.oldTitle, data, function() { - hb.align(); - }); - } else { - hb.align(); - } - }; - - $rootScope.$on('viewState.viewEnter', function(e, data) { + //Navbar events + $scope.$on('viewState.showNavBar', function(e, showNavBar) { + $scope.isInvisible = !showNavBar; + }); + $scope.$on('viewState.viewEnter', function(e, data) { updateHeaderData(data); }); - $rootScope.$on('viewState.titleUpdated', function(e, data) { - $scope.currentTitle = (data && data.title ? data.title : ''); + // All of these these are emitted from children, so we listen on parent + // so we can catch them as they bubble up + var unregisterEventListeners = [ + $scope.$parent.$on('$viewHistory.historyChange', function(e, data) { + $scope.backButtonEnabled = !!data.showBack; + }), + $scope.$parent.$on('viewState.leftButtonsChanged', function(e, data) { + $scope.leftButtons = data; + }), + $scope.$parent.$on('viewState.rightButtonsChanged', function(e, data) { + $scope.rightButtons = data; + }), + $scope.$parent.$on('viewState.showBackButton', function(e, data) { + $scope.backButtonEnabled = !!data; + }), + $scope.$parent.$on('viewState.titleUpdated', function(e, data) { + $scope.currentTitle = (data && data.title ? data.title : ''); + }) + ]; + $scope.$on('$destroy', function() { + for (var i=0; i' + + ' ' + + '{{label}}' + + '', + link: function($scope) { + $scope.goBack = goBack; } - }; }]) @@ -324,6 +308,7 @@ angular.module('ionic.ui.viewState', ['ionic.service.view', 'ionic.service.gestu if (locals === viewLocals) return; // nothing to do var renderer = $ionicViewService.getRenderer(element, attr, scope); + // Destroy previous view scope if (viewScope) { viewScope.$destroy(); @@ -352,6 +337,8 @@ angular.module('ionic.ui.viewState', ['ionic.service.view', 'ionic.service.gestu var link = $compile(newElement); viewScope = scope.$new(); + viewScope.$navDirection = viewRegisterData.navDirection; + if (locals.$$controller) { locals.$scope = viewScope; var controller = $controller(locals.$$controller, locals); diff --git a/js/ext/angular/test/directive/ionicView.unit.js b/js/ext/angular/test/directive/ionicView.unit.js index 456d4c6d3b..1d4534c068 100644 --- a/js/ext/angular/test/directive/ionicView.unit.js +++ b/js/ext/angular/test/directive/ionicView.unit.js @@ -45,13 +45,29 @@ describe('Ionic View', function() { expect(scope.$broadcast).toHaveBeenCalledWith('viewState.showBackButton', false); }); - it('should show/hide viewBack', function() { - var element = compile('')(scope); - expect(element.hasClass('hide')).toEqual(true); + it('should add/remove back button based on events', function() { + var element = compile('')(scope); + scope.$apply(); + function backButton() { + return angular.element(element[0].querySelector('.back-button')); + }; + expect(backButton().length).toEqual(1); + + scope.$broadcast('viewState.showBackButton', false); + scope.$apply(); + expect(backButton().length).toEqual(0); + scope.$broadcast('viewState.showBackButton', true); - expect(element.hasClass('hide')).toEqual(false); + scope.$apply(); + expect(backButton().length).toEqual(1); + scope.$broadcast('$viewHistory.historyChange', { showBack: false }); - expect(element.hasClass('hide')).toEqual(true); + scope.$apply(); + expect(backButton().length).toEqual(0); + + scope.$broadcast('$viewHistory.historyChange', { showBack: true }); + scope.$apply(); + expect(backButton().length).toEqual(1); }); it('should show/hide navBar', function() { @@ -89,68 +105,71 @@ describe('Ionic View', function() { it('should not have the back button if no back button attributes set', function() { var element = compile('')(scope); scope.$digest(); - var backButton = element.find('div').find('button'); + var backButton = angular.element(element[0].querySelector('.back-button')); expect(backButton.length).toEqual(0); }); it('should have the back button if back-button-type attributes set', function() { var element = compile('')(scope); scope.$digest(); - var backButton = element.find('div').find('button'); + var backButton = angular.element(element[0].querySelector('.back-button')); expect(backButton.length).toEqual(1); }); it('should have the back button if back-button-icon attributes set', function() { var element = compile('')(scope); scope.$digest(); - var backButton = element.find('div').find('button'); + var backButton = angular.element(element[0].querySelector('.back-button')); expect(backButton.length).toEqual(1); }); it('should have the back button if back-button-label attributes set', function() { var element = compile('')(scope); scope.$digest(); - var backButton = element.find('div').find('button'); + var backButton = angular.element(element[0].querySelector('.back-button')); expect(backButton.length).toEqual(1); }); it('should have the back button if all back button attributes set', function() { var element = compile('')(scope); scope.$digest(); - var backButton = element.find('div').find('button'); + var backButton = angular.element(element[0].querySelector('.back-button')); expect(backButton.length).toEqual(1); }); it('should set just a back button icon, no text', function() { var element = compile('')(scope); scope.$digest(); - var backButton = element.find('div').find('button'); + var backButton = angular.element(element[0].querySelector('.back-button')); expect(backButton.hasClass('button')).toEqual(true); expect(backButton.hasClass('button-icon')).toEqual(true); expect(backButton.hasClass('icon')).toEqual(true); expect(backButton.hasClass('ion-back')).toEqual(true); - expect(backButton.html()).toEqual(''); + expect(backButton.children().length).toEqual(0); + expect(backButton.text().trim()).toEqual(''); }); it('should set just a back button with only text, button-clear', function() { var element = compile('')(scope); - scope.$digest(); - var backButton = element.find('div').find('button'); + scope.$apply(); + var backButton = angular.element(element[0].querySelector('.back-button')); expect(backButton.hasClass('button')).toEqual(true); expect(backButton.hasClass('button-clear')).toEqual(true); expect(backButton.hasClass('icon')).toEqual(false); - expect(backButton.html()).toEqual('Back'); + expect(backButton.text().trim()).toEqual('Back'); }); it('should set a back button with an icon and text, button-icon', function() { var element = compile('')(scope); scope.$digest(); - var backButton = element.find('div').find('button'); + var backButton = angular.element(element[0].querySelector('.back-button')); expect(backButton.hasClass('button')).toEqual(true); expect(backButton.hasClass('button-icon')).toEqual(true); var icon = backButton.find('i'); expect(icon.hasClass('icon')).toEqual(true); - expect(backButton.html()).toEqual(' Back'); + expect(backButton.children()[0].tagName.toLowerCase()).toBe('i'); + expect(backButton.children()[0].className).toBe('icon ion-back'); + expect(backButton.text().trim()).toBe('Back'); }); }); diff --git a/js/ext/angular/test/dom-trace.js b/js/ext/angular/test/dom-trace.js index 7242836ec7..6313f2c0af 100644 --- a/js/ext/angular/test/dom-trace.js +++ b/js/ext/angular/test/dom-trace.js @@ -214,4 +214,9 @@ } }, false); + //Shortcuts for tests + angular.element(document).ready(function() { + window.$s = angular.element(document).scope().$root; + window.$i = angular.element(document).injector(); + }); })(window); diff --git a/js/views/headerBarView.js b/js/views/headerBarView.js index e997505047..5bf3663d30 100644 --- a/js/views/headerBarView.js +++ b/js/views/headerBarView.js @@ -17,74 +17,71 @@ * so that the header text size is maximized and aligned * correctly as long as possible. */ - align: function() { + align: function(titleSelector) { var _this = this; window.rAF(ionic.proxy(function() { - var i, c, childSize; - var childNodes = this.el.childNodes; - // Find the title element - var title = this.el.querySelector('.title'); - if(!title) { + // Find the titleEl element + var titleEl = this.el.querySelector(titleSelector || '.title'); + if(!titleEl) { return; } - + + var i, c, childSize; + var childNodes = this.el.childNodes; var leftWidth = 0; var rightWidth = 0; - var titlePos = Array.prototype.indexOf.call(childNodes, title); + var isCountingRightWidth = true; // Compute how wide the left children are - for(i = 0; i < titlePos; i++) { - childSize = null; + // Skip all titles (there may still be two titles, one leaving the dom) + // Once we encounter a titleEl, realize we are now counting the right-buttons, not left + for(i = 0; i < childNodes.length; i++) { c = childNodes[i]; - if(c.nodeType == 3) { - childSize = ionic.DomUtil.getTextBounds(c); - } else if(c.nodeType == 1) { - childSize = c.getBoundingClientRect(); + if (c.tagName && c.tagName.toLowerCase() == 'h1') { + isCountingRightWidth = false; + continue; } - if(childSize) { - leftWidth += childSize.width; - } - } - // Compute how wide the right children are - for(i = titlePos + 1; i < childNodes.length; i++) { childSize = null; - c = childNodes[i]; if(c.nodeType == 3) { childSize = ionic.DomUtil.getTextBounds(c); } else if(c.nodeType == 1) { childSize = c.getBoundingClientRect(); } if(childSize) { - rightWidth += childSize.width; + if (isCountingRightWidth) { + rightWidth += childSize.width; + } else { + leftWidth += childSize.width; + } } } var margin = Math.max(leftWidth, rightWidth) + 10; - // Size and align the header title based on the sizes of the left and + // Size and align the header titleEl based on the sizes of the left and // right children, and the desired alignment mode if(this.alignTitle == 'center') { if(margin > 10) { - title.style.left = margin + 'px'; - title.style.right = margin + 'px'; + titleEl.style.left = margin + 'px'; + titleEl.style.right = margin + 'px'; } - if(title.offsetWidth < title.scrollWidth) { + if(titleEl.offsetWidth < titleEl.scrollWidth) { if(rightWidth > 0) { - title.style.right = (rightWidth + 5) + 'px'; + titleEl.style.right = (rightWidth + 5) + 'px'; } } } else if(this.alignTitle == 'left') { - title.classList.add('title-left'); + titleEl.classList.add('titleEl-left'); if(leftWidth > 0) { - title.style.left = (leftWidth + 15) + 'px'; + titleEl.style.left = (leftWidth + 15) + 'px'; } } else if(this.alignTitle == 'right') { - title.classList.add('title-right'); + titleEl.classList.add('titleEl-right'); if(rightWidth > 0) { - title.style.right = (rightWidth + 15) + 'px'; + titleEl.style.right = (rightWidth + 15) + 'px'; } } }, this));