Merge branch '26703-todos-count' into 'master'

#26703 change todos counter position & format ( for large counts )

Closes #26703

See merge request !9171

11 files changed, 58 insertions, 5 deletions

diff --git a/app/assets/javascripts/header.js b/app/assets/javascripts/header.js
index fa85f9a6c86..a853c3aeb1f 100644
--- a/app/assets/javascripts/header.js
+++ b/app/assets/javascripts/header.js
@@ -2,7 +2,7 @@
 (function() {
   $(document).on('todo:toggle', function(e, count) {
     var $todoPendingCount = $('.todos-pending-count');
-    $todoPendingCount.text(gl.text.addDelimiter(count));
+    $todoPendingCount.text(gl.text.highCountTrim(count));
     $todoPendingCount.toggleClass('hidden', count === 0);
   });
 })();
diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js
index f755d212b3c..579d322e3fb 100644
--- a/app/assets/javascripts/lib/utils/text_utility.js
+++ b/app/assets/javascripts/lib/utils/text_utility.js
@@ -14,6 +14,9 @@ require('vendor/latinise');
     gl.text.addDelimiter = function(text) {
       return text ? text.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ",") : text;
     };
+    gl.text.highCountTrim = function(count) {
+      return count > 99 ? '99+' : count;
+    };
     gl.text.randomString = function() {
       return Math.random().toString(36).substring(7);
     };
diff --git a/app/assets/stylesheets/pages/todos.scss b/app/assets/stylesheets/pages/todos.scss
index bac831ec315..af9ddb9ff80 100644
--- a/app/assets/stylesheets/pages/todos.scss
+++ b/app/assets/stylesheets/pages/todos.scss
@@ -6,6 +6,8 @@
 .navbar-nav {
   li {
     .badge.todos-pending-count {
+      position: inherit;
+      top: -6px;
       margin-top: -5px;
       font-weight: normal;
       background: $todo-alert-blue;
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb
index 4e61b0886d8..ee2358b986d 100644
--- a/app/controllers/dashboard/todos_controller.rb
+++ b/app/controllers/dashboard/todos_controller.rb
@@ -35,6 +35,11 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     render json: todos_counts
   end
 
+  # Used in TodosHelper also
+  def self.todos_count_format(count)
+    count >= 100 ? '99+' : count
+  end
+
   private
 
   def find_todos
diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index 845f1a0e840..c52afd6db1c 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -3,6 +3,10 @@ module TodosHelper
     @todos_pending_count ||= current_user.todos_pending_count
   end
 
+  def todos_count_format(count)
+    count > 99 ? '99+' : count
+  end
+
   def todos_done_count
     @todos_done_count ||= current_user.todos_done_count
   end
diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml
index 60b9b8bdbc4..f8986893776 100644
--- a/app/views/layouts/header/_default.html.haml
+++ b/app/views/layouts/header/_default.html.haml
@@ -35,7 +35,7 @@
               = link_to dashboard_todos_path, title: 'Todos', aria: { label: "Todos" }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
                 = icon('bell fw')
                 %span.badge.todos-pending-count{ class: ("hidden" if todos_pending_count == 0) }
-                  = todos_pending_count
+                  = todos_count_format(todos_pending_count)
             - if Gitlab::Sherlock.enabled?
               %li
                 = link_to sherlock_transactions_path, title: 'Sherlock Transactions',
diff --git a/changelogs/unreleased/26703-todos-count.yml b/changelogs/unreleased/26703-todos-count.yml
new file mode 100644
index 00000000000..24fd0c406e2
--- /dev/null
+++ b/changelogs/unreleased/26703-todos-count.yml
@@ -0,0 +1,4 @@
+---
+title: show 99+ for large count in todos notification bell
+merge_request: 9171
+author: mhasbini
diff --git a/doc/workflow/todos.md b/doc/workflow/todos.md
index 6eb457fde3f..4b0fba842e9 100644
--- a/doc/workflow/todos.md
+++ b/doc/workflow/todos.md
@@ -16,7 +16,8 @@ in a simple dashboard.
 
 You can quickly access the Todos dashboard using the bell icon next to the
 search bar in the upper right corner. The number in blue is the number of Todos
-you still have open.
+you still have open if the count is < 100, else it's 99+. The exact number
+will still be shown in the body of the _To do_ tab.
 
 ![Todos icon](img/todos_icon.png)
 
diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb
index fb19dac1d6a..3495091a0d5 100644
--- a/spec/features/todos/todos_spec.rb
+++ b/spec/features/todos/todos_spec.rb
@@ -171,6 +171,29 @@ describe 'Dashboard Todos', feature: true do
       end
     end
 
+    context 'User have large number of todos' do
+      before do
+        create_list(:todo, 101, :mentioned, user: user, project: project, target: issue, author: author)
+
+        login_as(user)
+        visit dashboard_todos_path
+      end
+
+      it 'shows 99+ for count >= 100 in notification' do
+        expect(page).to have_selector('.todos-pending-count', text: '99+')
+      end
+
+      it 'shows exact number in To do tab' do
+        expect(page).to have_selector('.todos-pending .badge', text: '101')
+      end
+
+      it 'shows exact number for count < 100' do
+        3.times { first('.js-done-todo').click }
+
+        expect(page).to have_selector('.todos-pending-count', text: '98')
+      end
+    end
+
     context 'User has a Build Failed todo' do
       let!(:todo) { create(:todo, :build_failed, user: user, project: project, author: author) }
 
diff --git a/spec/javascripts/header_spec.js b/spec/javascripts/header_spec.js
index 576b6cfefdc..46a27b8c98f 100644
--- a/spec/javascripts/header_spec.js
+++ b/spec/javascripts/header_spec.js
@@ -45,8 +45,8 @@ require('~/lib/utils/text_utility');
         expect(isTodosCountHidden()).toEqual(false);
       });
 
-      it('should add delimiter to todos-pending-count', function() {
-        expect($(todosPendingCount).text()).toEqual('1,000');
+      it('should show 99+ for todos-pending-count', function() {
+        expect($(todosPendingCount).text()).toEqual('99+');
       });
     });
   });
diff --git a/spec/javascripts/lib/utils/text_utility_spec.js.es6 b/spec/javascripts/lib/utils/text_utility_spec.js.es6
index 86ade66ec29..06b69b8ac17 100644
--- a/spec/javascripts/lib/utils/text_utility_spec.js.es6
+++ b/spec/javascripts/lib/utils/text_utility_spec.js.es6
@@ -35,5 +35,16 @@ require('~/lib/utils/text_utility');
         expect(gl.text.pluralize('test', 1)).toBe('test');
       });
     });
+
+    describe('gl.text.highCountTrim', () => {
+      it('returns 99+ for count >= 100', () => {
+        expect(gl.text.highCountTrim(105)).toBe('99+');
+        expect(gl.text.highCountTrim(100)).toBe('99+');
+      });
+
+      it('returns exact number for count < 100', () => {
+        expect(gl.text.highCountTrim(45)).toBe(45);
+      });
+    });
   });
 })();