Merge branch '327013-draggable-comment-icon-for-multiline-comment-broken-on-firefox' into 'master'

Resolve "Draggable comment icon for multiline comment broken on Firefox"

See merge request gitlab-org/gitlab!58692

app/assets/javascripts/diffs/components/diff_row.vue

@@ -204,27 +204,32 @@ export default {
       <template v-if="line.left && line.left.type !== $options.CONFLICT_MARKER">
         <div
           :class="classNameMapCellLeft"
-          data-testid="leftLineNumber"
+          data-testid="left-line-number"
           class="diff-td diff-line-num"
         >
           <template v-if="!isLeftConflictMarker">
             <span
               v-if="shouldRenderCommentButton && !line.hasDiscussionsLeft"
               v-gl-tooltip
-              data-testid="leftCommentButton"
               class="add-diff-note tooltip-wrapper"
               :title="addCommentTooltipLeft"
             >
-              <button
-                :draggable="glFeatures.dragCommentSelection"
+              <div
+                data-testid="left-comment-button"
+                role="button"
+                tabindex="0"
+                :draggable="!line.left.commentsDisabled && glFeatures.dragCommentSelection"
                 type="button"
                 class="add-diff-note unified-diff-components-diff-note-button note-button js-add-diff-note-button qa-diff-comment"
                 data-qa-selector="diff_comment_button"
                 :class="{ 'gl-cursor-grab': dragging }"
                 :disabled="line.left.commentsDisabled"
-                @click="handleCommentButton(line.left)"
-                @dragstart="onDragStart({ ...line.left, index })"
-              ></button>
+                :aria-disabled="line.left.commentsDisabled"
+                @click="!line.left.commentsDisabled && handleCommentButton(line.left)"
+                @keydown.enter="!line.left.commentsDisabled && handleCommentButton(line.left)"
+                @keydown.space="!line.left.commentsDisabled && handleCommentButton(line.left)"
+                @dragstart="!line.left.commentsDisabled && onDragStart({ ...line.left, index })"
+              ></div>
             </span>
           </template>
           <a
@@ -238,7 +243,7 @@ export default {
             v-if="line.hasDiscussionsLeft"
             :discussions="line.left.discussions"
             :discussions-expanded="line.left.discussionsExpanded"
-            data-testid="leftDiscussions"
+            data-testid="left-discussions"
             @toggleLineDiscussions="
               toggleLineDiscussions({
                 lineCode: line.left.line_code,
@@ -268,7 +273,7 @@ export default {
           :key="line.left.line_code"
           :class="[parallelViewLeftLineType, { parallel: !inline }]"
           class="diff-td line_content with-coverage left-side"
-          data-testid="leftContent"
+          data-testid="left-content"
           @mousedown="handleParallelLineMouseDown"
         >
           <strong v-if="isLeftConflictMarker">{{ conflictText(line.left) }}</strong>
@@ -277,7 +282,7 @@ export default {
       </template>
       <template v-else-if="!inline || (line.left && line.left.type === $options.CONFLICT_MARKER)">
         <div
-          data-testid="leftEmptyCell"
+          data-testid="left-empty-cell"
           class="diff-td diff-line-num old_line empty-cell"
           :class="emptyCellLeftClassMap"
         >
@@ -313,19 +318,24 @@ export default {
             <span
               v-if="shouldRenderCommentButton && !line.hasDiscussionsRight"
               v-gl-tooltip
-              data-testid="rightCommentButton"
               class="add-diff-note tooltip-wrapper"
               :title="addCommentTooltipRight"
             >
-              <button
-                :draggable="glFeatures.dragCommentSelection"
+              <div
+                data-testid="right-comment-button"
+                role="button"
+                tabindex="0"
+                :draggable="!line.right.commentsDisabled && glFeatures.dragCommentSelection"
                 type="button"
                 class="add-diff-note unified-diff-components-diff-note-button note-button js-add-diff-note-button qa-diff-comment"
                 :class="{ 'gl-cursor-grab': dragging }"
                 :disabled="line.right.commentsDisabled"
-                @click="handleCommentButton(line.right)"
-                @dragstart="onDragStart({ ...line.right, index })"
-              ></button>
+                :aria-disabled="line.right.commentsDisabled"
+                @click="!line.right.commentsDisabled && handleCommentButton(line.right)"
+                @keydown.enter="!line.right.commentsDisabled && handleCommentButton(line.right)"
+                @keydown.space="!line.right.commentsDisabled && handleCommentButton(line.right)"
+                @dragstart="!line.right.commentsDisabled && onDragStart({ ...line.right, index })"
+              ></div>
             </span>
           </template>
           <a
@@ -339,7 +349,7 @@ export default {
             v-if="line.hasDiscussionsRight"
             :discussions="line.right.discussions"
             :discussions-expanded="line.right.discussionsExpanded"
-            data-testid="rightDiscussions"
+            data-testid="right-discussions"
             @toggleLineDiscussions="
               toggleLineDiscussions({
                 lineCode: line.right.line_code,
@@ -381,7 +391,7 @@ export default {
       </template>
       <template v-else>
         <div
-          data-testid="rightEmptyCell"
+          data-testid="right-empty-cell"
           class="diff-td diff-line-num old_line empty-cell"
           :class="emptyCellRightClassMap"
         ></div>

changelogs/unreleased/327013-draggable-comment-icon-for-multiline-comment-broken-on-firefox.yml

+---
+title: Fix multiline comment dragging in Firefox
+merge_request: 58692
+author:
+type: fixed

spec/frontend/diffs/components/diff_row_spec.js

 import { getByTestId, fireEvent } from '@testing-library/dom';
-import { shallowMount, createLocalVue } from '@vue/test-utils';
+import { shallowMount } from '@vue/test-utils';
+import Vue from 'vue';
 import Vuex from 'vuex';
 import DiffRow from '~/diffs/components/diff_row.vue';
 import { mapParallel } from '~/diffs/components/diff_row_utils';
@@ -28,12 +29,12 @@ describe('DiffRow', () => {
     },
   ];
 
-  const createWrapper = ({ props, state, isLoggedIn = true }) => {
-    const localVue = createLocalVue();
-    localVue.use(Vuex);
+  const createWrapper = ({ props, state, actions, isLoggedIn = true }) => {
+    Vue.use(Vuex);
 
     const diffs = diffsModule();
     diffs.state = { ...diffs.state, ...state };
+    diffs.actions = { ...diffs.actions, ...actions };
 
     const getters = { isLoggedIn: () => isLoggedIn };
 
@@ -54,7 +55,7 @@ describe('DiffRow', () => {
       glFeatures: { dragCommentSelection: true },
     };
 
-    return shallowMount(DiffRow, { propsData, localVue, store, provide });
+    return shallowMount(DiffRow, { propsData, store, provide });
   };
 
   it('isHighlighted returns true given line.left', () => {
@@ -95,6 +96,9 @@ describe('DiffRow', () => {
     expect(wrapper.vm.isHighlighted).toBe(false);
   });
 
+  const getCommentButton = (wrapper, side) =>
+    wrapper.find(`[data-testid="${side}-comment-button"]`);
+
   describe.each`
     side
     ${'left'}
@@ -102,18 +106,59 @@ describe('DiffRow', () => {
   `('$side side', ({ side }) => {
     it(`renders empty cells if ${side} is unavailable`, () => {
       const wrapper = createWrapper({ props: { line: testLines[2], inline: false } });
-      expect(wrapper.find(`[data-testid="${side}LineNumber"]`).exists()).toBe(false);
-      expect(wrapper.find(`[data-testid="${side}EmptyCell"]`).exists()).toBe(true);
+      expect(wrapper.find(`[data-testid="${side}-line-number"]`).exists()).toBe(false);
+      expect(wrapper.find(`[data-testid="${side}-empty-cell"]`).exists()).toBe(true);
     });
 
-    it('renders comment button', () => {
-      const wrapper = createWrapper({ props: { line: testLines[3], inline: false } });
-      expect(wrapper.find(`[data-testid="${side}CommentButton"]`).exists()).toBe(true);
+    describe('comment button', () => {
+      const showCommentForm = jest.fn();
+      let line;
+
+      beforeEach(() => {
+        showCommentForm.mockReset();
+        // https://eslint.org/docs/rules/prefer-destructuring#when-not-to-use-it
+        // eslint-disable-next-line prefer-destructuring
+        line = testLines[3];
+      });
+
+      it('renders', () => {
+        const wrapper = createWrapper({ props: { line, inline: false } });
+        expect(getCommentButton(wrapper, side).exists()).toBe(true);
+      });
+
+      it('responds to click and keyboard events', async () => {
+        const wrapper = createWrapper({
+          props: { line, inline: false },
+          actions: { showCommentForm },
+        });
+        const commentButton = getCommentButton(wrapper, side);
+
+        await commentButton.trigger('click');
+        await commentButton.trigger('keydown.enter');
+        await commentButton.trigger('keydown.space');
+
+        expect(showCommentForm).toHaveBeenCalledTimes(3);
+      });
+
+      it('ignores click and keyboard events when comments are disabled', async () => {
+        line[side].commentsDisabled = true;
+        const wrapper = createWrapper({
+          props: { line, inline: false },
+          actions: { showCommentForm },
+        });
+        const commentButton = getCommentButton(wrapper, side);
+
+        await commentButton.trigger('click');
+        await commentButton.trigger('keydown.enter');
+        await commentButton.trigger('keydown.space');
+
+        expect(showCommentForm).not.toHaveBeenCalled();
+      });
     });
 
     it('renders avatars', () => {
       const wrapper = createWrapper({ props: { line: testLines[0], inline: false } });
-      expect(wrapper.find(`[data-testid="${side}Discussions"]`).exists()).toBe(true);
+      expect(wrapper.find(`[data-testid="${side}-discussions"]`).exists()).toBe(true);
     });
   });