From 49e0dac9992d60ca098eb1dbd1acecbc74eae85c Mon Sep 17 00:00:00 2001 From: Andy Joslin Date: Mon, 24 Feb 2014 08:08:17 -0500 Subject: [PATCH] feat(ionContent): use child scope instead of isolate scope Adds new '$ionicBind' service, which takes an object containing binding definitions (similar to angular directive isolate scope definition). Allows binding of any directive attribute & expressions from a scope, letting us do normal attribute -> scope binding without having to create isolate scopes. Closes #555. Closes #669 --- js/ext/angular/src/directive/ionicContent.js | 61 +++++--- js/ext/angular/src/ionicAngular.js | 1 + js/ext/angular/src/service/ionicBind.js | 53 +++++++ .../test/directive/ionicContent.unit.js | 37 +++-- js/ext/angular/test/list-fit.html | 12 +- .../delegates/ionicScrollDelegate.unit.js | 2 +- js/ext/angular/test/service/ionicBind.unit.js | 137 ++++++++++++++++++ 7 files changed, 268 insertions(+), 35 deletions(-) create mode 100644 js/ext/angular/src/service/ionicBind.js create mode 100644 js/ext/angular/test/service/ionicBind.unit.js diff --git a/js/ext/angular/src/directive/ionicContent.js b/js/ext/angular/src/directive/ionicContent.js index f69f187d696..ea04c436f8e 100644 --- a/js/ext/angular/src/directive/ionicContent.js +++ b/js/ext/angular/src/directive/ionicContent.js @@ -18,33 +18,23 @@ angular.module('ionic.ui.content', ['ionic.ui.service', 'ionic.ui.scroll']) // The content directive is a core scrollable content area // that is part of many View hierarchies -.directive('ionContent', ['$parse', '$timeout', '$ionicScrollDelegate', '$controller', function($parse, $timeout, $ionicScrollDelegate, $controller) { +.directive('ionContent', [ + '$parse', + '$timeout', + '$ionicScrollDelegate', + '$controller', + '$ionicBind', +function($parse, $timeout, $ionicScrollDelegate, $controller, $ionicBind) { return { restrict: 'E', replace: true, - template: '
', transclude: true, require: '^?ionNavView', - scope: { - onRefresh: '&', - onRefreshOpening: '&', - onScroll: '&', - onScrollComplete: '&', - refreshComplete: '=', - onInfiniteScroll: '=', - infiniteScrollDistance: '@', - hasBouncing: '@', - scroll: '@', - padding: '@', - hasScrollX: '@', - hasScrollY: '@', - scrollbarX: '@', - scrollbarY: '@', - startX: '@', - startY: '@', - scrollEventInterval: '@' - }, - + scope: true, + template: + '
' + + '
' + + '
', compile: function(element, attr, transclude) { if(attr.hasHeader == "true") { element.addClass('has-header'); } if(attr.hasSubheader == "true") { element.addClass('has-subheader'); } @@ -60,7 +50,31 @@ angular.module('ionic.ui.content', ['ionic.ui.service', 'ionic.ui.scroll']) function prelink($scope, $element, $attr, navViewCtrl) { var clone, sc, scrollView, scrollCtrl, - c = angular.element($element.children()[0]); + scrollContent = angular.element($element[0].querySelector('.scroll')); + + transclude($scope, function(clone) { + scrollContent.append(clone); + }); + + $ionicBind($scope, $attr, { + onRefresh: '&', + onRefreshOpening: '&', + onScroll: '&', + onScrollComplete: '&', + refreshComplete: '=', + onInfiniteScroll: '&', + infiniteScrollDistance: '@', + hasBouncing: '@', + scroll: '@', + padding: '@', + hasScrollX: '@', + hasScrollY: '@', + scrollbarX: '@', + scrollbarY: '@', + startX: '@', + startY: '@', + scrollEventInterval: '@' + }); if($scope.scroll === "false") { // No scrolling @@ -92,6 +106,7 @@ angular.module('ionic.ui.content', ['ionic.ui.service', 'ionic.ui.scroll']) } } }); + //Publish scrollView to parent so children can access it scrollView = $scope.$parent.scrollView = scrollCtrl.scrollView; diff --git a/js/ext/angular/src/ionicAngular.js b/js/ext/angular/src/ionicAngular.js index e0a14257c2b..5e0fbf92833 100644 --- a/js/ext/angular/src/ionicAngular.js +++ b/js/ext/angular/src/ionicAngular.js @@ -3,6 +3,7 @@ * modules. */ angular.module('ionic.service', [ + 'ionic.service.bind', 'ionic.service.platform', 'ionic.service.actionSheet', 'ionic.service.gesture', diff --git a/js/ext/angular/src/service/ionicBind.js b/js/ext/angular/src/service/ionicBind.js new file mode 100644 index 00000000000..a06ce69abd9 --- /dev/null +++ b/js/ext/angular/src/service/ionicBind.js @@ -0,0 +1,53 @@ +angular.module('ionic.service.bind', []) +.factory('$ionicBind', ['$parse', '$interpolate', function($parse, $interpolate) { + var LOCAL_REGEXP = /^\s*([@=&])(\??)\s*(\w*)\s*$/; + return function(scope, attrs, bindDefinition) { + angular.forEach(bindDefinition || {}, function (definition, scopeName) { + //Adapted from angular.js $compile + var match = definition.match(LOCAL_REGEXP) || [], + attrName = match[3] || scopeName, + mode = match[1], // @, =, or & + parentGet, + unwatch; + + switch(mode) { + case '@': + if (!attrs[attrName]) { + return; + } + attrs.$observe(attrName, function(value) { + scope[scopeName] = value; + }); + // we trigger an interpolation to ensure + // the value is there for use immediately + if (attrs[attrName]) { + scope[scopeName] = $interpolate(attrs[attrName])(scope); + } + break; + + case '=': + if (!attrs[attrName]) { + return; + } + unwatch = scope.$watch(attrs[attrName], function(value) { + scope[scopeName] = value; + }); + //Destroy parent scope watcher when this scope is destroyed + scope.$on('$destroy', unwatch); + break; + + case '&': + /* jshint -W044 */ + if (attrs[attrName] && attrs[attrName].match(RegExp(scopeName + '\(.*?\)'))) { + throw new Error('& expression binding "' + scopeName + '" looks like it will recursively call "' + + attrs[attrName] + '" and cause a stack overflow! Please choose a different scopeName.'); + } + parentGet = $parse(attrs[attrName]); + scope[scopeName] = function(locals) { + return parentGet(scope, locals); + }; + break; + } + }); + }; +}]); diff --git a/js/ext/angular/test/directive/ionicContent.unit.js b/js/ext/angular/test/directive/ionicContent.unit.js index e9e4ec1ebff..41f18159dbb 100644 --- a/js/ext/angular/test/directive/ionicContent.unit.js +++ b/js/ext/angular/test/directive/ionicContent.unit.js @@ -1,5 +1,5 @@ describe('Ionic Content directive', function() { - var compile, element, scope; + var compile, scope; beforeEach(module('ionic')); @@ -12,22 +12,22 @@ describe('Ionic Content directive', function() { })); it('Has $ionicScroll controller', function() { - element = compile('')(scope); + var element = compile('')(scope); expect(element.controller('$ionicScroll').element).toBe(element[0]); }); it('Has content class', function() { - element = compile('')(scope); + var element = compile('')(scope); expect(element.hasClass('scroll-content')).toBe(true); }); it('Has header', function() { - element = compile('')(scope); + var element = compile('')(scope); expect(element.hasClass('has-header')).toEqual(true); }); it('should add padding classname', function() { - element = compile('')(scope); + var element = compile('')(scope); expect(element.hasClass('scroll-content')).toEqual(true); expect(element.hasClass('padding')).toEqual(false); var scrollElement = element.find('.scroll'); @@ -36,7 +36,7 @@ describe('Ionic Content directive', function() { // it('Enables bouncing by default', function() { // ionic.Platform.setPlatform('iPhone'); - // element = compile('')(scope); + // var element = compile('')(scope); // scope.$apply(); // var newScope = element.isolateScope(); // var scrollView = scope.scrollView; @@ -45,7 +45,7 @@ describe('Ionic Content directive', function() { it('Disables bouncing when has-bouncing = false', function() { ionic.Platform.setPlatform('iPhone'); - element = compile('')(scope); + var element = compile('')(scope); scope.$apply(); var newScope = element.isolateScope(); var scrollView = scope.scrollView; @@ -54,7 +54,7 @@ describe('Ionic Content directive', function() { it('Disables bouncing by default on Android', function() { ionic.Platform.setPlatform('Android'); - element = compile('')(scope); + var element = compile('')(scope); scope.$apply(); var newScope = element.isolateScope(); var scrollView = scope.scrollView; @@ -63,7 +63,7 @@ describe('Ionic Content directive', function() { it('Disables bouncing by default on Android unless has-bouncing = true', function() { ionic.Platform.setPlatform('Android'); - element = compile('')(scope); + var element = compile('')(scope); scope.$apply(); var newScope = element.isolateScope(); var scrollView = scope.scrollView; @@ -72,7 +72,7 @@ describe('Ionic Content directive', function() { it('Should set start x and y', function() { - element = compile('')(scope); + var element = compile('')(scope); scope.$apply(); var newScope = element.isolateScope(); var scrollView = scope.scrollView; @@ -139,3 +139,20 @@ describe('Ionic Content directive', function() { }); }); }); +/* Tests #555 */ +describe('Ionic Content Directive scoping', function() { + beforeEach(module('ionic', function($controllerProvider) { + $controllerProvider.register('ContentTestCtrl', function($scope){ + this.$scope = $scope; + }); + })); + it('should have same scope as content', inject(function($compile, $rootScope) { + var element = $compile('' + + '
' + + '')($rootScope.$new()); + var contentScope = element.scope(); + var ctrl = element.data('$ngControllerController'); + expect(contentScope.myForm).toBeTruthy(); + expect(ctrl.$scope.myForm).toBeTruthy(); + })); +}); diff --git a/js/ext/angular/test/list-fit.html b/js/ext/angular/test/list-fit.html index 62b9333249e..70715a8814d 100644 --- a/js/ext/angular/test/list-fit.html +++ b/js/ext/angular/test/list-fit.html @@ -14,7 +14,7 @@ - + @@ -45,7 +45,11 @@
  • 24
  • 25
  • 26
    • +
  • more {{$index}}
    • + + +     @@ -59,6 +63,12 @@

    Footer!

    $scope.$broadcast('scroll.refreshComplete'); }, 1000); }; + $scope.more = []; + $scope.addMore = function() { + for (var i=0; i<15; i++) { + $scope.more.push(i); + } + }; } diff --git a/js/ext/angular/test/service/delegates/ionicScrollDelegate.unit.js b/js/ext/angular/test/service/delegates/ionicScrollDelegate.unit.js index 063feab16b5..08256d75532 100644 --- a/js/ext/angular/test/service/delegates/ionicScrollDelegate.unit.js +++ b/js/ext/angular/test/service/delegates/ionicScrollDelegate.unit.js @@ -34,7 +34,7 @@ describe('Ionic ScrollDelegate Service', function() { it('scroll event', function() { var scope = rootScope.$new(); var el = compile('')(scope); - scope = el.isolateScope(); + scope = el.scope(); scope.$apply(); var top, left; scope.onScroll = jasmine.createSpy('scroll').andCallFake(function(data) { diff --git a/js/ext/angular/test/service/ionicBind.unit.js b/js/ext/angular/test/service/ionicBind.unit.js new file mode 100644 index 00000000000..32523305f2c --- /dev/null +++ b/js/ext/angular/test/service/ionicBind.unit.js @@ -0,0 +1,137 @@ +describe('$ionicBind', function() { + beforeEach(module('ionic.service.bind')); + + var $bind, scope, attr, $observeFn; + beforeEach(inject(function($ionicBind, $rootScope, $interpolate) { + $bind = $ionicBind; + scope = $rootScope.$new(); + attr = { + $observe: jasmine.createSpy('observe').andCallFake(function(name, fn) { + $observeFn = fn; + }) + }; + })); + + describe('= bind', function() { + + it('should bind expression to scope', function() { + scope.$parent.coffee = 2; + attr.eq = 'coffee'; + $bind(scope, attr, { + eq: '=' + }); + scope.$apply(); + expect(scope.eq).toEqual(2); + scope.$parent.$apply('coffee = 100'); + expect(scope.eq).toEqual(100); + }); + + it('should allow binding a different name to scope', function() { + scope.$parent.coffee = 2; + attr.eq = 'coffee'; + $bind(scope, attr, { + coolVar: '=eq' + }); + scope.$apply(); + expect(scope.coolVar).toEqual(scope.$parent.coffee); + scope.$parent.$apply('coffee = 100'); + expect(scope.coolVar).toEqual(100); + }); + + it('should work as expected if bind name is same', function() { + scope.$parent.foo = 2; + attr.espresso = 'foo'; + $bind(scope, attr, { + foo: '=espresso' + }); + scope.$apply(); + expect(scope.$parent.foo).toBe(2); + scope.$parent.$apply('foo = 4'); + expect(scope.$parent.foo).toBe(4); + }); + + it('should unwatch on $destroy', function() { + var watchUnregister = jasmine.createSpy('watchUnreg'); + spyOn(scope.$parent, '$watch').andCallFake(function() { + return watchUnregister; + }); + attr.binding = 'something'; + $bind(scope, attr, { + binding: '=' + }); + scope.$destroy(); + expect(watchUnregister).toHaveBeenCalled(); + }); + }); + + describe ('@ bind', function() { + + it('should bind expression to scope', function() { + scope.$parent.coffee = 'cool'; + attr.special = '{{coffee}}'; + $bind(scope, attr, { + special: '@' + }); + expect(attr.$observe).toHaveBeenCalledWith('special', $observeFn); + expect(scope.special).toBe('cool'); + scope.$parent.coffee = 'espresso'; + $observeFn(scope.$parent.coffee); + expect(scope.special).toBe('espresso'); + }); + + it('should allow binding a different name to scope', function() { + scope.$parent.coffee = 'cool'; + attr.special = '{{coffee}}'; + $bind(scope, attr, { + scopeName: '@special' + }); + expect(scope.scopeName).toBe('cool'); + scope.$parent.coffee = 'espresso'; + $observeFn(scope.$parent.coffee); + expect(scope.scopeName).toBe('espresso'); + }); + + it('should allow binding a different name to scope', function() { + scope.$parent.coffee = 'cool'; + attr.special = '{{coffee}}'; + $bind(scope, attr, { + coffee: '@special' + }); + expect(scope.coffee).toBe('cool'); + scope.$parent.coffee = 'espresso'; + $observeFn(scope.$parent.coffee); + expect(scope.coffee).toBe('espresso'); + }); + + }); + + describe('& bind', function() { + + it('should bind expression to scope', function() { + attr.math = '1+1'; + $bind(scope, attr, { + two: '&math' + }); + expect(scope.two()).toBe(2); + }); + + it('should bind expression with different name to scope', function() { + attr.doIt = 'fun()'; + scope.$parent.fun = function() { + return 'this is cool!'; + }; + $bind(scope, attr, { + party: '&doIt' + }); + expect(scope.party()).toBe('this is cool!'); + }); + + it('should error for similar scopeName and expression', function() { + scope.$parent.foo = function(){}; + attr.bar = 'foo()'; + expect(function() { + $bind(scope, attr, { foo: '&bar' }); + }).toThrow(); + }); + }); +});