From f45815cb114ebb044dad91caa97afde66e46cda7 Mon Sep 17 00:00:00 2001 From: Tasos Bekos Date: Sun, 28 Jul 2013 02:34:03 +0300 Subject: [PATCH] fix(pagination): use interpolation for text attributes Closes #696 BREAKING CHANGE: The 'first-text', 'previous-text', 'next-text' and 'last-text' attributes are now interpolated. To migrate your code, remove quotes for constant attributes and/or interpolate scope variables. Before: and/or $scope.var1 = '<<'; After: and/or $scope.var1 = '<<'; --- src/pagination/pagination.js | 28 +++++++++++++++----------- src/pagination/test/pager.spec.js | 14 +++++++++++-- src/pagination/test/pagination.spec.js | 26 ++++++++++-------------- 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/pagination/pagination.js b/src/pagination/pagination.js index 1c4165d660..f4becfcd2c 100644 --- a/src/pagination/pagination.js +++ b/src/pagination/pagination.js @@ -1,6 +1,6 @@ angular.module('ui.bootstrap.pagination', []) -.controller('PaginationController', ['$scope', function ($scope) { +.controller('PaginationController', ['$scope', '$interpolate', function ($scope, $interpolate) { this.currentPage = 1; @@ -31,6 +31,10 @@ angular.module('ui.bootstrap.pagination', []) $scope.onSelectPage({ page: page }); } }; + + this.getAttributeValue = function(attribute, defaultValue, interpolate) { + return angular.isDefined(attribute) ? (interpolate ? $interpolate(attribute)($scope.$parent) : $scope.$parent.$eval(attribute)) : defaultValue; + }; }]) .constant('paginationConfig', { @@ -43,7 +47,7 @@ angular.module('ui.bootstrap.pagination', []) rotate: true }) -.directive('pagination', ['paginationConfig', function(paginationConfig) { +.directive('pagination', ['paginationConfig', function(config) { return { restrict: 'EA', scope: { @@ -58,13 +62,13 @@ angular.module('ui.bootstrap.pagination', []) link: function(scope, element, attrs, paginationCtrl) { // Setup configuration parameters - var boundaryLinks = angular.isDefined(attrs.boundaryLinks) ? scope.$eval(attrs.boundaryLinks) : paginationConfig.boundaryLinks; - var directionLinks = angular.isDefined(attrs.directionLinks) ? scope.$eval(attrs.directionLinks) : paginationConfig.directionLinks; - var firstText = angular.isDefined(attrs.firstText) ? scope.$parent.$eval(attrs.firstText) : paginationConfig.firstText; - var previousText = angular.isDefined(attrs.previousText) ? scope.$parent.$eval(attrs.previousText) : paginationConfig.previousText; - var nextText = angular.isDefined(attrs.nextText) ? scope.$parent.$eval(attrs.nextText) : paginationConfig.nextText; - var lastText = angular.isDefined(attrs.lastText) ? scope.$parent.$eval(attrs.lastText) : paginationConfig.lastText; - var rotate = angular.isDefined(attrs.rotate) ? scope.$eval(attrs.rotate) : paginationConfig.rotate; + var boundaryLinks = paginationCtrl.getAttributeValue(attrs.boundaryLinks, config.boundaryLinks ), + directionLinks = paginationCtrl.getAttributeValue(attrs.directionLinks, config.directionLinks ), + firstText = paginationCtrl.getAttributeValue(attrs.firstText, config.firstText, true), + previousText = paginationCtrl.getAttributeValue(attrs.previousText, config.previousText, true), + nextText = paginationCtrl.getAttributeValue(attrs.nextText, config.nextText, true), + lastText = paginationCtrl.getAttributeValue(attrs.lastText, config.lastText, true), + rotate = paginationCtrl.getAttributeValue(attrs.rotate, config.rotate); // Create page object used in template function makePage(number, text, isActive, isDisabled) { @@ -165,9 +169,9 @@ angular.module('ui.bootstrap.pagination', []) link: function(scope, element, attrs, paginationCtrl) { // Setup configuration parameters - var previousText = angular.isDefined(attrs.previousText) ? scope.$parent.$eval(attrs.previousText) : config.previousText; - var nextText = angular.isDefined(attrs.nextText) ? scope.$parent.$eval(attrs.nextText) : config.nextText; - var align = angular.isDefined(attrs.align) ? scope.$parent.$eval(attrs.align) : config.align; + var previousText = paginationCtrl.getAttributeValue(attrs.previousText, config.previousText, true), + nextText = paginationCtrl.getAttributeValue(attrs.nextText, config.nextText, true), + align = paginationCtrl.getAttributeValue(attrs.align, config.align); // Create page object used in template function makePage(number, text, isDisabled, isPrevious, isNext) { diff --git a/src/pagination/test/pager.spec.js b/src/pagination/test/pager.spec.js index de863a507d..91874d6e8c 100644 --- a/src/pagination/test/pager.spec.js +++ b/src/pagination/test/pager.spec.js @@ -153,7 +153,7 @@ describe('setting pagerConfig', function() { }); -describe('pagination bypass configuration from attributes', function () { +describe('pager bypass configuration from attributes', function () { var $rootScope, element; beforeEach(module('ui.bootstrap.pagination')); beforeEach(module('template/pagination/pager.html')); @@ -162,7 +162,7 @@ describe('pagination bypass configuration from attributes', function () { $rootScope = _$rootScope_; $rootScope.numPages = 5; $rootScope.currentPage = 3; - element = $compile('')($rootScope); + element = $compile('')($rootScope); $rootScope.$digest(); })); @@ -180,4 +180,14 @@ describe('pagination bypass configuration from attributes', function () { expect(element.find('li').eq(-1).hasClass('next')).toBe(false); }); + it('changes "previous" & "next" text from interpolated attributes', function() { + $rootScope.previousText = '<<'; + $rootScope.nextText = '>>'; + element = $compile('')($rootScope); + $rootScope.$digest(); + + expect(element.find('li').eq(0).text()).toBe('<<'); + expect(element.find('li').eq(-1).text()).toBe('>>'); + }); + }); \ No newline at end of file diff --git a/src/pagination/test/pagination.spec.js b/src/pagination/test/pagination.spec.js index 8ef067f648..764cf2f5ac 100644 --- a/src/pagination/test/pagination.spec.js +++ b/src/pagination/test/pagination.spec.js @@ -222,7 +222,8 @@ describe('pagination directive with max size option & no rotate', function () { $rootScope.numPages = 12; $rootScope.currentPage = 7; $rootScope.maxSize = 5; - element = $compile('')($rootScope); + $rootScope.rotate = false; + element = $compile('')($rootScope); $rootScope.$digest(); })); @@ -296,7 +297,6 @@ describe('pagination directive with added first & last links', function () { $rootScope.$digest(); })); - it('contains one ul and num-pages + 4 li elements', function() { expect(element.find('ul').length).toBe(1); expect(element.find('li').length).toBe(9); @@ -331,7 +331,6 @@ describe('pagination directive with added first & last links', function () { expect(element.find('li').eq(-1).hasClass('disabled')).toBe(true); }); - it('changes currentPage if the "first" link is clicked', function() { var first = element.find('li').eq(0).find('a').eq(0); first.click(); @@ -365,7 +364,7 @@ describe('pagination directive with added first & last links', function () { }); it('changes "first" & "last" text from attributes', function() { - element = $compile('')($rootScope); + element = $compile('')($rootScope); $rootScope.$digest(); expect(element.find('li').eq(0).text()).toBe('<<<'); @@ -373,27 +372,27 @@ describe('pagination directive with added first & last links', function () { }); it('changes "previous" & "next" text from attributes', function() { - element = $compile('')($rootScope); + element = $compile('')($rootScope); $rootScope.$digest(); expect(element.find('li').eq(1).text()).toBe('<<'); expect(element.find('li').eq(-2).text()).toBe('>>'); }); - it('changes "first" & "last" text from attribute variables', function() { + it('changes "first" & "last" text from interpolated attributes', function() { $rootScope.myfirstText = '<<<'; $rootScope.mylastText = '>>>'; - element = $compile('')($rootScope); + element = $compile('')($rootScope); $rootScope.$digest(); expect(element.find('li').eq(0).text()).toBe('<<<'); expect(element.find('li').eq(-1).text()).toBe('>>>'); }); - it('changes "previous" & "next" text from attribute variables', function() { + it('changes "previous" & "next" text from interpolated attributes', function() { $rootScope.previousText = '<<'; $rootScope.nextText = '>>'; - element = $compile('')($rootScope); + element = $compile('')($rootScope); $rootScope.$digest(); expect(element.find('li').eq(1).text()).toBe('<<'); @@ -461,7 +460,6 @@ describe('pagination directive with just number links', function () { expect($rootScope.currentPage).toBe(2); }); - it('executes the onSelectPage expression when the current page changes', function() { $rootScope.selectPageHandler = jasmine.createSpy('selectPageHandler'); element = $compile('')($rootScope); @@ -541,11 +539,11 @@ describe('pagination directive with first, last & number links', function () { $rootScope = _$rootScope_; $rootScope.numPages = 5; $rootScope.currentPage = 3; - element = $compile('')($rootScope); + $rootScope.directions = false; + element = $compile('')($rootScope); $rootScope.$digest(); })); - it('contains one ul and num-pages + 2 li elements', function() { expect(element.find('ul').length).toBe(1); expect(element.find('li').length).toBe(7); @@ -555,7 +553,6 @@ describe('pagination directive with first, last & number links', function () { expect(element.find('li').eq(-1).text()).toBe('Last'); }); - it('disables the "first" & activates "1" link if current-page is 1', function() { $rootScope.currentPage = 1; $rootScope.$digest(); @@ -572,7 +569,6 @@ describe('pagination directive with first, last & number links', function () { expect(element.find('li').eq(-1).hasClass('disabled')).toBe(true); }); - it('changes currentPage if the "first" link is clicked', function() { var first = element.find('li').eq(0).find('a').eq(0); first.click(); @@ -598,7 +594,7 @@ describe('pagination bypass configuration from attributes', function () { $rootScope = _$rootScope_; $rootScope.numPages = 5; $rootScope.currentPage = 3; - element = $compile('')($rootScope); + element = $compile('')($rootScope); $rootScope.$digest(); }));