Merge branch 'remove-bootstrap-dropdowns-from-note-components' into 'master'

Migrate Bootstrap dropdown to GitLab UI GlDropdown in comment_form.vue

See merge request gitlab-org/gitlab!50933

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

 <script>
-import { GlButton, GlIcon, GlFormCheckbox, GlTooltipDirective } from '@gitlab/ui';
+import {
+  GlButton,
+  GlIcon,
+  GlFormCheckbox,
+  GlTooltipDirective,
+  GlDropdown,
+  GlDropdownItem,
+  GlDropdownDivider,
+} from '@gitlab/ui';
 import Autosize from 'autosize';
 import $ from 'jquery';
 import { mapActions, mapGetters, mapState } from 'vuex';
@@ -25,6 +33,15 @@ import noteSignedOutWidget from './note_signed_out_widget.vue';
 
 export default {
   name: 'CommentForm',
+  i18n: {
+    submitButton: {
+      startThread: __('Start thread'),
+      comment: __('Comment'),
+      commentHelp: __('Add a general comment to this %{noteableDisplayName}.'),
+    },
+  },
+  noteTypeComment: constants.COMMENT,
+  noteTypeDiscussion: constants.DISCUSSION,
   components: {
     noteSignedOutWidget,
     discussionLockedWidget,
@@ -34,6 +51,9 @@ export default {
     GlIcon,
     CommentFieldLayout,
     GlFormCheckbox,
+    GlDropdown,
+    GlDropdownItem,
+    GlDropdownDivider,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
@@ -63,6 +83,12 @@ export default {
       'openState',
     ]),
     ...mapState(['isToggleStateButtonLoading']),
+    isNoteTypeComment() {
+      return this.noteType === constants.COMMENT;
+    },
+    isNoteTypeDiscussion() {
+      return this.noteType === constants.DISCUSSION;
+    },
     noteableDisplayName() {
       return splitCamelCase(this.noteableType).toLowerCase();
     },
@@ -77,6 +103,11 @@ export default {
         ? __('Discuss a specific suggestion or question that needs to be resolved.')
         : __('Discuss a specific suggestion or question.');
     },
+    commentDescription() {
+      return sprintf(this.$options.i18n.submitButton.commentHelp, {
+        noteableDisplayName: this.noteableDisplayName,
+      });
+    },
     isOpen() {
       return this.openState === constants.OPENED || this.openState === constants.REOPENED;
     },
@@ -260,6 +291,12 @@ export default {
     setNoteType(type) {
       this.noteType = type;
     },
+    setNoteTypeToComment() {
+      this.setNoteType(constants.COMMENT);
+    },
+    setNoteTypeToDiscussion() {
+      this.setNoteType(constants.DISCUSSION);
+    },
     editCurrentUserLastNote() {
       if (this.note === '') {
         const lastNote = this.getCurrentUserLastNote;
@@ -354,73 +391,40 @@ export default {
                   class="gl-text-gray-500"
                 />
               </gl-form-checkbox>
-              <div
-                class="btn-group gl-mr-3 comment-type-dropdown js-comment-type-dropdown droplab-dropdown"
+              <gl-dropdown
+                split
+                :text="commentButtonTitle"
+                class="gl-mr-3 js-comment-button js-comment-submit-button comment-type-dropdown"
+                category="primary"
+                variant="success"
+                :disabled="disableSubmitButton"
+                data-testid="comment-button"
+                data-qa-selector="comment_button"
+                :data-track-label="trackingLabel"
+                data-track-event="click_button"
+                @click="handleSave()"
               >
-                <gl-button
-                  :disabled="disableSubmitButton"
-                  class="js-comment-button js-comment-submit-button"
-                  data-qa-selector="comment_button"
-                  data-testid="comment-button"
-                  type="submit"
-                  category="primary"
-                  variant="success"
-                  :data-track-label="trackingLabel"
-                  data-track-event="click_button"
-                  @click.prevent="handleSave()"
-                  >{{ commentButtonTitle }}</gl-button
+                <gl-dropdown-item
+                  is-check-item
+                  :is-checked="isNoteTypeComment"
+                  :selected="isNoteTypeComment"
+                  @click="setNoteTypeToComment"
                 >
-                <gl-button
-                  :disabled="disableSubmitButton"
-                  name="button"
-                  category="primary"
-                  variant="success"
-                  class="note-type-toggle js-note-new-discussion dropdown-toggle"
-                  data-qa-selector="note_dropdown"
-                  data-display="static"
-                  data-toggle="dropdown"
-                  icon="chevron-down"
-                  :aria-label="__('Open comment type dropdown')"
-                />
-
-                <ul class="note-type-dropdown dropdown-open-top dropdown-menu">
-                  <li :class="{ 'droplab-item-selected': noteType === 'comment' }">
-                    <button
-                      type="button"
-                      class="btn btn-transparent"
-                      @click.prevent="setNoteType('comment')"
-                    >
-                      <gl-icon name="check" class="icon gl-flex-shrink-0" />
-                      <div class="description">
-                        <strong>{{ __('Comment') }}</strong>
-                        <p>
-                          {{
-                            sprintf(__('Add a general comment to this %{noteableDisplayName}.'), {
-                              noteableDisplayName,
-                            })
-                          }}
-                        </p>
-                      </div>
-                    </button>
-                  </li>
-                  <li class="divider droplab-item-ignore"></li>
-                  <li :class="{ 'droplab-item-selected': noteType === 'discussion' }">
-                    <button
-                      type="button"
-                      class="btn btn-transparent"
-                      data-qa-selector="discussion_menu_item"
-                      @click.prevent="setNoteType('discussion')"
-                    >
-                      <gl-icon name="check" class="icon gl-flex-shrink-0" />
-                      <div class="description">
-                        <strong>{{ __('Start thread') }}</strong>
-                        <p>{{ startDiscussionDescription }}</p>
-                      </div>
-                    </button>
-                  </li>
-                </ul>
-              </div>
-
+                  <strong>{{ $options.i18n.submitButton.comment }}</strong>
+                  <p class="gl-m-0">{{ commentDescription }}</p>
+                </gl-dropdown-item>
+                <gl-dropdown-divider />
+                <gl-dropdown-item
+                  is-check-item
+                  :is-checked="isNoteTypeDiscussion"
+                  :selected="isNoteTypeDiscussion"
+                  data-qa-selector="discussion_menu_item"
+                  @click="setNoteTypeToDiscussion"
+                >
+                  <strong>{{ $options.i18n.submitButton.startThread }}</strong>
+                  <p class="gl-m-0">{{ startDiscussionDescription }}</p>
+                </gl-dropdown-item>
+              </gl-dropdown>
               <gl-button
                 v-if="hasCloseAndCommentButton && canToggleIssueState"
                 :loading="isToggleStateButtonLoading"

app/assets/stylesheets/pages/note_form.scss

@@ -394,32 +394,6 @@ table {
 }
 
 .comment-type-dropdown {
-  .btn-success {
-    width: auto;
-  }
-
-  .dropdown-toggle {
-    float: right;
-
-    i {
-      color: $white;
-      padding-right: 2px;
-      margin-top: 2px;
-    }
-
-    &[disabled] {
-      i {
-        color: $gl-text-color-disabled;
-      }
-    }
-  }
-
-  .dropdown-menu {
-    top: initial;
-    bottom: 100%;
-    width: 298px;
-  }
-
   @include media-breakpoint-down(xs) {
     display: flex;
     width: 100%;

changelogs/unreleased/remove-bootstrap-dropdowns-from-note-components.yml

+---
+title: Migrated Bootstrap dropdown to GitLab UI GlDropdown used for comment submit
+  button
+merge_request: 50933
+author:
+type: other

ee/spec/features/discussion_comments/epic_spec.rb

@@ -14,5 +14,5 @@ RSpec.describe 'Thread Comments Epic', :js do
     visit group_epic_path(epic.group, epic)
   end
 
-  it_behaves_like 'thread comments', 'epic'
+  it_behaves_like 'thread comments for issue, epic and merge request', 'epic'
 end

qa/qa/page/component/note.rb

@@ -17,7 +17,6 @@ module QA
             element :comment_button
             element :comment_field
             element :discussion_menu_item
-            element :note_dropdown
           end
 
           base.view 'app/assets/javascripts/notes/components/discussion_actions.vue' do
@@ -146,7 +145,7 @@ module QA
 
         def start_discussion(text)
           fill_element :comment_field, text
-          click_element :note_dropdown
+          within_element(:comment_button) { click_button(class: 'dropdown-toggle-split') }
           click_element :discussion_menu_item
           click_element :comment_button
 

spec/features/discussion_comments/commit_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe 'Thread Comments Commit', :js do
     visit project_commit_path(project, sample_commit.id)
   end
 
-  it_behaves_like 'thread comments', 'commit'
+  it_behaves_like 'thread comments for commit and snippet', 'commit'
 
   it 'has class .js-note-emoji' do
     expect(page).to have_css('.js-note-emoji')

spec/features/discussion_comments/issue_spec.rb

@@ -16,5 +16,5 @@ RSpec.describe 'Thread Comments Issue', :js do
     visit project_issue_path(project, issue)
   end
 
-  it_behaves_like 'thread comments', 'issue'
+  it_behaves_like 'thread comments for issue, epic and merge request', 'issue'
 end

spec/features/discussion_comments/merge_request_spec.rb

@@ -20,5 +20,5 @@ RSpec.describe 'Thread Comments Merge Request', :js do
     wait_for_requests
   end
 
-  it_behaves_like 'thread comments', 'merge request'
+  it_behaves_like 'thread comments for issue, epic and merge request', 'merge request'
 end

spec/features/discussion_comments/snippets_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe 'Thread Comments Snippet', :js do
       visit project_snippet_path(project, snippet)
     end
 
-    it_behaves_like 'thread comments', 'snippet'
+    it_behaves_like 'thread comments for commit and snippet', 'snippet'
   end
 
   context 'with personal snippets' do
@@ -32,6 +32,6 @@ RSpec.describe 'Thread Comments Snippet', :js do
       visit snippet_path(snippet)
     end
 
-    it_behaves_like 'thread comments', 'snippet'
+    it_behaves_like 'thread comments for commit and snippet', 'snippet'
   end
 end

spec/features/merge_request/user_posts_notes_spec.rb

@@ -44,7 +44,10 @@ RSpec.describe 'Merge request > User posts notes', :js do
 
       it 'has enable submit button, preview button and saves content to local storage' do
         page.within('.js-main-target-form') do
-          expect(page).not_to have_css('.js-comment-button[disabled]')
+          page.within('[data-testid="comment-button"]') do
+            expect(page).to have_css('.split-content-button')
+            expect(page).not_to have_css('.split-content-button[disabled]')
+          end
           expect(page).to have_css('.js-md-preview-button', visible: true)
         end
 

spec/frontend/notes/components/comment_form_spec.js

+import { GlDropdown } from '@gitlab/ui';
 import { mount, shallowMount } from '@vue/test-utils';
 import Autosize from 'autosize';
 import MockAdapter from 'axios-mock-adapter';
@@ -23,9 +24,10 @@ describe('issue_comment_form component', () => {
   let axiosMock;
 
   const findCloseReopenButton = () => wrapper.findByTestId('close-reopen-button');
-  const findCommentButton = () => wrapper.findByTestId('comment-button');
   const findTextArea = () => wrapper.findByTestId('comment-field');
   const findConfidentialNoteCheckbox = () => wrapper.findByTestId('confidential-note-checkbox');
+  const findCommentGlDropdown = () => wrapper.find(GlDropdown);
+  const findCommentButton = () => findCommentGlDropdown().find('button');
 
   const createNotableDataMock = (data = {}) => {
     return {
@@ -243,7 +245,7 @@ describe('issue_comment_form component', () => {
       it('should render comment button as disabled', () => {
         mountComponent();
 
-        expect(findCommentButton().props('disabled')).toBe(true);
+        expect(findCommentGlDropdown().props('disabled')).toBe(true);
       });
 
       it('should enable comment button if it has note', async () => {
@@ -251,7 +253,7 @@ describe('issue_comment_form component', () => {
 
         await wrapper.setData({ note: 'Foo' });
 
-        expect(findCommentButton().props('disabled')).toBe(false);
+        expect(findCommentGlDropdown().props('disabled')).toBe(false);
       });
 
       it('should update buttons texts when it has note', () => {
@@ -437,7 +439,7 @@ describe('issue_comment_form component', () => {
             await wrapper.vm.$nextTick();
 
             // submit comment
-            wrapper.findByTestId('comment-button').trigger('click');
+            findCommentButton().trigger('click');
 
             const [providedData] = wrapper.vm.saveNote.mock.calls[0];
             expect(providedData.data.note.confidential).toBe(shouldCheckboxBeChecked);

spec/frontend/notes/components/notes_app_spec.js

@@ -33,6 +33,8 @@ describe('note_app', () => {
   let wrapper;
   let store;
 
+  const findCommentButton = () => wrapper.find('[data-testid="comment-button"]');
+
   const getComponentOrder = () => {
     return wrapper
       .findAll('#notes-list,.js-comment-form')
@@ -144,7 +146,7 @@ describe('note_app', () => {
     });
 
     it('should render form comment button as disabled', () => {
-      expect(wrapper.find('.js-note-new-discussion').attributes('disabled')).toEqual('disabled');
+      expect(findCommentButton().props('disabled')).toEqual(true);
     });
 
     it('updates discussions badge', () => {

spec/support/shared_examples/features/discussion_comments_shared_example.rb

 # frozen_string_literal: true
 
-RSpec.shared_examples 'thread comments' do |resource_name|
+RSpec.shared_examples 'thread comments for commit and snippet' do |resource_name|
   let(:form_selector) { '.js-main-target-form' }
   let(:dropdown_selector) { "#{form_selector} .comment-type-dropdown" }
   let(:toggle_selector) { "#{dropdown_selector} .dropdown-toggle" }
@@ -24,23 +24,6 @@ RSpec.shared_examples 'thread comments' do |resource_name|
     expect(new_comment).not_to have_selector '.discussion'
   end
 
-  if resource_name == 'issue'
-    it "clicking 'Comment & close #{resource_name}' will post a comment and close the #{resource_name}" do
-      find("#{form_selector} .note-textarea").send_keys(comment)
-
-      click_button 'Comment & close issue'
-
-      wait_for_all_requests
-
-      expect(page).to have_content(comment)
-      expect(page).to have_content "@#{user.username} closed"
-
-      new_comment = all(comments_selector).last
-
-      expect(new_comment).not_to have_selector '.discussion'
-    end
-  end
-
   describe 'when the toggle is clicked' do
     before do
       find("#{form_selector} .note-textarea").send_keys(comment)
@@ -110,33 +93,172 @@ RSpec.shared_examples 'thread comments' do |resource_name|
       end
 
       it 'updates the submit button text and closes the dropdown' do
-        button = find(submit_selector)
+        expect(find(submit_selector).value).to eq 'Start thread'
 
-        # on issues page, the submit input is a <button>, on other pages it is <input>
-        if button.tag_name == 'button'
-          expect(find(submit_selector)).to have_content 'Start thread'
-        else
-          expect(find(submit_selector).value).to eq 'Start thread'
+        expect(page).not_to have_selector menu_selector
+      end
+
+      describe 'creating a thread' do
+        before do
+          find(submit_selector).click
+          wait_for_requests
+
+          find(comments_selector, match: :first)
         end
 
-        expect(page).not_to have_selector menu_selector
+        def submit_reply(text)
+          find("#{comments_selector} .js-vue-discussion-reply").click
+          find("#{comments_selector} .note-textarea").send_keys(text)
+
+          find("#{comments_selector} .js-comment-button").click
+          wait_for_requests
+        end
+
+        it 'clicking "Start thread" will post a thread' do
+          expect(page).to have_content(comment)
+
+          new_comment = all(comments_selector).last
+
+          expect(new_comment).to have_selector('.discussion')
+        end
       end
 
-      if resource_name =~ /(issue|merge request)/
-        it 'updates the close button text' do
-          expect(find(close_selector)).to have_content "Start thread & close #{resource_name}"
+      describe 'when opening the menu' do
+        before do
+          find(toggle_selector).click
+        end
+
+        it 'has "Start thread" selected' do
+          find("#{menu_selector} li", match: :first)
+          items = all("#{menu_selector} li")
+
+          expect(items.first).to have_content 'Comment'
+          expect(items.first).not_to have_selector '[data-testid="check-icon"]'
+          expect(items.first['class']).not_to match 'droplab-item-selected'
+
+          expect(items.last).to have_content 'Start thread'
+          expect(items.last).to have_selector '[data-testid="check-icon"]'
+          expect(items.last['class']).to match 'droplab-item-selected'
         end
 
-        it 'typing does not change the close button text' do
-          find("#{form_selector} .note-textarea").send_keys('b')
+        describe 'when selecting "Comment"' do
+          before do
+            find("#{menu_selector} li", match: :first).click
+          end
+
+          it 'updates the submit button text and closes the dropdown' do
+            button = find(submit_selector)
+
+            expect(button.value).to eq 'Comment'
+
+            expect(page).not_to have_selector menu_selector
+          end
 
-          expect(find(close_selector)).to have_content "Start thread & close #{resource_name}"
+          it 'has "Comment" selected when opening the menu', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/196825' do
+            find(toggle_selector).click
+
+            find("#{menu_selector} li", match: :first)
+            items = all("#{menu_selector} li")
+
+            aggregate_failures do
+              expect(items.first).to have_content 'Comment'
+              expect(items.first).to have_selector '[data-testid="check-icon"]'
+              expect(items.first['class']).to match 'droplab-item-selected'
+
+              expect(items.last).to have_content 'Start thread'
+              expect(items.last).not_to have_selector '[data-testid="check-icon"]'
+              expect(items.last['class']).not_to match 'droplab-item-selected'
+            end
+          end
         end
       end
+    end
+  end
+end
+
+RSpec.shared_examples 'thread comments for issue, epic and merge request' do |resource_name|
+  let(:form_selector) { '.js-main-target-form' }
+  let(:dropdown_selector) { "#{form_selector} [data-testid='comment-button']" }
+  let(:submit_button_selector) { "#{dropdown_selector} .split-content-button" }
+  let(:toggle_selector) { "#{dropdown_selector} .dropdown-toggle-split" }
+  let(:menu_selector) { "#{dropdown_selector} .dropdown-menu" }
+  let(:close_selector) { "#{form_selector} .btn-comment-and-close" }
+  let(:comments_selector) { '.timeline > .note.timeline-entry' }
+  let(:comment) { 'My comment' }
+
+  it 'clicking "Comment" will post a comment' do
+    expect(page).to have_selector toggle_selector
+
+    find("#{form_selector} .note-textarea").send_keys(comment)
+
+    find(submit_button_selector).click
+
+    expect(page).to have_content(comment)
+
+    new_comment = all(comments_selector).last
+
+    expect(new_comment).not_to have_selector '.discussion'
+  end
+
+  if resource_name == 'issue'
+    it "clicking 'Comment & close #{resource_name}' will post a comment and close the #{resource_name}" do
+      find("#{form_selector} .note-textarea").send_keys(comment)
+
+      click_button 'Comment & close issue'
+
+      wait_for_all_requests
+
+      expect(page).to have_content(comment)
+      expect(page).to have_content "@#{user.username} closed"
+
+      new_comment = all(comments_selector).last
+
+      expect(new_comment).not_to have_selector '.discussion'
+    end
+  end
+
+  describe 'when the toggle is clicked' do
+    before do
+      find("#{form_selector} .note-textarea").send_keys(comment)
+
+      find(toggle_selector).click
+    end
+
+    it 'has a "Comment" item (selected by default) and "Start thread" item' do
+      expect(page).to have_selector menu_selector
+
+      find("#{menu_selector} li", match: :first)
+      items = all("#{menu_selector} li")
+
+      expect(page).to have_selector("#{dropdown_selector}[data-track-label='comment_button']")
+
+      expect(items.first).to have_content 'Comment'
+      expect(items.first).to have_content "Add a general comment to this #{resource_name}."
+
+      expect(items.last).to have_content 'Start thread'
+      expect(items.last).to have_content "Discuss a specific suggestion or question#{' that needs to be resolved' if resource_name == 'merge request'}."
+    end
+
+    it 'closes the menu when clicking the toggle or body' do
+      find(toggle_selector).click
+
+      expect(page).not_to have_selector menu_selector
+
+      find(toggle_selector).click
+      find("#{form_selector} .note-textarea").click
+
+      expect(page).not_to have_selector menu_selector
+    end
+
+    describe 'when selecting "Start thread"' do
+      before do
+        find("#{menu_selector} li", match: :first)
+        all("#{menu_selector} li").last.click
+      end
 
       describe 'creating a thread' do
         before do
-          find(submit_selector).click
+          find(submit_button_selector).click
           wait_for_requests
 
           find(comments_selector, match: :first)
@@ -146,6 +268,7 @@ RSpec.shared_examples 'thread comments' do |resource_name|
           find("#{comments_selector} .js-vue-discussion-reply").click
           find("#{comments_selector} .note-textarea").send_keys(text)
 
+          # .js-comment-button here refers to the reply button in note_form.vue
           find("#{comments_selector} .js-comment-button").click
           wait_for_requests
         end
@@ -228,13 +351,11 @@ RSpec.shared_examples 'thread comments' do |resource_name|
           find("#{menu_selector} li", match: :first)
           items = all("#{menu_selector} li")
 
+          expect(page).to have_selector("#{dropdown_selector}[data-track-label='start_thread_button']")
+
           expect(items.first).to have_content 'Comment'
-          expect(items.first).not_to have_selector '[data-testid="check-icon"]'
-          expect(items.first['class']).not_to match 'droplab-item-selected'
 
           expect(items.last).to have_content 'Start thread'
-          expect(items.last).to have_selector '[data-testid="check-icon"]'
-          expect(items.last['class']).to match 'droplab-item-selected'
         end
 
         describe 'when selecting "Comment"' do
@@ -243,14 +364,9 @@ RSpec.shared_examples 'thread comments' do |resource_name|
           end
 
           it 'updates the submit button text and closes the dropdown' do
-            button = find(submit_selector)
+            button = find(submit_button_selector)
 
-            # on issues page, the submit input is a <button>, on other pages it is <input>
-            if button.tag_name == 'button'
-              expect(button).to have_content 'Comment'
-            else
-              expect(button.value).to eq 'Comment'
-            end
+            expect(button).to have_content 'Comment'
 
             expect(page).not_to have_selector menu_selector
           end
@@ -267,21 +383,17 @@ RSpec.shared_examples 'thread comments' do |resource_name|
             end
           end
 
-          it 'has "Comment" selected when opening the menu', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/196825' do
+          it 'has "Comment" selected when opening the menu' do
             find(toggle_selector).click
 
             find("#{menu_selector} li", match: :first)
             items = all("#{menu_selector} li")
 
-            aggregate_failures do
-              expect(items.first).to have_content 'Comment'
-              expect(items.first).to have_selector '[data-testid="check-icon"]'
-              expect(items.first['class']).to match 'droplab-item-selected'
+            expect(page).to have_selector("#{dropdown_selector}[data-track-label='comment_button']")
 
-              expect(items.last).to have_content 'Start thread'
-              expect(items.last).not_to have_selector '[data-testid="check-icon"]'
-              expect(items.last['class']).not_to match 'droplab-item-selected'
-            end
+            expect(items.first).to have_content 'Comment'
+
+            expect(items.last).to have_content 'Start thread'
           end
         end
       end

spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb

@@ -93,6 +93,6 @@ end
 
 def submit_time(quick_action)
   fill_in 'note[note]', with: quick_action
-  find('.js-comment-submit-button').click
+  find('[data-testid="comment-button"]').click
   wait_for_requests
 end