Re-implemented image commenting on diffs

This re-implements image commenting in merge request diffs.
This feature was previously lost when the merge request
page was refactored into Vue.

With this, we create an overlay component. The overlay
component handles displaying the comment badges
and the comment form badge.
Badges are displayed based on the position attribute
sent with the discussion.

Comment forms for diff files are controlled through
a different state property. This is so we don't
tie comment forms to diff files directly creating
deep nested state. Instead we create a flat array
which holds the file hash & the X & Y position of
the comment form.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/48956

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

@@ -223,7 +223,10 @@ export default {
         :commit="commit"
       />
 
-      <div class="files d-flex prepend-top-default">
+      <div
+        :data-can-create-note="getNoteableData.current_user.can_create_note"
+        class="files d-flex prepend-top-default"
+      >
         <div
           v-show="showTreeList"
           class="diff-tree-list"

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

 <script>
-import { mapGetters, mapState } from 'vuex';
+import { mapActions, mapGetters, mapState } from 'vuex';
 import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
-import { diffModes } from '~/ide/constants';
 import InlineDiffView from './inline_diff_view.vue';
 import ParallelDiffView from './parallel_diff_view.vue';
+import NoteForm from '../../notes/components/note_form.vue';
+import ImageDiffOverlay from './image_diff_overlay.vue';
+import DiffDiscussions from './diff_discussions.vue';
+import { IMAGE_DIFF_POSITION_TYPE } from '../constants';
+import { getDiffMode } from '../store/utils';
 
 export default {
   components: {
     InlineDiffView,
     ParallelDiffView,
     DiffViewer,
+    NoteForm,
+    DiffDiscussions,
+    ImageDiffOverlay,
   },
   props: {
     diffFile: {
@@ -23,13 +30,36 @@ export default {
       endpoint: state => state.diffs.endpoint,
     }),
     ...mapGetters('diffs', ['isInlineView', 'isParallelView']),
+    ...mapGetters('diffs', ['getCommentFormForDiffFile']),
+    ...mapGetters(['getNoteableData', 'noteableType']),
     diffMode() {
-      const diffModeKey = Object.keys(diffModes).find(key => this.diffFile[`${key}File`]);
-      return diffModes[diffModeKey] || diffModes.replaced;
+      return getDiffMode(this.diffFile);
     },
     isTextFile() {
       return this.diffFile.viewer.name === 'text';
     },
+    diffFileCommentForm() {
+      return this.getCommentFormForDiffFile(this.diffFile.fileHash);
+    },
+    showNotesContainer() {
+      return this.diffFile.discussions.length || this.diffFileCommentForm;
+    },
+  },
+  methods: {
+    ...mapActions('diffs', ['saveDiffDiscussion', 'closeDiffFileCommentForm']),
+    handleSaveNote(note) {
+      this.saveDiffDiscussion({
+        note,
+        formData: {
+          noteableData: this.getNoteableData,
+          noteableType: this.noteableType,
+          diffFile: this.diffFile,
+          positionType: IMAGE_DIFF_POSITION_TYPE,
+          x: this.diffFileCommentForm.x,
+          y: this.diffFileCommentForm.y,
+        },
+      });
+    },
   },
 };
 </script>
@@ -56,7 +86,37 @@ export default {
         :new-sha="diffFile.diffRefs.headSha"
         :old-path="diffFile.oldPath"
         :old-sha="diffFile.diffRefs.baseSha"
-        :project-path="projectPath"/>
+        :file-hash="diffFile.fileHash"
+        :project-path="projectPath"
+      >
+        <image-diff-overlay
+          slot="image-overlay"
+          :discussions="diffFile.discussions"
+          :file-hash="diffFile.fileHash"
+          :can-comment="getNoteableData.current_user.can_create_note"
+        />
+        <div
+          v-if="showNotesContainer"
+          class="note-container"
+        >
+          <diff-discussions
+            v-if="diffFile.discussions.length"
+            class="diff-file-discussions"
+            :discussions="diffFile.discussions"
+            :should-collapse-discussions="true"
+            :render-avatar-badge="true"
+          />
+          <note-form
+            v-if="diffFileCommentForm"
+            ref="noteForm"
+            :is-editing="false"
+            :save-button-title="__('Comment')"
+            class="diff-comment-form new-note discussion-form discussion-form-container"
+            @handleFormUpdate="handleSaveNote"
+            @cancelForm="closeDiffFileCommentForm(diffFile.fileHash)"
+          />          
+        </div>
+      </diff-viewer>
     </div>
   </div>
 </template>

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

 <script>
 import { mapActions } from 'vuex';
+import Icon from '~/vue_shared/components/icon.vue';
 import noteableDiscussion from '../../notes/components/noteable_discussion.vue';
 
 export default {
   components: {
     noteableDiscussion,
+    Icon,
   },
   props: {
     discussions: {
       type: Array,
       required: true,
     },
+    shouldCollapseDiscussions: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    renderAvatarBadge: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   methods: {
+    ...mapActions(['toggleDiscussion']),
     ...mapActions('diffs', ['removeDiscussionsFromDiff']),
     deleteNoteHandler(discussion) {
       if (discussion.notes.length <= 1) {
         this.removeDiscussionsFromDiff(discussion);
       }
     },
+    isExpanded(discussion) {
+      return this.shouldCollapseDiscussions ? discussion.expanded : true;
+    },
   },
 };
 </script>
@@ -26,22 +42,54 @@ export default {
 <template>
   <div>
     <div
-      v-for="discussion in discussions"
+      v-for="(discussion, index) in discussions"
       :key="discussion.id"
-      class="discussion-notes diff-discussions"
+      :class="{
+        collapsed: !isExpanded(discussion)
+      }"
+      class="discussion-notes diff-discussions position-relative"
     >
       <ul
         :data-discussion-id="discussion.id"
         class="notes"
       >
+        <template v-if="shouldCollapseDiscussions">
+          <button
+            :class="{
+              'diff-notes-collapse': discussion.expanded,
+              'btn-transparent badge badge-pill': !discussion.expanded
+            }"
+            type="button"
+            class="js-diff-notes-toggle"
+            @click="toggleDiscussion({ discussionId: discussion.id })"
+          >
+            <icon
+              v-if="discussion.expanded"
+              name="collapse"
+              class="collapse-icon"
+            />
+            <template v-else>
+              {{ index + 1 }}
+            </template>
+          </button>
+        </template>
         <noteable-discussion
+          v-show="isExpanded(discussion)"
           :discussion="discussion"
           :render-header="false"
           :render-diff-file="false"
           :always-expanded="true"
           :discussions-by-diff-order="true"
           @noteDeleted="deleteNoteHandler"
-        />
+        >
+          <span
+            v-if="renderAvatarBadge"
+            slot="avatar-badge"
+            class="badge badge-pill"
+          >
+            {{ index + 1 }}
+          </span>
+        </noteable-discussion>
       </ul>
     </div>
   </div>

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

+<script>
+import { mapActions, mapGetters } from 'vuex';
+import _ from 'underscore';
+import Icon from '~/vue_shared/components/icon.vue';
+
+export default {
+  name: 'ImageDiffOverlay',
+  components: {
+    Icon,
+  },
+  props: {
+    discussions: {
+      type: [Array, Object],
+      required: true,
+    },
+    fileHash: {
+      type: String,
+      required: true,
+    },
+    canComment: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    showCommentIcon: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+    badgeClass: {
+      type: String,
+      required: false,
+      default: 'badge badge-pill',
+    },
+    shouldToggleDiscussion: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
+  },
+  computed: {
+    ...mapGetters('diffs', ['getDiffFileByHash', 'getCommentFormForDiffFile']),
+    currentCommentForm() {
+      return this.getCommentFormForDiffFile(this.fileHash);
+    },
+    allDiscussions() {
+      return _.isArray(this.discussions) ? this.discussions : [this.discussions];
+    },
+  },
+  methods: {
+    ...mapActions(['toggleDiscussion']),
+    ...mapActions('diffs', ['openDiffFileCommentForm']),
+    getPosition(discussion) {
+      return {
+        left: `${discussion.position.x}px`,
+        top: `${discussion.position.y}px`,
+      };
+    },
+    clickedImage(x, y) {
+      this.openDiffFileCommentForm({
+        fileHash: this.fileHash,
+        x,
+        y,
+      });
+    },
+  },
+};
+</script>
+
+<template>
+  <div class="position-absolute w-100 h-100 image-diff-overlay">
+    <button
+      v-if="canComment"
+      type="button"
+      class="btn-transparent position-absolute image-diff-overlay-add-comment w-100 h-100 js-add-image-diff-note-button"
+      @click="clickedImage($event.offsetX, $event.offsetY)"
+    >
+      <span class="sr-only">
+        {{ __('Add image comment') }}
+      </span>
+    </button>
+    <button
+      v-for="(discussion, index) in allDiscussions"
+      :key="discussion.id"
+      :style="getPosition(discussion)"
+      :class="badgeClass"
+      :disabled="!shouldToggleDiscussion"
+      class="js-image-badge"
+      type="button"
+      @click="toggleDiscussion({ discussionId: discussion.id })"
+    >
+      <icon
+        v-if="showCommentIcon"
+        name="image-comment-dark"
+      />
+      <template v-else>
+        {{ index + 1 }}
+      </template>
+    </button>
+    <button
+      v-if="currentCommentForm"
+      :style="{
+        left: `${currentCommentForm.x}px`,
+        top: `${currentCommentForm.y}px`
+      }"
+      :aria-label="__('Comment form position')"
+      class="btn-transparent comment-indicator"
+      type="button"
+    >
+      <icon
+        name="image-comment-dark"
+      />
+    </button>
+  </div>
+</template>

app/assets/javascripts/diffs/constants.js

@@ -12,6 +12,7 @@ export const NOTE_TYPE = 'Note';
 export const NEW_LINE_TYPE = 'new';
 export const OLD_LINE_TYPE = 'old';
 export const TEXT_DIFF_POSITION_TYPE = 'text';
+export const IMAGE_DIFF_POSITION_TYPE = 'image';
 
 export const LINE_POSITION_LEFT = 'left';
 export const LINE_POSITION_RIGHT = 'right';

app/assets/javascripts/diffs/store/actions.js

@@ -50,8 +50,8 @@ export const assignDiscussionsToDiff = (
 };
 
 export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => {
-  const { fileHash, line_code } = removeDiscussion;
-  commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code });
+  const { fileHash, line_code, id } = removeDiscussion;
+  commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code, id });
 };
 
 export const startRenderDiffsQueue = ({ state, commit }) => {
@@ -189,6 +189,7 @@ export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => {
   return dispatch('saveNote', postData, { root: true })
     .then(result => dispatch('updateDiscussion', result.discussion, { root: true }))
     .then(discussion => dispatch('assignDiscussionsToDiff', [discussion]))
+    .then(() => dispatch('closeDiffFileCommentForm', formData.diffFile.fileHash))
     .catch(() => createFlash(s__('MergeRequests|Saving the comment failed')));
 };
 
@@ -210,5 +211,19 @@ export const toggleShowTreeList = ({ commit, state }) => {
   localStorage.setItem(MR_TREE_SHOW_KEY, state.showTreeList);
 };
 
+export const openDiffFileCommentForm = ({ commit, getters }, formData) => {
+  const form = getters.getCommentFormForDiffFile(formData.fileHash);
+
+  if (form) {
+    commit(types.UPDATE_DIFF_FILE_COMMENT_FORM, formData);
+  } else {
+    commit(types.OPEN_DIFF_FILE_COMMENT_FORM, formData);
+  }
+};
+
+export const closeDiffFileCommentForm = ({ commit }, fileHash) => {
+  commit(types.CLOSE_DIFF_FILE_COMMENT_FORM, fileHash);
+};
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

app/assets/javascripts/diffs/store/getters.js

@@ -114,5 +114,8 @@ export const allBlobs = state => Object.values(state.treeEntries).filter(f => f.
 
 export const diffFilesLength = state => state.diffFiles.length;
 
+export const getCommentFormForDiffFile = state => fileHash =>
+  state.commentForms.find(form => form.fileHash === fileHash);
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

app/assets/javascripts/diffs/store/modules/diff_state.js

@@ -24,4 +24,6 @@ export default () => ({
   showTreeList:
     storedTreeShow === null ? bp.getBreakpointSize() !== 'xs' : storedTreeShow === 'true',
   currentDiffFileId: '',
+  projectPath: '',
+  commentForms: [],
 });

app/assets/javascripts/diffs/store/mutation_types.js

@@ -14,3 +14,7 @@ export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FIL
 export const TOGGLE_FOLDER_OPEN = 'TOGGLE_FOLDER_OPEN';
 export const TOGGLE_SHOW_TREE_LIST = 'TOGGLE_SHOW_TREE_LIST';
 export const UPDATE_CURRENT_DIFF_FILE_ID = 'UPDATE_CURRENT_DIFF_FILE_ID';
+
+export const OPEN_DIFF_FILE_COMMENT_FORM = 'OPEN_DIFF_FILE_COMMENT_FORM';
+export const UPDATE_DIFF_FILE_COMMENT_FORM = 'UPDATE_DIFF_FILE_COMMENT_FORM';
+export const CLOSE_DIFF_FILE_COMMENT_FORM = 'CLOSE_DIFF_FILE_COMMENT_FORM';

app/assets/javascripts/diffs/store/mutations.js

@@ -153,20 +153,22 @@ export default {
     });
   },
 
-  [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) {
+  [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode, id }) {
     const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
     if (selectedFile) {
-      const targetLine = selectedFile.parallelDiffLines.find(
-        line =>
-          (line.left && line.left.lineCode === lineCode) ||
-          (line.right && line.right.lineCode === lineCode),
-      );
-      if (targetLine) {
-        const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right';
-
-        Object.assign(targetLine[side], {
-          discussions: [],
-        });
+      if (selectedFile.parallelDiffLines) {
+        const targetLine = selectedFile.parallelDiffLines.find(
+          line =>
+            (line.left && line.left.lineCode === lineCode) ||
+            (line.right && line.right.lineCode === lineCode),
+        );
+        if (targetLine) {
+          const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right';
+
+          Object.assign(targetLine[side], {
+            discussions: [],
+          });
+        }
       }
 
       if (selectedFile.highlightedDiffLines) {
@@ -180,6 +182,12 @@ export default {
           });
         }
       }
+
+      if (selectedFile.discussions && selectedFile.discussions.length) {
+        selectedFile.discussions = selectedFile.discussions.filter(
+          discussion => discussion.id !== id,
+        );
+      }
     }
   },
   [types.TOGGLE_FOLDER_OPEN](state, path) {
@@ -191,4 +199,25 @@ export default {
   [types.UPDATE_CURRENT_DIFF_FILE_ID](state, fileId) {
     state.currentDiffFileId = fileId;
   },
+  [types.OPEN_DIFF_FILE_COMMENT_FORM](state, formData) {
+    state.commentForms.push({
+      ...formData,
+    });
+  },
+  [types.UPDATE_DIFF_FILE_COMMENT_FORM](state, formData) {
+    const { fileHash } = formData;
+
+    state.commentForms = state.commentForms.map(form => {
+      if (form.fileHash === fileHash) {
+        return {
+          ...formData,
+        };
+      }
+
+      return form;
+    });
+  },
+  [types.CLOSE_DIFF_FILE_COMMENT_FORM](state, fileHash) {
+    state.commentForms = state.commentForms.filter(form => form.fileHash !== fileHash);
+  },
 };

app/assets/javascripts/diffs/store/utils.js

 import _ from 'underscore';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+import { diffModes } from '~/ide/constants';
 import {
   LINE_POSITION_LEFT,
   LINE_POSITION_RIGHT,
@@ -34,6 +35,7 @@ export function getFormData(params) {
     noteTargetLine,
     diffViewType,
     linePosition,
+    positionType,
   } = params;
 
   const position = JSON.stringify({
@@ -42,9 +44,11 @@ export function getFormData(params) {
     head_sha: diffFile.diffRefs.headSha,
     old_path: diffFile.oldPath,
     new_path: diffFile.newPath,
-    position_type: TEXT_DIFF_POSITION_TYPE,
-    old_line: noteTargetLine.oldLine,
-    new_line: noteTargetLine.newLine,
+    position_type: positionType || TEXT_DIFF_POSITION_TYPE,
+    old_line: noteTargetLine ? noteTargetLine.oldLine : null,
+    new_line: noteTargetLine ? noteTargetLine.newLine : null,
+    x: params.x,
+    y: params.y,
   });
 
   const postData = {
@@ -66,7 +70,7 @@ export function getFormData(params) {
         diffFile.diffRefs.startSha && diffFile.diffRefs.headSha
           ? DIFF_NOTE_TYPE
           : LEGACY_DIFF_NOTE_TYPE,
-      line_code: noteTargetLine.lineCode,
+      line_code: noteTargetLine ? noteTargetLine.lineCode : null,
     },
   };
 
@@ -225,6 +229,7 @@ export function prepareDiffData(diffData) {
     Object.assign(file, {
       renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
       collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
+      discussions: [],
     });
   }
 }
@@ -320,3 +325,8 @@ export const generateTreeList = files =>
     },
     { treeEntries: {}, tree: [] },
   );
+
+export const getDiffMode = diffFile => {
+  const diffModeKey = Object.keys(diffModes).find(key => diffFile[`${key}File`]);
+  return diffModes[diffModeKey] || diffModes.replaced;
+};

app/assets/javascripts/merge_request_tabs.js

@@ -11,7 +11,6 @@ import bp from './breakpoints';
 import { parseUrlPathname, handleLocationHash, isMetaClick } from './lib/utils/common_utils';
 import { isInVueNoteablePage } from './lib/utils/dom_utils';
 import { getLocationHash } from './lib/utils/url_utility';
-import initDiscussionTab from './image_diff/init_discussion_tab';
 import Diff from './diff';
 import { localTimeAgo } from './lib/utils/datetime_utility';
 import syntaxHighlight from './syntax_highlight';
@@ -207,8 +206,6 @@ export default class MergeRequestTabs {
         }
         this.resetViewContainer();
         this.destroyPipelinesView();
-
-        initDiscussionTab();
       }
       if (this.setUrl) {
         this.setCurrentAction(action);

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

 <script>
 import { mapState, mapActions } from 'vuex';
-import imageDiffHelper from '~/image_diff/helpers/index';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import DiffFileHeader from '~/diffs/components/diff_file_header.vue';
+import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
+import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue';
 import { GlSkeletonLoading } from '@gitlab-org/gitlab-ui';
-import { trimFirstCharOfLineContent } from '~/diffs/store/utils';
+import { trimFirstCharOfLineContent, getDiffMode } from '~/diffs/store/utils';
 
 export default {
   components: {
     DiffFileHeader,
     GlSkeletonLoading,
+    DiffViewer,
+    ImageDiffOverlay,
   },
   props: {
     discussion: {
@@ -25,7 +28,11 @@ export default {
   computed: {
     ...mapState({
       noteableData: state => state.notes.noteableData,
+      projectPath: state => state.diffs.projectPath,
     }),
+    diffMode() {
+      return getDiffMode(this.diffFile);
+    },
     hasTruncatedDiffLines() {
       return this.discussion.truncatedDiffLines && this.discussion.truncatedDiffLines.length !== 0;
     },
@@ -62,11 +69,7 @@ export default {
     },
   },
   mounted() {
-    if (this.isImageDiff) {
-      const canCreateNote = false;
-      const renderCommentBadge = true;
-      imageDiffHelper.initImageDiff(this.$refs.fileHolder, canCreateNote, renderCommentBadge);
-    } else if (!this.hasTruncatedDiffLines) {
+    if (!this.hasTruncatedDiffLines) {
       this.fetchDiff();
     }
   },
@@ -160,7 +163,24 @@ export default {
     <div
       v-else
     >
-      <div v-html="imageDiffHtml"></div>
+      <diff-viewer
+        :diff-mode="diffMode"
+        :new-path="diffFile.newPath"
+        :new-sha="diffFile.diffRefs.headSha"
+        :old-path="diffFile.oldPath"
+        :old-sha="diffFile.diffRefs.baseSha"
+        :file-hash="diffFile.fileHash"
+        :project-path="projectPath"
+      >
+        <image-diff-overlay
+          slot="image-overlay"
+          :discussions="discussion"
+          :file-hash="diffFile.fileHash"
+          :show-comment-icon="true"
+          :should-toggle-discussion="false"
+          badge-class="image-comment-badge"
+        />
+      </diff-viewer>
       <slot></slot>
     </div>
   </div>

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

@@ -350,11 +350,18 @@ Please check your network connection and try again.`;
                 <ul class="notes">
                   <component
                     :is="componentName(note)"
-                    v-for="note in discussion.notes"
+                    v-for="(note, index) in discussion.notes"
                     :key="note.id"
                     :note="componentData(note)"
                     @handleDeleteNote="deleteNoteHandler"
-                  />
+                  >
+                    <slot
+                      v-if="index === 0"
+                      slot="avatar-badge"
+                      name="avatar-badge"
+                    >
+                    </slot>
+                  </component>
                 </ul>
                 <div
                   :class="{ 'is-replying': isReplying }"

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

@@ -182,7 +182,13 @@ export default {
           :img-src="author.avatar_url"
           :img-alt="author.name"
           :img-size="40"
-        />
+        >
+          <slot
+            slot="avatar-badge"
+            name="avatar-badge"
+          >
+          </slot>
+        </user-avatar-link>
       </div>
       <div class="timeline-content">
         <div class="note-header">

app/assets/javascripts/vue_shared/components/content_viewer/viewers/image_viewer.vue

@@ -17,19 +17,37 @@ export default {
       type: Boolean,
       default: true,
     },
+    innerCssClasses: {
+      type: [Array, Object, String],
+      required: false,
+      default: '',
+    },
   },
   data() {
     return {
       width: 0,
       height: 0,
-      isZoomable: false,
-      isZoomed: false,
+      isLoaded: false,
     };
   },
   computed: {
     fileSizeReadable() {
       return numberToHumanSize(this.fileSize);
     },
+    dimensionStyles() {
+      if (!this.isLoaded) return {};
+
+      return {
+        width: `${this.width}px`,
+        height: `${this.height}px`,
+      };
+    },
+    hasFileSize() {
+      return this.fileSize > 0;
+    },
+    hasDimensions() {
+      return this.width && this.height;
+    },
   },
   beforeDestroy() {
     window.removeEventListener('resize', this.resizeThrottled, false);
@@ -48,51 +66,52 @@ export default {
       const { contentImg } = this.$refs;
 
       if (contentImg) {
-        this.isZoomable =
-          contentImg.naturalWidth > contentImg.width ||
-          contentImg.naturalHeight > contentImg.height;
-
         this.width = contentImg.naturalWidth;
         this.height = contentImg.naturalHeight;
 
-        this.$emit('imgLoaded', {
-          width: this.width,
-          height: this.height,
-          renderedWidth: contentImg.clientWidth,
-          renderedHeight: contentImg.clientHeight,
+        this.$nextTick(() => {
+          this.isLoaded = true;
+
+          this.$emit('imgLoaded', {
+            width: this.width,
+            height: this.height,
+            renderedWidth: contentImg.clientWidth,
+            renderedHeight: contentImg.clientHeight,
+          });
         });
       }
     },
-    onImgClick() {
-      if (this.isZoomable) this.isZoomed = !this.isZoomed;
-    },
   },
 };
 </script>
 
 <template>
-  <div class="file-container">
-    <div class="file-content image_file">
+  <div>
+    <div
+      :class="innerCssClasses"
+      :style="dimensionStyles"
+      class="position-relative"
+    >
       <img
         ref="contentImg"
-        :class="{ 'is-zoomable': isZoomable, 'is-zoomed': isZoomed }"
         :src="path"
-        :alt="path"
         @load="onImgLoad"
-        @click="onImgClick"/>
-      <p
-        v-if="renderInfo"
-        class="file-info prepend-top-10">
-        <template v-if="fileSize>0">
-          {{ fileSizeReadable }}
-        </template>
-        <template v-if="fileSize>0 && width && height">
-          |
-        </template>
-        <template v-if="width && height">
-          W: {{ width }} | H: {{ height }}
-        </template>
-      </p>
+      />
+      <slot name="image-overlay"></slot>
     </div>
+    <p
+      v-if="renderInfo"
+      class="image-info"
+    >
+      <template v-if="hasFileSize">
+        {{ fileSizeReadable }}
+      </template>
+      <template v-if="hasFileSize && hasDimensions">
+        |
+      </template>
+      <template v-if="hasDimensions">
+        <strong>W</strong>: {{ width }} | <strong>H</strong>: {{ height }}
+      </template>
+    </p>
   </div>
 </template>

app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue

@@ -69,6 +69,13 @@ export default {
       :new-path="fullNewPath"
       :old-path="fullOldPath"
       :project-path="projectPath"
-    />
+    >
+      <slot
+        slot="image-overlay"
+        name="image-overlay"
+      >
+      </slot>
+    </component>
+    <slot></slot>
   </div>
 </template>

app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/onion_skin_viewer.vue

@@ -15,11 +15,6 @@ export default {
       type: String,
       required: true,
     },
-    projectPath: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
   data() {
     return {
@@ -120,7 +115,6 @@ export default {
           key="onionOldImg"
           :render-info="false"
           :path="oldPath"
-          :project-path="projectPath"
           @imgLoaded="onionOldImgLoaded"
         />
       </div>
@@ -136,9 +130,14 @@ export default {
           key="onionNewImg"
           :render-info="false"
           :path="newPath"
-          :project-path="projectPath"
           @imgLoaded="onionNewImgLoaded"
-        />
+        >
+          <slot
+            slot="image-overlay"
+            name="image-overlay"
+          >
+          </slot>
+        </image-viewer>
       </div>
       <div class="controls">
         <div class="transparent"></div>

app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue

@@ -16,11 +16,6 @@ export default {
       type: String,
       required: true,
     },
-    projectPath: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
   data() {
     return {
@@ -117,16 +112,14 @@ export default {
         'height': swipeMaxPixelHeight,
       }"
       class="swipe-frame">
-      <div class="frame deleted">
-        <image-viewer
-          key="swipeOldImg"
-          ref="swipeOldImg"
-          :render-info="false"
-          :path="oldPath"
-          :project-path="projectPath"
-          @imgLoaded="swipeOldImgLoaded"
-        />
-      </div>
+      <image-viewer
+        key="swipeOldImg"
+        ref="swipeOldImg"
+        :render-info="false"
+        :path="oldPath"
+        class="frame deleted"
+        @imgLoaded="swipeOldImgLoaded"
+      />
       <div
         ref="swipeWrap"
         :style="{
@@ -134,15 +127,19 @@ export default {
           'height': swipeMaxPixelHeight,
         }"
         class="swipe-wrap">
-        <div class="frame added">
-          <image-viewer
-            key="swipeNewImg"
-            :render-info="false"
-            :path="newPath"
-            :project-path="projectPath"
-            @imgLoaded="swipeNewImgLoaded"
-          />
-        </div>
+        <image-viewer
+          key="swipeNewImg"
+          :render-info="false"
+          :path="newPath"
+          class="frame added"
+          @imgLoaded="swipeNewImgLoaded"
+        >
+          <slot
+            slot="image-overlay"
+            name="image-overlay"
+          >
+          </slot>
+        </image-viewer>
       </div>
       <span
         ref="swipeBar"

app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/two_up_viewer.vue

@@ -14,28 +14,29 @@ export default {
       type: String,
       required: true,
     },
-    projectPath: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
 };
 </script>
 
 <template>
-  <div class="two-up view row">
-    <div class="col-sm-6 frame deleted">
-      <image-viewer
-        :path="oldPath"
-        :project-path="projectPath"
-      />
-    </div>
-    <div class="col-sm-6 frame added">
-      <image-viewer
-        :path="newPath"
-        :project-path="projectPath"
-      />
-    </div>
+  <div class="two-up view">
+    <image-viewer
+      :path="oldPath"
+      :render-info="true"
+      inner-css-classes="frame deleted"
+      class="wrap"
+    />
+    <image-viewer
+      :path="newPath"
+      :render-info="true"
+      :inner-css-classes="['frame', 'added']"
+      class="wrap"
+    >
+      <slot
+        slot="image-overlay"
+        name="image-overlay"
+      >
+      </slot>
+    </image-viewer>
   </div>
 </template>

app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer.vue

@@ -8,9 +8,6 @@ import { diffModes, imageViewMode } from '../constants';
 export default {
   components: {
     ImageViewer,
-    TwoUpViewer,
-    SwipeViewer,
-    OnionSkinViewer,
   },
   props: {
     diffMode: {
@@ -25,17 +22,32 @@ export default {
       type: String,
       required: true,
     },
-    projectPath: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
   data() {
     return {
       mode: imageViewMode.twoup,
     };
   },
+  computed: {
+    imageViewComponent() {
+      switch (this.mode) {
+        case imageViewMode.twoup:
+          return TwoUpViewer;
+        case imageViewMode.swipe:
+          return SwipeViewer;
+        case imageViewMode.onion:
+          return OnionSkinViewer;
+        default:
+          return undefined;
+      }
+    },
+    isNew() {
+      return this.diffMode === diffModes.new;
+    },
+    imagePath() {
+      return this.isNew ? this.newPath : this.oldPath;
+    },
+  },
   methods: {
     changeMode(newMode) {
       this.mode = newMode;
@@ -52,15 +64,16 @@ export default {
       v-if="diffMode === $options.diffModes.replaced"
       class="diff-viewer">
       <div class="image js-replaced-image">
-        <two-up-viewer
-          v-if="mode === $options.imageViewMode.twoup"
-          v-bind="$props"/>
-        <swipe-viewer
-          v-else-if="mode === $options.imageViewMode.swipe"
-          v-bind="$props"/>
-        <onion-skin-viewer
-          v-else-if="mode === $options.imageViewMode.onion"
-          v-bind="$props"/>
+        <component
+          :is="imageViewComponent"
+          v-bind="$props"
+        >
+          <slot
+            slot="image-overlay"
+            name="image-overlay"
+          >
+          </slot>
+        </component>
       </div>
       <div class="view-modes">
         <ul class="view-modes-menu">
@@ -87,23 +100,27 @@ export default {
           </li>
         </ul>
       </div>
-      <div class="note-container"></div>
-    </div>
-    <div
-      v-else-if="diffMode === $options.diffModes.new"
-      class="diff-viewer added">
-      <image-viewer
-        :path="newPath"
-        :project-path="projectPath"
-      />
     </div>
     <div
       v-else
-      class="diff-viewer deleted">
-      <image-viewer
-        :path="oldPath"
-        :project-path="projectPath"
-      />
+      class="diff-viewer"
+    >
+      <div class="image">
+        <image-viewer
+          :path="imagePath"
+          :inner-css-classes="['frame', {
+            'added': isNew,
+            'deleted': diffMode === $options.diffModes.deleted
+          }]"
+        >
+          <slot
+            v-if="isNew"
+            slot="image-overlay"
+            name="image-overlay"
+          >
+          </slot>
+        </image-viewer>
+      </div>
     </div>
   </div>
 </template>

app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue

@@ -100,5 +100,6 @@ export default {
       :title="tooltipText"
       :tooltip-placement="tooltipPlacement"
     >{{ username }}</span>
+    <slot name="avatar-badge"></slot>
   </gl-link>
 </template>

app/assets/stylesheets/pages/diff.scss

@@ -421,21 +421,13 @@
 
   .diff-file-container {
     .frame.deleted {
-      border: 0;
+      border: 1px solid $deleted;
       background-color: inherit;
-
-      .image_file img {
-        border: 1px solid $deleted;
-      }
     }
 
     .frame.added {
-      border: 0;
+      border: 1px solid $added;
       background-color: inherit;
-
-      .image_file img {
-        border: 1px solid $added;
-      }
     }
 
     .swipe.view,
@@ -481,6 +473,11 @@
       bottom: -25px;
     }
   }
+
+  .discussion-notes .discussion-notes {
+    margin-left: 0;
+    border-left: 0;
+  }
 }
 
 .file-content .diff-file {
@@ -804,7 +801,7 @@
 
   // double jagged line divider
   .discussion-notes + .discussion-notes::before,
-  .discussion-notes + .discussion-form::before {
+  .diff-file-discussions + .discussion-form::before {
     content: '';
     position: relative;
     display: block;
@@ -844,6 +841,13 @@
     background-repeat: repeat;
   }
 
+  .diff-file-discussions + .discussion-form::before {
+    width: auto;
+    margin-left: -16px;
+    margin-right: -16px;
+    margin-bottom: 16px;
+  }
+
   .notes {
     position: relative;
   }
@@ -870,11 +874,13 @@
   }
 }
 
-.files:not([data-can-create-note]) .frame {
+.files:not([data-can-create-note="true"]) .frame {
   cursor: auto;
 }
 
-.frame.click-to-comment {
+.frame,
+.frame.click-to-comment,
+.btn-transparent.image-diff-overlay-add-comment {
   position: relative;
   cursor: image-url('illustrations/image_comment_light_cursor.svg')
       $image-comment-cursor-left-offset $image-comment-cursor-top-offset,
@@ -910,6 +916,7 @@
 
 .frame .badge.badge-pill,
 .image-diff-avatar-link .badge.badge-pill,
+.user-avatar-link .badge.badge-pill,
 .notes > .badge.badge-pill {
   position: absolute;
   background-color: $blue-400;
@@ -944,7 +951,8 @@
   }
 }
 
-.image-diff-avatar-link {
+.image-diff-avatar-link,
+.user-avatar-link {
   position: relative;
 
   .badge.badge-pill,
@@ -1073,3 +1081,14 @@
     top: 0;
   }
 }
+
+.image-diff-overlay,
+.image-diff-overlay-add-comment {
+  top: 0;
+  left: 0;
+
+  &:active,
+  &:focus {
+    outline: 0;
+  }
+}

changelogs/unreleased/mr-image-commenting.yml

+---
+title: Reimplemented image commenting in merge request diffs
+merge_request:
+author:
+type: added

lib/gitlab/diff/formatters/image_formatter.rb

@@ -21,7 +21,7 @@ module Gitlab
         end
 
         def complete?
-          x && y && width && height
+          x && y
         end
 
         def to_h

locale/gitlab.pot

@@ -336,6 +336,9 @@ msgstr ""
 msgid "Add a table"
 msgstr ""
 
+msgid "Add image comment"
+msgstr ""
+
 msgid "Add license"
 msgstr ""
 
@@ -1721,12 +1724,18 @@ msgstr ""
 msgid "Collapse sidebar"
 msgstr ""
 
+msgid "Comment"
+msgstr ""
+
 msgid "Comment & resolve discussion"
 msgstr ""
 
 msgid "Comment & unresolve discussion"
 msgstr ""
 
+msgid "Comment form position"
+msgstr ""
+
 msgid "Comments"
 msgstr ""
 

spec/features/merge_request/user_creates_image_diff_notes_spec.rb

@@ -114,10 +114,9 @@ describe 'Merge request > User creates image diff notes', :js do
           create_image_diff_note
         end
 
-        # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034
-        xit 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes' do
+        it 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes' do
           indicator = find('.js-image-badge', match: :first)
-          badge = find('.image-diff-avatar-link .badge', match: :first)
+          badge = find('.user-avatar-link .badge', match: :first)
 
           expect(indicator).to have_content('1')
           expect(badge).to have_content('1')
@@ -157,8 +156,7 @@ describe 'Merge request > User creates image diff notes', :js do
       visit project_merge_request_path(project, merge_request)
     end
 
-    # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034
-    xit 'render diff indicators within the image frame' do
+    it 'render diff indicators within the image frame' do
       diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position)
 
       wait_for_requests
@@ -200,7 +198,6 @@ describe 'Merge request > User creates image diff notes', :js do
 
   def create_image_diff_note
     find('.js-add-image-diff-note-button', match: :first).click
-    page.all('.js-add-image-diff-note-button')[0].click
     find('.diff-content .note-textarea').native.send_keys('image diff test comment')
     click_button 'Comment'
     wait_for_requests

spec/javascripts/diffs/components/diff_content_spec.js

 import Vue from 'vue';
 import DiffContentComponent from '~/diffs/components/diff_content.vue';
-import store from '~/mr_notes/stores';
+import { createStore } from '~/mr_notes/stores';
 import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
 import { GREEN_BOX_IMAGE_URL, RED_BOX_IMAGE_URL } from 'spec/test_constants';
+import '~/behaviors/markdown/render_gfm';
 import diffFileMockData from '../mock_data/diff_file';
+import discussionsMockData from '../mock_data/diff_discussions';
 
 describe('DiffContent', () => {
   const Component = Vue.extend(DiffContentComponent);
   let vm;
 
   beforeEach(() => {
+    const store = createStore();
+    store.state.notes.noteableData = {
+      current_user: {
+        can_create_note: false,
+      },
+    };
+
     vm = mountComponentWithStore(Component, {
       store,
       props: {
@@ -46,21 +55,49 @@ describe('DiffContent', () => {
     });
 
     describe('image diff', () => {
-      beforeEach(() => {
+      beforeEach(done => {
         vm.diffFile.newPath = GREEN_BOX_IMAGE_URL;
         vm.diffFile.newSha = 'DEF';
         vm.diffFile.oldPath = RED_BOX_IMAGE_URL;
         vm.diffFile.oldSha = 'ABC';
         vm.diffFile.viewPath = '';
+        vm.diffFile.discussions = [{ ...discussionsMockData }];
+        vm.$store.state.diffs.commentForms.push({ fileHash: vm.diffFile.fileHash, x: 10, y: 20 });
+
+        vm.$nextTick(done);
       });
 
-      it('should have image diff view in place', done => {
-        vm.$nextTick(() => {
-          expect(vm.$el.querySelectorAll('.js-diff-inline-view').length).toEqual(0);
+      it('should have image diff view in place', () => {
+        expect(vm.$el.querySelectorAll('.js-diff-inline-view').length).toEqual(0);
+
+        expect(vm.$el.querySelectorAll('.diff-viewer .image').length).toEqual(1);
+      });
 
-          expect(vm.$el.querySelectorAll('.diff-viewer .image').length).toEqual(1);
+      it('renders image diff overlay', () => {
+        expect(vm.$el.querySelector('.image-diff-overlay')).not.toBe(null);
+      });
 
-          done();
+      it('renders diff file discussions', () => {
+        expect(vm.$el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5);
+      });
+
+      describe('handleSaveNote', () => {
+        it('dispatches handleSaveNote', () => {
+          spyOn(vm.$store, 'dispatch').and.stub();
+
+          vm.handleSaveNote('test');
+
+          expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/saveDiffDiscussion', {
+            note: 'test',
+            formData: {
+              noteableData: jasmine.anything(),
+              noteableType: jasmine.anything(),
+              diffFile: vm.diffFile,
+              positionType: 'image',
+              x: 10,
+              y: 20,
+            },
+          });
         });
       });
     });

spec/javascripts/diffs/components/diff_discussions_spec.js

 import Vue from 'vue';
 import DiffDiscussions from '~/diffs/components/diff_discussions.vue';
-import store from '~/mr_notes/stores';
+import { createStore } from '~/mr_notes/stores';
 import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
+import '~/behaviors/markdown/render_gfm';
 import discussionsMockData from '../mock_data/diff_discussions';
 
 describe('DiffDiscussions', () => {
-  let component;
+  let vm;
   const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
 
-  beforeEach(() => {
-    component = createComponentWithStore(Vue.extend(DiffDiscussions), store, {
+  function createComponent(props = {}) {
+    const store = createStore();
+
+    vm = createComponentWithStore(Vue.extend(DiffDiscussions), store, {
       discussions: getDiscussionsMockData(),
+      ...props,
     }).$mount();
+  }
+
+  afterEach(() => {
+    vm.$destroy();
   });
 
   describe('template', () => {
     it('should have notes list', () => {
-      const { $el } = component;
+      createComponent();
+
+      expect(vm.$el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5);
+    });
+  });
+
+  describe('image commenting', () => {
+    it('renders collapsible discussion button', () => {
+      createComponent({ shouldCollapseDiscussions: true });
+
+      expect(vm.$el.querySelector('.js-diff-notes-toggle')).not.toBe(null);
+      expect(vm.$el.querySelector('.js-diff-notes-toggle svg')).not.toBe(null);
+      expect(vm.$el.querySelector('.js-diff-notes-toggle').classList).toContain(
+        'diff-notes-collapse',
+      );
+    });
+
+    it('dispatches toggleDiscussion when clicking collapse button', () => {
+      createComponent({ shouldCollapseDiscussions: true });
+
+      spyOn(vm.$store, 'dispatch').and.stub();
+
+      vm.$el.querySelector('.js-diff-notes-toggle').click();
+
+      expect(vm.$store.dispatch).toHaveBeenCalledWith('toggleDiscussion', {
+        discussionId: vm.discussions[0].id,
+      });
+    });
+
+    it('renders expand button when discussion is collapsed', done => {
+      createComponent({ shouldCollapseDiscussions: true });
+
+      vm.discussions[0].expanded = false;
+
+      vm.$nextTick(() => {
+        expect(vm.$el.querySelector('.js-diff-notes-toggle').textContent.trim()).toBe('1');
+        expect(vm.$el.querySelector('.js-diff-notes-toggle').className).toContain(
+          'btn-transparent badge badge-pill',
+        );
+
+        done();
+      });
+    });
+
+    it('hides discussion when collapsed', done => {
+      createComponent({ shouldCollapseDiscussions: true });
+
+      vm.discussions[0].expanded = false;
+
+      vm.$nextTick(() => {
+        expect(vm.$el.querySelector('.note-discussion').style.display).toBe('none');
+
+        done();
+      });
+    });
+
+    it('renders badge on avatar', () => {
+      createComponent({ renderAvatarBadge: true, discussions: [{ ...discussionsMockData }] });
 
-      expect($el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5);
+      expect(vm.$el.querySelector('.user-avatar-link .badge-pill')).not.toBe(null);
+      expect(vm.$el.querySelector('.user-avatar-link .badge-pill').textContent.trim()).toBe('1');
     });
   });
 });

spec/javascripts/diffs/components/image_diff_overlay_spec.js

+import Vue from 'vue';
+import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue';
+import { createStore } from '~/mr_notes/stores';
+import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
+import { imageDiffDiscussions } from '../mock_data/diff_discussions';
+
+describe('Diffs image diff overlay component', () => {
+  let Component;
+  let vm;
+
+  function createComponent(props = {}, extendStore = () => {}) {
+    const store = createStore();
+
+    extendStore(store);
+
+    vm = mountComponentWithStore(Component, {
+      store,
+      props: { discussions: [...imageDiffDiscussions], fileHash: 'ABC', ...props },
+    });
+  }
+
+  beforeAll(() => {
+    Component = Vue.extend(ImageDiffOverlay);
+  });
+
+  afterEach(() => {
+    vm.$destroy();
+  });
+
+  it('renders comment badges', () => {
+    createComponent();
+
+    expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(2);
+  });
+
+  it('renders index of discussion in badge', () => {
+    createComponent();
+
+    expect(vm.$el.querySelectorAll('.js-image-badge')[0].textContent.trim()).toBe('1');
+    expect(vm.$el.querySelectorAll('.js-image-badge')[1].textContent.trim()).toBe('2');
+  });
+
+  it('renders icon when showCommentIcon is true', () => {
+    createComponent({ showCommentIcon: true });
+
+    expect(vm.$el.querySelector('.js-image-badge svg')).not.toBe(null);
+  });
+
+  it('sets badge comment positions', () => {
+    createComponent();
+
+    expect(vm.$el.querySelectorAll('.js-image-badge')[0].style.left).toBe('10px');
+    expect(vm.$el.querySelectorAll('.js-image-badge')[0].style.top).toBe('10px');
+
+    expect(vm.$el.querySelectorAll('.js-image-badge')[1].style.left).toBe('5px');
+    expect(vm.$el.querySelectorAll('.js-image-badge')[1].style.top).toBe('5px');
+  });
+
+  it('renders single badge for discussion object', () => {
+    createComponent({
+      discussions: {
+        ...imageDiffDiscussions[0],
+      },
+    });
+
+    expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(1);
+  });
+
+  it('dispatches openDiffFileCommentForm when clcking overlay', () => {
+    createComponent({ canComment: true });
+
+    spyOn(vm.$store, 'dispatch').and.stub();
+
+    vm.$el.querySelector('.js-add-image-diff-note-button').click();
+
+    expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/openDiffFileCommentForm', {
+      fileHash: 'ABC',
+      x: 0,
+      y: 0,
+    });
+  });
+
+  describe('toggle discussion', () => {
+    it('disables buttons when shouldToggleDiscussion is false', () => {
+      createComponent({ shouldToggleDiscussion: false });
+
+      expect(vm.$el.querySelector('.js-image-badge').hasAttribute('disabled')).toBe(true);
+    });
+
+    it('dispatches toggleDiscussion when clicking image badge', () => {
+      createComponent();
+
+      spyOn(vm.$store, 'dispatch').and.stub();
+
+      vm.$el.querySelector('.js-image-badge').click();
+
+      expect(vm.$store.dispatch).toHaveBeenCalledWith('toggleDiscussion', { discussionId: '1' });
+    });
+  });
+
+  describe('comment form', () => {
+    beforeEach(() => {
+      createComponent({}, store => {
+        store.state.diffs.commentForms.push({
+          fileHash: 'ABC',
+          x: 20,
+          y: 10,
+        });
+      });
+    });
+
+    it('renders comment form badge', () => {
+      expect(vm.$el.querySelector('.comment-indicator')).not.toBe(null);
+    });
+
+    it('sets comment form badge position', () => {
+      expect(vm.$el.querySelector('.comment-indicator').style.left).toBe('20px');
+      expect(vm.$el.querySelector('.comment-indicator').style.top).toBe('10px');
+    });
+  });
+});

spec/javascripts/diffs/mock_data/diff_discussions.js

@@ -492,3 +492,20 @@ export default {
   image_diff_html:
     '<div class="image js-replaced-image" data="">\n<div class="two-up view">\n<div class="wrap">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n<div class="wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{&quot;base_sha&quot;:&quot;e63f41fe459e62e1228fcef60d7189127aeba95a&quot;,&quot;start_sha&quot;:&quot;d9eaefe5a676b820c57ff18cf5b68316025f7962&quot;,&quot;head_sha&quot;:&quot;c48ee0d1bf3b30453f5b32250ce03134beaa6d13&quot;,&quot;old_path&quot;:&quot;CHANGELOG&quot;,&quot;new_path&quot;:&quot;CHANGELOG&quot;,&quot;position_type&quot;:&quot;text&quot;,&quot;old_line&quot;:null,&quot;new_line&quot;:2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n</div>\n<div class="swipe view hide">\n<div class="swipe-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="swipe-wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{&quot;base_sha&quot;:&quot;e63f41fe459e62e1228fcef60d7189127aeba95a&quot;,&quot;start_sha&quot;:&quot;d9eaefe5a676b820c57ff18cf5b68316025f7962&quot;,&quot;head_sha&quot;:&quot;c48ee0d1bf3b30453f5b32250ce03134beaa6d13&quot;,&quot;old_path&quot;:&quot;CHANGELOG&quot;,&quot;new_path&quot;:&quot;CHANGELOG&quot;,&quot;position_type&quot;:&quot;text&quot;,&quot;old_line&quot;:null,&quot;new_line&quot;:2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n</div>\n<span class="swipe-bar">\n<span class="top-handle"></span>\n<span class="bottom-handle"></span>\n</span>\n</div>\n</div>\n<div class="onion-skin view hide">\n<div class="onion-skin-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{&quot;base_sha&quot;:&quot;e63f41fe459e62e1228fcef60d7189127aeba95a&quot;,&quot;start_sha&quot;:&quot;d9eaefe5a676b820c57ff18cf5b68316025f7962&quot;,&quot;head_sha&quot;:&quot;c48ee0d1bf3b30453f5b32250ce03134beaa6d13&quot;,&quot;old_path&quot;:&quot;CHANGELOG&quot;,&quot;new_path&quot;:&quot;CHANGELOG&quot;,&quot;position_type&quot;:&quot;text&quot;,&quot;old_line&quot;:null,&quot;new_line&quot;:2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<div class="controls">\n<div class="transparent"></div>\n<div class="drag-track">\n<div class="dragger" style="left: 0px;"></div>\n</div>\n<div class="opaque"></div>\n</div>\n</div>\n</div>\n</div>\n<div class="view-modes hide">\n<ul class="view-modes-menu">\n<li class="two-up" data-mode="two-up">2-up</li>\n<li class="swipe" data-mode="swipe">Swipe</li>\n<li class="onion-skin" data-mode="onion-skin">Onion skin</li>\n</ul>\n</div>\n',
 };
+
+export const imageDiffDiscussions = [
+  {
+    id: '1',
+    position: {
+      x: 10,
+      y: 10,
+    },
+  },
+  {
+    id: '2',
+    position: {
+      x: 5,
+      y: 5,
+    },
+  },
+];

spec/javascripts/diffs/mock_data/diff_file.js

@@ -237,4 +237,5 @@ export default {
       },
     },
   ],
+  discussions: [],
 };

spec/javascripts/diffs/store/actions_spec.js

@@ -218,6 +218,7 @@ describe('DiffsStoreActions', () => {
         ],
       };
       const singleDiscussion = {
+        id: '1',
         fileHash: 'ABC',
         line_code: 'ABC_1_1',
       };
@@ -230,6 +231,7 @@ describe('DiffsStoreActions', () => {
           {
             type: types.REMOVE_LINE_DISCUSSIONS_FOR_FILE,
             payload: {
+              id: '1',
               fileHash: 'ABC',
               lineCode: 'ABC_1_1',
             },

spec/javascripts/notes/components/diff_with_note_spec.js

 import Vue from 'vue';
 import DiffWithNote from '~/notes/components/diff_with_note.vue';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import createStore from '~/notes/stores';
+import { createStore } from '~/mr_notes/stores';
 import { mountComponentWithStore } from 'spec/helpers';
 
 const discussionFixture = 'merge_requests/diff_discussion.json';

spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js

@@ -47,7 +47,7 @@ describe('ContentViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.image_file img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
+      expect(vm.$el.querySelector('img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
 
       done();
     });

spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js

@@ -30,11 +30,11 @@ describe('DiffViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe(
+      expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(
         `//raw/DEF/${RED_BOX_IMAGE_URL}`,
       );
 
-      expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe(
+      expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(
         `//raw/ABC/${GREEN_BOX_IMAGE_URL}`,
       );
 

spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js

@@ -52,13 +52,9 @@ describe('ImageDiffViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe(
-        GREEN_BOX_IMAGE_URL,
-      );
+      expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
 
-      expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe(
-        RED_BOX_IMAGE_URL,
-      );
+      expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(RED_BOX_IMAGE_URL);
 
       expect(vm.$el.querySelector('.view-modes-menu li.active').textContent.trim()).toBe('2-up');
       expect(vm.$el.querySelector('.view-modes-menu li:nth-child(2)').textContent.trim()).toBe(
@@ -81,9 +77,7 @@ describe('ImageDiffViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe(
-        GREEN_BOX_IMAGE_URL,
-      );
+      expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
 
       done();
     });
@@ -97,9 +91,7 @@ describe('ImageDiffViewer', () => {
     });
 
     setTimeout(() => {
-      expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe(
-        RED_BOX_IMAGE_URL,
-      );
+      expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(RED_BOX_IMAGE_URL);
 
       done();
     });