Merge branch 'ph/223077/lazyRenderDiffCommentButton' into 'master'

Lazy render diff line comment button

Closes #223077

See merge request gitlab-org/gitlab!35006

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

@@ -4,15 +4,13 @@ import { GlIcon } from '@gitlab/ui';
 import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils';
 import DiffGutterAvatars from './diff_gutter_avatars.vue';
 import {
-  MATCH_LINE_TYPE,
   CONTEXT_LINE_TYPE,
   LINE_POSITION_RIGHT,
   EMPTY_CELL_TYPE,
-  OLD_LINE_TYPE,
   OLD_NO_NEW_LINE_TYPE,
+  OLD_LINE_TYPE,
   NEW_NO_NEW_LINE_TYPE,
   LINE_HOVER_CLASS_NAME,
-  LINE_UNFOLD_CLASS_NAME,
 } from '../constants';
 
 export default {
@@ -29,10 +27,6 @@ export default {
       type: String,
       required: true,
     },
-    contextLinesPath: {
-      type: String,
-      required: true,
-    },
     isHighlighted: {
       type: Boolean,
       required: true,
@@ -52,11 +46,6 @@ export default {
       required: false,
       default: '',
     },
-    isContentLine: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
     isBottom: {
       type: Boolean,
       required: false,
@@ -68,6 +57,11 @@ export default {
       default: false,
     },
   },
+  data() {
+    return {
+      isCommentButtonRendered: false,
+    };
+  },
   computed: {
     ...mapGetters(['isLoggedIn']),
     lineCode() {
@@ -81,13 +75,7 @@ export default {
       return `#${this.line.line_code || ''}`;
     },
     shouldShowCommentButton() {
-      return (
-        this.isHover &&
-        !this.isMatchLine &&
-        !this.isContextLine &&
-        !this.isMetaLine &&
-        !this.hasDiscussions
-      );
+      return this.isHover && !this.isContextLine && !this.isMetaLine && !this.hasDiscussions;
     },
     hasDiscussions() {
       return this.line.discussions && this.line.discussions.length > 0;
@@ -99,6 +87,10 @@ export default {
       return this.showCommentButton && this.hasDiscussions;
     },
     shouldRenderCommentButton() {
+      if (!this.isCommentButtonRendered) {
+        return false;
+      }
+
       if (this.isLoggedIn && this.showCommentButton) {
         const isDiffHead = parseBoolean(getParameterByName('diff_head'));
         return !isDiffHead || gon.features?.mergeRefHeadComments;
@@ -106,9 +98,6 @@ export default {
 
       return false;
     },
-    isMatchLine() {
-      return this.line.type === MATCH_LINE_TYPE;
-    },
     isContextLine() {
       return this.line.type === CONTEXT_LINE_TYPE;
     },
@@ -126,13 +115,8 @@ export default {
         type,
         {
           hll: this.isHighlighted,
-          [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine,
           [LINE_HOVER_CLASS_NAME]:
-            this.isLoggedIn &&
-            this.isHover &&
-            !this.isMatchLine &&
-            !this.isContextLine &&
-            !this.isMetaLine,
+            this.isLoggedIn && this.isHover && !this.isContextLine && !this.isMetaLine,
         },
       ];
     },
@@ -140,6 +124,17 @@ export default {
       return this.lineType === OLD_LINE_TYPE ? this.line.old_line : this.line.new_line;
     },
   },
+  mounted() {
+    this.unwatchShouldShowCommentButton = this.$watch('shouldShowCommentButton', newVal => {
+      if (newVal) {
+        this.isCommentButtonRendered = true;
+        this.unwatchShouldShowCommentButton();
+      }
+    });
+  },
+  beforeDestroy() {
+    this.unwatchShouldShowCommentButton();
+  },
   methods: {
     ...mapActions('diffs', ['showCommentForm', 'setHighlightedRow', 'toggleLineDiscussions']),
     handleCommentButton() {
@@ -151,34 +146,32 @@ export default {
 
 <template>
   <td ref="td" :class="classNameMap">
-    <div>
-      <button
-        v-if="shouldRenderCommentButton"
-        v-show="shouldShowCommentButton"
-        ref="addDiffNoteButton"
-        type="button"
-        class="add-diff-note js-add-diff-note-button qa-diff-comment"
-        title="Add a comment to this line"
-        @click="handleCommentButton"
-      >
-        <gl-icon :size="12" name="comment" />
-      </button>
-      <a
-        v-if="lineNumber"
-        ref="lineNumberRef"
-        :data-linenumber="lineNumber"
-        :href="lineHref"
-        @click="setHighlightedRow(lineCode)"
-      >
-      </a>
-      <diff-gutter-avatars
-        v-if="shouldShowAvatarsOnGutter"
-        :discussions="line.discussions"
-        :discussions-expanded="line.discussionsExpanded"
-        @toggleLineDiscussions="
-          toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded })
-        "
-      />
-    </div>
+    <button
+      v-if="shouldRenderCommentButton"
+      v-show="shouldShowCommentButton"
+      ref="addDiffNoteButton"
+      type="button"
+      class="add-diff-note js-add-diff-note-button qa-diff-comment"
+      title="Add a comment to this line"
+      @click="handleCommentButton"
+    >
+      <gl-icon :size="12" name="comment" />
+    </button>
+    <a
+      v-if="lineNumber"
+      ref="lineNumberRef"
+      :data-linenumber="lineNumber"
+      :href="lineHref"
+      @click="setHighlightedRow(lineCode)"
+    >
+    </a>
+    <diff-gutter-avatars
+      v-if="shouldShowAvatarsOnGutter"
+      :discussions="line.discussions"
+      :discussions-expanded="line.discussionsExpanded"
+      @toggleLineDiscussions="
+        toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded })
+      "
+    />
   </td>
 </template>

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

@@ -28,10 +28,6 @@ export default {
       type: String,
       required: true,
     },
-    contextLinesPath: {
-      type: String,
-      required: true,
-    },
     line: {
       type: Object,
       required: true,
@@ -106,7 +102,6 @@ export default {
   >
     <diff-table-cell
       :file-hash="fileHash"
-      :context-lines-path="contextLinesPath"
       :line="line"
       :line-type="oldLineType"
       :is-bottom="isBottom"
@@ -117,7 +112,6 @@ export default {
     />
     <diff-table-cell
       :file-hash="fileHash"
-      :context-lines-path="contextLinesPath"
       :line="line"
       :line-type="newLineType"
       :is-bottom="isBottom"

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

@@ -65,7 +65,6 @@ export default {
           :key="`${line.line_code || index}`"
           :file-hash="diffFile.file_hash"
           :file-path="diffFile.file_path"
-          :context-lines-path="diffFile.context_lines_path"
           :line="line"
           :is-bottom="index + 1 === diffLinesLength"
         />

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

@@ -31,10 +31,6 @@ export default {
       type: String,
       required: true,
     },
-    contextLinesPath: {
-      type: String,
-      required: true,
-    },
     line: {
       type: Object,
       required: true,
@@ -144,7 +140,6 @@ export default {
     <template v-if="line.left && !isMatchLineLeft">
       <diff-table-cell
         :file-hash="fileHash"
-        :context-lines-path="contextLinesPath"
         :line="line.left"
         :line-type="oldLineType"
         :is-bottom="isBottom"
@@ -172,7 +167,6 @@ export default {
     <template v-if="line.right && !isMatchLineRight">
       <diff-table-cell
         :file-hash="fileHash"
-        :context-lines-path="contextLinesPath"
         :line="line.right"
         :line-type="newLineType"
         :is-bottom="isBottom"

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

@@ -67,7 +67,6 @@ export default {
           :key="line.line_code"
           :file-hash="diffFile.file_hash"
           :file-path="diffFile.file_path"
-          :context-lines-path="diffFile.context_lines_path"
           :line="line"
           :is-bottom="index + 1 === diffLinesLength"
         />

spec/frontend/diffs/components/diff_table_cell_spec.js

@@ -100,7 +100,11 @@ describe('DiffTableCell', () => {
         setWindowLocation({ href: `${TEST_HOST}?${query}` });
         createComponent({ showCommentButton });
 
-        expect(findNoteButton().exists()).toBe(expectation);
+        wrapper.setData({ isCommentButtonRendered: showCommentButton });
+
+        return wrapper.vm.$nextTick().then(() => {
+          expect(findNoteButton().exists()).toBe(expectation);
+        });
       },
     );
 
@@ -108,7 +112,6 @@ describe('DiffTableCell', () => {
       isHover  | otherProps                                      | discussions | expectation
       ${true}  | ${{}}                                           | ${[]}       | ${true}
       ${false} | ${{}}                                           | ${[]}       | ${false}
-      ${true}  | ${{ line: { ...line, type: 'match' } }}         | ${[]}       | ${false}
       ${true}  | ${{ line: { ...line, type: 'context' } }}       | ${[]}       | ${false}
       ${true}  | ${{ line: { ...line, type: 'old-nonewline' } }} | ${[]}       | ${false}
       ${true}  | ${{}}                                           | ${[{}]}     | ${false}
@@ -122,7 +125,13 @@ describe('DiffTableCell', () => {
           ...otherProps,
         });
 
-        expect(findNoteButton().isVisible()).toBe(expectation);
+        wrapper.setData({
+          isCommentButtonRendered: true,
+        });
+
+        return wrapper.vm.$nextTick().then(() => {
+          expect(findNoteButton().isVisible()).toBe(expectation);
+        });
       },
     );
   });