From 67f4408d741b62e61f1fd767b4724727c489b42c Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Tue, 10 Oct 2017 07:47:42 +0000 Subject: [PATCH] Fix bad type checking to prevent 0 count badge to be shown --- app/assets/javascripts/header.js | 19 +++-- .../javascripts/lib/utils/text_utility.js | 14 +++- changelogs/unreleased/34841-todos.yml | 5 ++ spec/javascripts/header_spec.js | 77 +++++++++---------- .../lib/utils/text_utility_spec.js | 10 +-- 5 files changed, 71 insertions(+), 54 deletions(-) create mode 100644 changelogs/unreleased/34841-todos.yml diff --git a/app/assets/javascripts/header.js b/app/assets/javascripts/header.js index dc170c60456..ea2e2205077 100644 --- a/app/assets/javascripts/header.js +++ b/app/assets/javascripts/header.js @@ -1,7 +1,16 @@ -/* eslint-disable func-names, space-before-function-paren, prefer-arrow-callback, no-var */ +import { highCountTrim } from '~/lib/utils/text_utility'; -$(document).on('todo:toggle', function(e, count) { - var $todoPendingCount = $('.todos-count'); - $todoPendingCount.text(gl.text.highCountTrim(count)); - $todoPendingCount.toggleClass('hidden', count === 0); +/** + * Updates todo counter when todos are toggled. + * When count is 0, we hide the badge. + * + * @param {jQuery.Event} e + * @param {String} count + */ +$(document).on('todo:toggle', (e, count) => { + const parsedCount = parseInt(count, 10); + const $todoPendingCount = $('.todos-count'); + + $todoPendingCount.text(highCountTrim(parsedCount)); + $todoPendingCount.toggleClass('hidden', parsedCount === 0); }); diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index 021f936a4fa..f776829f69c 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -1,4 +1,4 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, no-var, no-param-reassign, no-cond-assign, quotes, one-var, one-var-declaration-per-line, operator-assignment, no-else-return, prefer-template, prefer-arrow-callback, no-empty, max-len, consistent-return, no-unused-vars, no-return-assign, max-len, vars-on-top */ +/* eslint-disable import/prefer-default-export, func-names, space-before-function-paren, wrap-iife, no-var, no-param-reassign, no-cond-assign, quotes, one-var, one-var-declaration-per-line, operator-assignment, no-else-return, prefer-template, prefer-arrow-callback, no-empty, max-len, consistent-return, no-unused-vars, no-return-assign, max-len, vars-on-top */ import 'vendor/latinise'; @@ -13,9 +13,17 @@ if ((base = w.gl).text == null) { gl.text.addDelimiter = function(text) { return text ? text.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ",") : text; }; -gl.text.highCountTrim = function(count) { + +/** + * Returns '99+' for numbers bigger than 99. + * + * @param {Number} count + * @return {Number|String} + */ +export function highCountTrim(count) { return count > 99 ? '99+' : count; -}; +} + gl.text.randomString = function() { return Math.random().toString(36).substring(7); }; diff --git a/changelogs/unreleased/34841-todos.yml b/changelogs/unreleased/34841-todos.yml new file mode 100644 index 00000000000..37180eefbfc --- /dev/null +++ b/changelogs/unreleased/34841-todos.yml @@ -0,0 +1,5 @@ +--- +title: Fix bad type checking to prevent 0 count badge to be shown +merge_request: +author: +type: fixed diff --git a/spec/javascripts/header_spec.js b/spec/javascripts/header_spec.js index 0e01934d3a3..4751eb868a4 100644 --- a/spec/javascripts/header_spec.js +++ b/spec/javascripts/header_spec.js @@ -1,53 +1,48 @@ -/* eslint-disable space-before-function-paren, no-var */ - import '~/header'; -import '~/lib/utils/text_utility'; -(function() { - describe('Header', function() { - var todosPendingCount = '.todos-count'; - var fixtureTemplate = 'issues/open-issue.html.raw'; +describe('Header', function () { + const todosPendingCount = '.todos-count'; + const fixtureTemplate = 'issues/open-issue.html.raw'; - function isTodosCountHidden() { - return $(todosPendingCount).hasClass('hidden'); - } + function isTodosCountHidden() { + return $(todosPendingCount).hasClass('hidden'); + } - function triggerToggle(newCount) { - $(document).trigger('todo:toggle', newCount); - } + function triggerToggle(newCount) { + $(document).trigger('todo:toggle', newCount); + } - preloadFixtures(fixtureTemplate); - beforeEach(function() { - loadFixtures(fixtureTemplate); + preloadFixtures(fixtureTemplate); + beforeEach(() => { + loadFixtures(fixtureTemplate); + }); + + it('should update todos-count after receiving the todo:toggle event', () => { + triggerToggle('5'); + expect($(todosPendingCount).text()).toEqual('5'); + }); + + it('should hide todos-count when it is 0', () => { + triggerToggle('0'); + expect(isTodosCountHidden()).toEqual(true); + }); + + it('should show todos-count when it is more than 0', () => { + triggerToggle('10'); + expect(isTodosCountHidden()).toEqual(false); + }); + + describe('when todos-count is 1000', () => { + beforeEach(() => { + triggerToggle('1000'); }); - it('should update todos-count after receiving the todo:toggle event', function() { - triggerToggle(5); - expect($(todosPendingCount).text()).toEqual('5'); - }); - - it('should hide todos-count when it is 0', function() { - triggerToggle(0); - expect(isTodosCountHidden()).toEqual(true); - }); - - it('should show todos-count when it is more than 0', function() { - triggerToggle(10); + it('should show todos-count', () => { expect(isTodosCountHidden()).toEqual(false); }); - describe('when todos-count is 1000', function() { - beforeEach(function() { - triggerToggle(1000); - }); - - it('should show todos-count', function() { - expect(isTodosCountHidden()).toEqual(false); - }); - - it('should show 99+ for todos-count', function() { - expect($(todosPendingCount).text()).toEqual('99+'); - }); + it('should show 99+ for todos-count', () => { + expect($(todosPendingCount).text()).toEqual('99+'); }); }); -}).call(window); +}); diff --git a/spec/javascripts/lib/utils/text_utility_spec.js b/spec/javascripts/lib/utils/text_utility_spec.js index f1a975ba962..829b3ef5735 100644 --- a/spec/javascripts/lib/utils/text_utility_spec.js +++ b/spec/javascripts/lib/utils/text_utility_spec.js @@ -1,4 +1,4 @@ -import '~/lib/utils/text_utility'; +import { highCountTrim } from '~/lib/utils/text_utility'; describe('text_utility', () => { describe('gl.text.getTextWidth', () => { @@ -35,14 +35,14 @@ describe('text_utility', () => { }); }); - describe('gl.text.highCountTrim', () => { + describe('highCountTrim', () => { it('returns 99+ for count >= 100', () => { - expect(gl.text.highCountTrim(105)).toBe('99+'); - expect(gl.text.highCountTrim(100)).toBe('99+'); + expect(highCountTrim(105)).toBe('99+'); + expect(highCountTrim(100)).toBe('99+'); }); it('returns exact number for count < 100', () => { - expect(gl.text.highCountTrim(45)).toBe(45); + expect(highCountTrim(45)).toBe(45); }); });