From 6276506d4f133c540f7b4fc943632c3fd310b64b Mon Sep 17 00:00:00 2001 From: Adam Bradley Date: Tue, 18 Nov 2014 08:46:46 -0600 Subject: [PATCH] feat(delegateService): create filterFn to find active --- js/angular/controller/scrollController.js | 76 +++++++++--------- js/angular/service/delegateService.js | 38 +++++---- js/angular/service/navBarDelegate.js | 9 --- js/angular/service/scrollDelegate.js | 2 +- js/angular/service/viewSwitcher.js | 1 + .../controller/scrollController.unit.js | 4 +- test/unit/angular/directive/navBar.unit.js | 9 --- test/unit/angular/directive/navView.unit.js | 3 + .../angular/service/delegateService.unit.js | 77 +++++++++++++++++++ 9 files changed, 140 insertions(+), 79 deletions(-) diff --git a/js/angular/controller/scrollController.js b/js/angular/controller/scrollController.js index fbce677c4c..832a497e3a 100644 --- a/js/angular/controller/scrollController.js +++ b/js/angular/controller/scrollController.js @@ -9,36 +9,37 @@ IonicModule '$timeout', '$window', '$location', - '$rootScope', '$document', '$ionicScrollDelegate', -function($scope, scrollViewOptions, $timeout, $window, $location, $rootScope, $document, $ionicScrollDelegate) { +function($scope, scrollViewOptions, $timeout, $window, $location, $document, $ionicScrollDelegate) { var self = this; // for testing - this.__timeout = $timeout; + self.__timeout = $timeout; - this._scrollViewOptions = scrollViewOptions; //for testing + self._scrollViewOptions = scrollViewOptions; //for testing - var element = this.element = scrollViewOptions.el; - var $element = this.$element = jqLite(element); - var scrollView = this.scrollView = new ionic.views.Scroll(scrollViewOptions); + var element = self.element = scrollViewOptions.el; + var $element = self.$element = jqLite(element); + var scrollView = self.scrollView = new ionic.views.Scroll(scrollViewOptions); //Attach self to element as a controller so other directives can require this controller //through `require: '$ionicScroll' //Also attach to parent so that sibling elements can require this ($element.parent().length ? $element.parent() : $element) - .data('$$ionicScrollController', this); + .data('$$ionicScrollController', self); var deregisterInstance = $ionicScrollDelegate._registerInstance( - this, scrollViewOptions.delegateHandle + self, scrollViewOptions.delegateHandle, function() { + return !$scope.$$disconnected; + } ); if (!angular.isDefined(scrollViewOptions.bouncing)) { ionic.Platform.ready(function() { scrollView.options.bouncing = true; - if(ionic.Platform.isAndroid()) { + if (ionic.Platform.isAndroid()) { // No bouncing by default on Android scrollView.options.bouncing = false; // Faster scroll decel @@ -60,7 +61,7 @@ function($scope, scrollViewOptions, $timeout, $window, $location, $rootScope, $d }); }; - $element.on('scroll', scrollFunc ); + $element.on('scroll', scrollFunc); $scope.$on('$destroy', function() { deregisterInstance(); @@ -83,66 +84,66 @@ function($scope, scrollViewOptions, $timeout, $window, $location, $rootScope, $d scrollView && scrollView.run && scrollView.run(); }); - this.getScrollView = function() { - return this.scrollView; + self.getScrollView = function() { + return self.scrollView; }; - this.getScrollPosition = function() { - return this.scrollView.getValues(); + self.getScrollPosition = function() { + return self.scrollView.getValues(); }; - this.resize = function() { + self.resize = function() { return $timeout(resize).then(function() { $element && $element.triggerHandler('scroll.resize'); }); }; - this.scrollTop = function(shouldAnimate) { + self.scrollTop = function(shouldAnimate) { ionic.DomUtil.blurAll(); - this.resize().then(function() { + self.resize().then(function() { scrollView.scrollTo(0, 0, !!shouldAnimate); }); }; - this.scrollBottom = function(shouldAnimate) { + self.scrollBottom = function(shouldAnimate) { ionic.DomUtil.blurAll(); - this.resize().then(function() { + self.resize().then(function() { var max = scrollView.getScrollMax(); scrollView.scrollTo(max.left, max.top, !!shouldAnimate); }); }; - this.scrollTo = function(left, top, shouldAnimate) { + self.scrollTo = function(left, top, shouldAnimate) { ionic.DomUtil.blurAll(); - this.resize().then(function() { + self.resize().then(function() { scrollView.scrollTo(left, top, !!shouldAnimate); }); }; - this.zoomTo = function(zoom, shouldAnimate, originLeft, originTop) { + self.zoomTo = function(zoom, shouldAnimate, originLeft, originTop) { ionic.DomUtil.blurAll(); - this.resize().then(function() { + self.resize().then(function() { scrollView.zoomTo(zoom, !!shouldAnimate, originLeft, originTop); }); }; - this.zoomBy = function(zoom, shouldAnimate, originLeft, originTop) { + self.zoomBy = function(zoom, shouldAnimate, originLeft, originTop) { ionic.DomUtil.blurAll(); - this.resize().then(function() { + self.resize().then(function() { scrollView.zoomBy(zoom, !!shouldAnimate, originLeft, originTop); }); }; - this.scrollBy = function(left, top, shouldAnimate) { + self.scrollBy = function(left, top, shouldAnimate) { ionic.DomUtil.blurAll(); - this.resize().then(function() { + self.resize().then(function() { scrollView.scrollBy(left, top, !!shouldAnimate); }); }; - this.anchorScroll = function(shouldAnimate) { + self.anchorScroll = function(shouldAnimate) { ionic.DomUtil.blurAll(); - this.resize().then(function() { + self.resize().then(function() { var hash = $location.hash(); var elm = hash && $document[0].getElementById(hash); if (!(hash && elm)) { @@ -152,8 +153,8 @@ function($scope, scrollViewOptions, $timeout, $window, $location, $rootScope, $d var curElm = elm; var scrollLeft = 0, scrollTop = 0, levelsClimbed = 0; do { - if(curElm !== null)scrollLeft += curElm.offsetLeft; - if(curElm !== null)scrollTop += curElm.offsetTop; + if (curElm !== null) scrollLeft += curElm.offsetLeft; + if (curElm !== null) scrollTop += curElm.offsetTop; curElm = curElm.offsetParent; levelsClimbed++; } while (curElm.attributes != self.element.attributes && curElm.offsetParent); @@ -165,8 +166,8 @@ function($scope, scrollViewOptions, $timeout, $window, $location, $rootScope, $d /** * @private */ - this._setRefresher = function(refresherScope, refresherElement) { - var refresher = this.refresher = refresherElement; + self._setRefresher = function(refresherScope, refresherElement) { + var refresher = self.refresher = refresherElement; var refresherHeight = self.refresher.clientHeight || 60; scrollView.activatePullToRefresh(refresherHeight, function() { // activateCallback @@ -179,16 +180,15 @@ function($scope, scrollViewOptions, $timeout, $window, $location, $rootScope, $d // startCallback refresher.classList.add('refreshing'); refresherScope.$onRefresh(); - },function(){ + }, function() { // showCallback refresher.classList.remove('invisible'); - },function(){ + }, function() { // hideCallback refresher.classList.add('invisible'); - },function(){ + }, function() { // tailCallback refresher.classList.add('refreshing-tail'); }); }; }]); - diff --git a/js/angular/service/delegateService.js b/js/angular/service/delegateService.js index 8477006b16..7ed6c76652 100644 --- a/js/angular/service/delegateService.js +++ b/js/angular/service/delegateService.js @@ -1,10 +1,14 @@ function delegateService(methodNames) { + + function trueFn() { return true; } + return ['$log', function($log) { var delegate = this; var instances = this._instances = []; - this._registerInstance = function(instance, handle) { + this._registerInstance = function(instance, handle, filterFn) { instance.$$delegateHandle = handle; + instance.$$filterFn = filterFn || trueFn; instances.push(instance); return function deregister() { @@ -41,6 +45,7 @@ function delegateService(methodNames) { function InstanceForHandle(handle) { this.handle = handle; } + methodNames.forEach(function(methodName) { InstanceForHandle.prototype[methodName] = function() { var handle = this.handle; @@ -52,7 +57,7 @@ function delegateService(methodNames) { //This logic is repeated below; we could factor some of it out to a function //but don't because it lets this method be more performant (one loop versus 2) instances.forEach(function(instance) { - if (instance.$$delegateHandle === handle) { + if (instance.$$delegateHandle === handle && instance.$$filterFn(instance)) { matchingInstancesFound++; result = instance[methodName].apply(instance, args); //Only return the value from the first call @@ -64,8 +69,8 @@ function delegateService(methodNames) { if (!matchingInstancesFound) { return $log.warn( - 'Delegate for handle "'+this.handle+'" could not find a ' + - 'corresponding element with delegate-handle="'+this.handle+'"! ' + + 'Delegate for handle "' + this.handle + '" could not find a ' + + 'corresponding element with delegate-handle="' + this.handle + '"! ' + methodName + '() was not called!\n' + 'Possible cause: If you are calling ' + methodName + '() immediately, and ' + 'your element with delegate-handle="' + this.handle + '" is a child of your ' + @@ -76,35 +81,28 @@ function delegateService(methodNames) { return finalResult; }; + delegate[methodName] = function() { var args = arguments; + var matchingInstancesFound = 0; var finalResult; var result; //This logic is repeated above instances.forEach(function(instance, index) { - result = instance[methodName].apply(instance, args); - //Only return the value from the first call - if (index === 0) { - finalResult = result; + if (instance.$$filterFn()) { + matchingInstancesFound++; + result = instance[methodName].apply(instance, args); + //Only return the value from the first call + if (matchingInstancesFound === 1) { + finalResult = result; + } } }); return finalResult; }; - function callMethod(instancesToUse, methodName, args) { - var finalResult; - var result; - instancesToUse.forEach(function(instance, index) { - result = instance[methodName].apply(instance, args); - //Make it so the first result is the one returned - if (index === 0) { - finalResult = result; - } - }); - return finalResult; - } }); }]; } diff --git a/js/angular/service/navBarDelegate.js b/js/angular/service/navBarDelegate.js index a342258889..60ddbeadde 100644 --- a/js/angular/service/navBarDelegate.js +++ b/js/angular/service/navBarDelegate.js @@ -62,15 +62,6 @@ IonicModule * @param {string} title The new title to show. */ 'title', - /** - * @ngdoc method - * @name $ionicNavBarDelegate#update - * @description - * Updates the {@link ionic.directive:ionNavBar} with a transition using the - * supplied view data. - * @param {object} viewData An object containing `title`, `showBar` properties. - */ - 'update', // DEPRECATED, as of v1.0.0-beta14 ------- 'changeTitle', diff --git a/js/angular/service/scrollDelegate.js b/js/angular/service/scrollDelegate.js index 9531d5a6ac..f64d3ad785 100644 --- a/js/angular/service/scrollDelegate.js +++ b/js/angular/service/scrollDelegate.js @@ -145,6 +145,6 @@ IonicModule * * Example: `$ionicScrollDelegate.$getByHandle('my-handle').scrollTop();` */ - 'getByHandle' + '$getByHandle' ])); diff --git a/js/angular/service/viewSwitcher.js b/js/angular/service/viewSwitcher.js index bbafcace12..6f315a0866 100644 --- a/js/angular/service/viewSwitcher.js +++ b/js/angular/service/viewSwitcher.js @@ -211,6 +211,7 @@ function($timeout, $compile, $controller, $document, $ionicClickBlock, $ionicCon var enteringData = getTransitionData(viewLocals, enteringEle, direction, showBack, enteringView); var leavingData = extend(extend({}, enteringData), getViewData(leavingView)); enteringData.transitionId = leavingData.transitionId = transitionId; + enteringData.fromCache = !!alreadyInDom; cachedAttr(enteringEle.parent(), 'nav-view-transition', enteringData.transition); cachedAttr(enteringEle.parent(), 'nav-view-direction', enteringData.direction); diff --git a/test/unit/angular/controller/scrollController.unit.js b/test/unit/angular/controller/scrollController.unit.js index 9cab3c6790..2cf81e66eb 100644 --- a/test/unit/angular/controller/scrollController.unit.js +++ b/test/unit/angular/controller/scrollController.unit.js @@ -32,7 +32,7 @@ describe('$ionicScroll Controller', function() { spyOn($ionicScrollDelegate, '_registerInstance'); var el = setup(); expect($ionicScrollDelegate._registerInstance) - .toHaveBeenCalledWith(ctrl, undefined); + .toHaveBeenCalledWith(ctrl, undefined, jasmine.any(Function)); })); it('should register with given handle and deregister on destroy', inject(function($ionicScrollDelegate) { @@ -44,7 +44,7 @@ describe('$ionicScroll Controller', function() { delegateHandle: 'something' }); expect($ionicScrollDelegate._registerInstance) - .toHaveBeenCalledWith(ctrl, 'something'); + .toHaveBeenCalledWith(ctrl, 'something', jasmine.any(Function)); expect(deregisterSpy).not.toHaveBeenCalled(); scope.$destroy(); diff --git a/test/unit/angular/directive/navBar.unit.js b/test/unit/angular/directive/navBar.unit.js index 117830f400..718a54da22 100644 --- a/test/unit/angular/directive/navBar.unit.js +++ b/test/unit/angular/directive/navBar.unit.js @@ -156,15 +156,6 @@ describe('ionNavBar', function() { expect(el[0].querySelector('[nav-bar="active"] .title').innerText).toEqual('Night Ranger'); })); - it('should update w/ $ionicNavBarDelegate', inject(function($ionicNavBarDelegate) { - var el = setup('delegate-handle="theBestHandle"'); - var instance = $ionicNavBarDelegate.$getByHandle('theBestHandle'); - instance.update({ - showBar: false - }); - expect(el.hasClass('hide')).toBe(true); - })); - }); }); diff --git a/test/unit/angular/directive/navView.unit.js b/test/unit/angular/directive/navView.unit.js index 9cf32a3a52..813eeff598 100644 --- a/test/unit/angular/directive/navView.unit.js +++ b/test/unit/angular/directive/navView.unit.js @@ -740,6 +740,7 @@ describe('Ionic nav-view', function() { expect(beforeEnter.stateName).toEqual('page1'); expect(afterEnter.stateName).toEqual('page1'); expect(enter.stateName).toEqual('page1'); + expect(enter.fromCache).toEqual(false); expect(enter.transitionId).toEqual(1); $state.go(page2State); @@ -750,6 +751,7 @@ describe('Ionic nav-view', function() { expect(beforeEnter.stateName).toEqual('page2'); expect(afterEnter.stateName).toEqual('page2'); expect(enter.stateName).toEqual('page2'); + expect(enter.fromCache).toEqual(false); expect(enter.transitionId).toEqual(2); $state.go(page1State); @@ -760,6 +762,7 @@ describe('Ionic nav-view', function() { expect(beforeEnter.stateName).toEqual('page1'); expect(afterEnter.stateName).toEqual('page1'); expect(enter.stateName).toEqual('page1'); + expect(enter.fromCache).toEqual(true); expect(enter.transitionId).toEqual(3); })); diff --git a/test/unit/angular/service/delegateService.unit.js b/test/unit/angular/service/delegateService.unit.js index 6db8133188..1061d3a552 100644 --- a/test/unit/angular/service/delegateService.unit.js +++ b/test/unit/angular/service/delegateService.unit.js @@ -87,6 +87,44 @@ describe('DelegateFactory', function() { expect(delegate.fn()).toBe('ret2'); }); + it('should return the first active instance return value when multiple instances w/ undefined handle', function() { + var delegate = setup(['fn']); + var instance1 = { + fn: jasmine.createSpy('fn').andReturn('ret1') + }; + var instance2 = { + fn: jasmine.createSpy('fn').andReturn('ret2') + }; + delegate._registerInstance(instance1, undefined, function() { + return false; + }); + delegate._registerInstance(instance2, undefined, function() { + return true; + }); + + expect(instance1.fn).not.toHaveBeenCalled(); + expect(delegate.fn()).toBe('ret2'); + }); + + it('should return the first active instance return value when multiple instances w/ same handle', function() { + var delegate = setup(['fn']); + var instance1 = { + fn: jasmine.createSpy('fn').andReturn('ret1') + }; + var instance2 = { + fn: jasmine.createSpy('fn').andReturn('ret2') + }; + delegate._registerInstance(instance1, 'myhandle', function() { + return true; + }); + delegate._registerInstance(instance2, 'myhandle', function() { + return false; + }); + + expect(instance2.fn).not.toHaveBeenCalled(); + expect(delegate.fn()).toBe('ret1'); + }); + it('$getByHandle should return this for blank handle', function() { var delegate = setup(); expect(delegate.$getByHandle()).toBe(delegate); @@ -106,11 +144,13 @@ describe('DelegateFactory', function() { a: jasmine.createSpy('a3').andReturn('a3') }; }); + it('should return an InstanceWithHandle object with fields', function() { expect(delegate.$getByHandle('one').a).toEqual(jasmine.any(Function)); expect(delegate.$getByHandle('two').a).toEqual(jasmine.any(Function)); expect(delegate.$getByHandle('invalid').a).toEqual(jasmine.any(Function)); }); + it('should noop & warn if calling for a non-added instance', inject(function($log) { spyOn($log, 'warn'); expect(delegate.$getByHandle('one').a()).toBeUndefined(); @@ -157,5 +197,42 @@ describe('DelegateFactory', function() { expect(instance3.a).not.toHaveBeenCalled(); expect(result).toBe('a2'); }); + + it('should get the only active instance from multiple instances with the same handle', function() { + delegate._registerInstance(instance1, 'myhandle', function() { return false; }); + delegate._registerInstance(instance2, 'myhandle', function() { return true; }); + delegate._registerInstance(instance3, 'myhandle', function() { return false; }); + + var delegateInstance = delegate.$getByHandle('myhandle'); + + expect(instance1.a).not.toHaveBeenCalled(); + expect(instance2.a).not.toHaveBeenCalled(); + expect(instance3.a).not.toHaveBeenCalled(); + + var result = delegateInstance.a('test-a'); + expect(instance1.a).not.toHaveBeenCalled(); + expect(instance2.a).toHaveBeenCalledWith('test-a'); + expect(instance3.a).not.toHaveBeenCalled(); + expect(result).toBe('a2'); + }); + + it('should get active instance when multiple sname name handles, among multiple active instances', function() { + delegate._registerInstance(instance1, 'myhandle-1', function() { return true; }); + delegate._registerInstance(instance2, 'myhandle-2', function() { return false; }); + delegate._registerInstance(instance3, 'myhandle-2', function() { return true; }); + + var delegateInstance = delegate.$getByHandle('myhandle-2'); + + expect(instance1.a).not.toHaveBeenCalled(); + expect(instance2.a).not.toHaveBeenCalled(); + expect(instance3.a).not.toHaveBeenCalled(); + + var result = delegateInstance.a('test-a'); + expect(instance1.a).not.toHaveBeenCalled(); + expect(instance2.a).not.toHaveBeenCalled(); + expect(instance3.a).toHaveBeenCalledWith('test-a'); + expect(result).toBe('a3'); + }); + }); });