From 30f8f0515158cbf6772cba766d4d6b1e2b21e651 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 23 Jan 2015 01:12:28 +0200 Subject: [PATCH] fix(ngPluralize): fix wrong text content when count is null/undefined When `lastCount` was evaluated to an non-numeric value (e.g. "other") and `count` was evaluated to `NaN` (e.g. `null`/`undefined`), the text content would be (wrongly) based on the previous template. This commits makes sure the text content is updated correctly. In order to customize the message shown upon `null`/`undefined` one can specify a `'NaN'` property on the `when` exression object. Closes #10836 --- src/ng/directive/ngPluralize.js | 6 +- test/ng/directive/ngPluralizeSpec.js | 132 +++++++++++++++++++-------- 2 files changed, 96 insertions(+), 42 deletions(-) diff --git a/src/ng/directive/ngPluralize.js b/src/ng/directive/ngPluralize.js index b7d3e089731b..8a1b11a959e3 100644 --- a/src/ng/directive/ngPluralize.js +++ b/src/ng/directive/ngPluralize.js @@ -217,11 +217,13 @@ var ngPluralizeDirective = ['$locale', '$interpolate', '$log', function($locale, // If both `count` and `lastCount` are NaN, we don't need to re-register a watch. // In JS `NaN !== NaN`, so we have to exlicitly check. - if ((count !== lastCount) && !(countIsNaN && isNaN(lastCount))) { + if ((count !== lastCount) && !(countIsNaN && isNumber(lastCount) && isNaN(lastCount))) { watchRemover(); var whenExpFn = whensExpFns[count]; if (isUndefined(whenExpFn)) { - $log.debug("ngPluralize: no rule defined for '" + count + "' in " + whenExp); + if (newVal != null) { + $log.debug("ngPluralize: no rule defined for '" + count + "' in " + whenExp); + } watchRemover = noop; updateElementText(); } else { diff --git a/test/ng/directive/ngPluralizeSpec.js b/test/ng/directive/ngPluralizeSpec.js index 0ff0c668fb9c..6f913ccc615c 100644 --- a/test/ng/directive/ngPluralizeSpec.js +++ b/test/ng/directive/ngPluralizeSpec.js @@ -83,52 +83,64 @@ describe('ngPluralize', function() { })); - it('should show single/plural strings with mal-formed inputs', inject(function($rootScope) { - $rootScope.email = ''; - $rootScope.$digest(); - expect(element.text()).toBe(''); - expect(elementAlt.text()).toBe(''); - - $rootScope.email = null; - $rootScope.$digest(); - expect(element.text()).toBe(''); - expect(elementAlt.text()).toBe(''); + it('should show single/plural strings with mal-formed inputs', inject( + function($log, $rootScope) { + $rootScope.email = ''; + $rootScope.$digest(); + expect(element.text()).toBe(''); + expect(elementAlt.text()).toBe(''); + expect($log.debug.logs.shift()).toEqual([ + "ngPluralize: no rule defined for 'NaN' in {" + + "'-1': 'You have negative email. Whohoo!'," + + "'0': 'You have no new email'," + + "'one': 'You have one new email'," + + "'other': 'You have {} new emails'}" + ]); + expect($log.debug.logs.shift()).toEqual([ + "ngPluralize: no rule defined for 'NaN' in undefined" + ]); + + $rootScope.email = null; + $rootScope.$digest(); + expect(element.text()).toBe(''); + expect(elementAlt.text()).toBe(''); - $rootScope.email = undefined; - $rootScope.$digest(); - expect(element.text()).toBe(''); - expect(elementAlt.text()).toBe(''); + $rootScope.email = undefined; + $rootScope.$digest(); + expect(element.text()).toBe(''); + expect(elementAlt.text()).toBe(''); - $rootScope.email = 'a3'; - $rootScope.$digest(); - expect(element.text()).toBe(''); - expect(elementAlt.text()).toBe(''); + $rootScope.email = 'a3'; + $rootScope.$digest(); + expect(element.text()).toBe(''); + expect(elementAlt.text()).toBe(''); - $rootScope.email = '011'; - $rootScope.$digest(); - expect(element.text()).toBe('You have 11 new emails'); - expect(elementAlt.text()).toBe('You have 11 new emails'); + $rootScope.email = '011'; + $rootScope.$digest(); + expect(element.text()).toBe('You have 11 new emails'); + expect(elementAlt.text()).toBe('You have 11 new emails'); - $rootScope.email = '-011'; - $rootScope.$digest(); - expect(element.text()).toBe('You have -11 new emails'); - expect(elementAlt.text()).toBe('You have -11 new emails'); + $rootScope.email = '-011'; + $rootScope.$digest(); + expect(element.text()).toBe('You have -11 new emails'); + expect(elementAlt.text()).toBe('You have -11 new emails'); - $rootScope.email = '1fff'; - $rootScope.$digest(); - expect(element.text()).toBe('You have one new email'); - expect(elementAlt.text()).toBe('You have one new email'); + $rootScope.email = '1fff'; + $rootScope.$digest(); + expect(element.text()).toBe('You have one new email'); + expect(elementAlt.text()).toBe('You have one new email'); - $rootScope.email = '0aa22'; - $rootScope.$digest(); - expect(element.text()).toBe('You have no new email'); - expect(elementAlt.text()).toBe('You have no new email'); + $rootScope.email = '0aa22'; + $rootScope.$digest(); + expect(element.text()).toBe('You have no new email'); + expect(elementAlt.text()).toBe('You have no new email'); - $rootScope.email = '000001'; - $rootScope.$digest(); - expect(element.text()).toBe('You have one new email'); - expect(elementAlt.text()).toBe('You have one new email'); - })); + $rootScope.email = '000001'; + $rootScope.$digest(); + expect(element.text()).toBe('You have one new email'); + expect(elementAlt.text()).toBe('You have one new email'); + } + )); }); @@ -144,6 +156,33 @@ describe('ngPluralize', function() { $rootScope.$digest(); expect(element.text()).toBe(''); })); + + it('should be able to specify a message for null/undefined values', inject( + function($compile, $rootScope) { + element = $compile( + '" + + '')($rootScope); + + $rootScope.email = '0'; + $rootScope.$digest(); + expect(element.text()).toBe(''); + + $rootScope.email = undefined; + $rootScope.$digest(); + expect(element.text()).toBe('Unspecified email count'); + + $rootScope.email = '1'; + $rootScope.$digest(); + expect(element.text()).toBe('Some text'); + + $rootScope.email = null; + $rootScope.$digest(); + expect(element.text()).toBe('Unspecified email count'); + })); }); describe('undefined rule cases', function() { @@ -195,6 +234,10 @@ describe('ngPluralize', function() { $rootScope.email = '1'; $rootScope.$digest(); expect(element.text()).toBe('Some text'); + + $rootScope.email = null; + $rootScope.$digest(); + expect(element.text()).toBe(''); })); }); @@ -243,6 +286,11 @@ describe('ngPluralize', function() { $rootScope.$digest(); expect(element.text()).toBe('Igor, Misko and 2 other people are viewing.'); expect(elementAlt.text()).toBe('Igor, Misko and 2 other people are viewing.'); + + $rootScope.viewCount = null; + $rootScope.$digest(); + expect(element.text()).toBe(''); + expect(elementAlt.text()).toBe(''); })); }); @@ -296,7 +344,6 @@ describe('ngPluralize', function() { describe('bind-once', function() { - it('should support for `count` to be a one-time expression', inject(function($compile, $rootScope) { element = $compile( @@ -320,6 +367,11 @@ describe('ngPluralize', function() { expect(element.text()).toBe('You have 3 new emails'); expect(elementAlt.text()).toBe('You have 3 new emails'); + $rootScope.email = null; + $rootScope.$digest(); + expect(element.text()).toBe('You have 3 new emails'); + expect(elementAlt.text()).toBe('You have 3 new emails'); + $rootScope.email = 2; $rootScope.$digest(); expect(element.text()).toBe('You have 3 new emails');