Merge branch 'pedroms-threads-counter-wording' into 'master'

Change wording of merge request threads counter

See merge request gitlab-org/gitlab!30217

app/assets/javascripts/notes/components/discussion_counter.vue

@@ -29,9 +29,6 @@ export default {
     resolveAllDiscussionsIssuePath() {
       return this.getNoteableData.create_issue_to_resolve_discussions_path;
     },
-    resolvedDiscussionsCount() {
-      return this.resolvableDiscussionsCount - this.unresolvedDiscussionsCount;
-    },
     toggeableDiscussions() {
       return this.discussions.filter(discussion => !discussion.individual_note);
     },
@@ -60,15 +57,15 @@ export default {
     <div class="full-width-mobile d-flex d-sm-flex">
       <div class="line-resolve-all">
         <span
-          :class="{ 'is-active': allResolved }"
-          class="line-resolve-btn is-disabled"
-          type="button"
+          :class="{ 'line-resolve-btn is-active': allResolved, 'line-resolve-text': !allResolved }"
         >
-          <icon :name="allResolved ? 'check-circle-filled' : 'check-circle'" />
-        </span>
-        <span class="line-resolve-text">
-          {{ resolvedDiscussionsCount }}/{{ resolvableDiscussionsCount }}
-          {{ n__('thread resolved', 'threads resolved', resolvableDiscussionsCount) }}
+          <template v-if="allResolved">
+            <icon name="check-circle-filled" />
+            {{ __('All threads resolved') }}
+          </template>
+          <template v-else>
+            {{ n__('%d unresolved thread', '%d unresolved threads', unresolvedDiscussionsCount) }}
+          </template>
         </span>
       </div>
       <div

app/assets/stylesheets/pages/notes.scss

@@ -908,11 +908,10 @@ $note-form-margin-left: 72px;
   border-right: 0;
 
   .line-resolve-btn {
-    margin-right: 5px;
     color: $gray-700;
 
     svg {
-      vertical-align: middle;
+      vertical-align: text-top;
     }
   }
 

changelogs/unreleased/pedroms-threads-counter-wording.yml

+---
+title: Change wording of merge request threads counter
+merge_request: 30217
+author:
+type: changed

ee/spec/features/merge_request/batch_comments_spec.rb

@@ -154,7 +154,7 @@ describe 'Merge request > Batch comments', :js do
         write_reply_to_discussion(button_text: 'Add comment now', resolve: true)
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -172,7 +172,7 @@ describe 'Merge request > Batch comments', :js do
         wait_for_requests
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -195,8 +195,8 @@ describe 'Merge request > Batch comments', :js do
         write_reply_to_discussion(button_text: 'Add comment now', unresolve: true)
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
-          expect(page).to have_selector('.line-resolve-btn.is-disabled')
+          expect(page).to have_content('1 unresolved thread')
+          expect(page).not_to have_selector('.line-resolve-btn.is-active')
         end
       end
 
@@ -215,8 +215,8 @@ describe 'Merge request > Batch comments', :js do
         wait_for_requests
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
-          expect(page).to have_selector('.line-resolve-btn.is-disabled')
+          expect(page).to have_content('1 unresolved thread')
+          expect(page).not_to have_selector('.line-resolve-btn.is-active')
         end
       end
     end

locale/gitlab.pot

@@ -214,6 +214,11 @@ msgid_plural "%d tags"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "%d unresolved thread"
+msgid_plural "%d unresolved threads"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "%d vulnerability dismissed"
 msgid_plural "%d vulnerabilities dismissed"
 msgstr[0] ""
@@ -1827,6 +1832,9 @@ msgstr ""
 msgid "All security scans are enabled because %{linkStart}Auto DevOps%{linkEnd} is enabled on this project"
 msgstr ""
 
+msgid "All threads resolved"
+msgstr ""
+
 msgid "All users"
 msgstr ""
 
@@ -25705,11 +25713,6 @@ msgstr ""
 msgid "this document"
 msgstr ""
 
-msgid "thread resolved"
-msgid_plural "threads resolved"
-msgstr[0] ""
-msgstr[1] ""
-
 msgid "to help your contributors communicate effectively!"
 msgstr ""
 

qa/qa/specs/features/ee/browser_ui/3_create/merge_request/add_batch_comments_in_merge_request_spec.rb

@@ -33,7 +33,7 @@ module QA
 
           expect(show).to have_content("I'm starting a new discussion")
           expect(show).to have_content("Could you please check this?")
-          expect(show).to have_content("0/1 thread resolved")
+          expect(show).to have_content("1 unresolved thread")
         end
       end
 
@@ -72,7 +72,7 @@ module QA
           show.resolve_discussion_at_index(0)
 
           expect(show).to have_content("Can you check this line of code?")
-          expect(show).to have_content("1/1 thread resolved")
+          expect(show).to have_content("All threads resolved")
         end
       end
     end

spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb

@@ -43,7 +43,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
     context 'single thread' do
       it 'shows text with how many threads' do
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
+          expect(page).to have_content('1 unresolved thread')
         end
       end
 
@@ -60,7 +60,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -77,7 +77,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -89,7 +89,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
+          expect(page).to have_content('1 unresolved thread')
         end
       end
 
@@ -162,7 +162,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
             end
 
             page.within '.line-resolve-all-container' do
-              expect(page).to have_content('1/1 thread resolved')
+              expect(page).to have_content('All threads resolved')
             end
           end
 
@@ -174,7 +174,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
             end
 
             page.within '.line-resolve-all-container' do
-              expect(page).to have_content('0/1 thread resolved')
+              expect(page).to have_content('1 unresolved thread')
               expect(page).not_to have_selector('.line-resolve-btn.is-active')
             end
           end
@@ -189,7 +189,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
             end
 
             page.within '.line-resolve-all-container' do
-              expect(page).to have_content('0/1 thread resolved')
+              expect(page).to have_content('1 unresolved thread')
             end
           end
         end
@@ -203,7 +203,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -218,7 +218,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -275,7 +275,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         expect(page).to have_content('Last updated')
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
+          expect(page).to have_content('1 unresolved thread')
         end
       end
 
@@ -292,7 +292,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
         end
       end
     end
@@ -305,7 +305,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'shows text with how many threads' do
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/2 threads resolved')
+          expect(page).to have_content('2 unresolved threads')
         end
       end
 
@@ -313,7 +313,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         click_button('Resolve thread', match: :first)
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/2 threads resolved')
+          expect(page).to have_content('1 unresolved thread')
         end
       end
 
@@ -323,7 +323,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('2/2 threads resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -336,7 +336,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('2/2 threads resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -392,7 +392,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
     context 'changes tab' do
       it 'shows text with how many threads' do
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
+          expect(page).to have_content('1 unresolved thread')
         end
       end
 
@@ -408,7 +408,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -423,7 +423,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -435,7 +435,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
+          expect(page).to have_content('1 unresolved thread')
         end
       end
 
@@ -449,7 +449,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -466,7 +466,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
+          expect(page).to have_content('1 unresolved thread')
         end
       end
     end
@@ -489,7 +489,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
+          expect(page).to have_content('1 unresolved thread')
         end
       end
 
@@ -519,7 +519,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1/1 thread resolved')
+          expect(page).to have_content('All threads resolved')
           expect(page).to have_selector('.line-resolve-btn.is-active')
         end
       end
@@ -538,7 +538,7 @@ describe 'Merge request > User resolves diff notes and threads', :js do
         end
 
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('0/1 thread resolved')
+          expect(page).to have_content('1 unresolved thread')
         end
       end
     end
@@ -550,17 +550,17 @@ describe 'Merge request > User resolves diff notes and threads', :js do
       end
 
       it 'shows resolved icon' do
-        expect(page).to have_content '1/1 thread resolved'
+        expect(page).to have_content 'All threads resolved'
 
         click_button 'Toggle thread'
         expect(page).to have_selector('.line-resolve-btn.is-active')
       end
 
       it 'does not allow user to click resolve button' do
-        expect(page).to have_selector('.line-resolve-btn.is-disabled')
+        expect(page).to have_selector('.line-resolve-btn.is-active')
         click_button 'Toggle thread'
 
-        expect(page).to have_selector('.line-resolve-btn.is-disabled')
+        expect(page).to have_selector('.line-resolve-btn.is-active')
       end
     end
   end

spec/frontend/notes/components/discussion_counter_spec.js

@@ -75,15 +75,14 @@ describe('DiscussionCounter component', () => {
     });
 
     it.each`
-      title                | resolved | isActive | icon                     | groupLength
-      ${'not allResolved'} | ${false} | ${false} | ${'check-circle'}        | ${3}
-      ${'allResolved'}     | ${true}  | ${true}  | ${'check-circle-filled'} | ${1}
-    `('renders correctly if $title', ({ resolved, isActive, icon, groupLength }) => {
+      title                | resolved | isActive | groupLength
+      ${'not allResolved'} | ${false} | ${false} | ${3}
+      ${'allResolved'}     | ${true}  | ${true}  | ${1}
+    `('renders correctly if $title', ({ resolved, isActive, groupLength }) => {
       updateStore({ resolvable: true, resolved });
       wrapper = shallowMount(DiscussionCounter, { store, localVue });
 
       expect(wrapper.find(`.is-active`).exists()).toBe(isActive);
-      expect(wrapper.find({ name: icon }).exists()).toBe(true);
       expect(wrapper.findAll('[role="group"').length).toBe(groupLength);
     });
   });