From f7df9ddb52be8a03b8fbd8c9a870f3e3af577562 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Thu, 18 Oct 2018 08:50:38 +0100
Subject: 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    |   5 +-
 .../javascripts/diffs/components/diff_content.vue  |  70 ++++++++++++-
 .../diffs/components/diff_discussions.vue          |  54 +++++++++-
 .../diffs/components/image_diff_overlay.vue        | 115 +++++++++++++++++++++
 app/assets/javascripts/diffs/constants.js          |   1 +
 app/assets/javascripts/diffs/store/actions.js      |  19 +++-
 app/assets/javascripts/diffs/store/getters.js      |   3 +
 .../javascripts/diffs/store/modules/diff_state.js  |   2 +
 .../javascripts/diffs/store/mutation_types.js      |   4 +
 app/assets/javascripts/diffs/store/mutations.js    |  53 +++++++---
 app/assets/javascripts/diffs/store/utils.js        |  18 +++-
 app/assets/javascripts/merge_request_tabs.js       |   3 -
 .../notes/components/diff_with_note.vue            |  36 +++++--
 .../notes/components/noteable_discussion.vue       |  11 +-
 .../javascripts/notes/components/noteable_note.vue |   8 +-
 .../content_viewer/viewers/image_viewer.vue        |  83 +++++++++------
 .../components/diff_viewer/diff_viewer.vue         |   9 +-
 .../viewers/image_diff/onion_skin_viewer.vue       |  15 ++-
 .../viewers/image_diff/swipe_viewer.vue            |  45 ++++----
 .../viewers/image_diff/two_up_viewer.vue           |  37 +++----
 .../diff_viewer/viewers/image_diff_viewer.vue      |  79 ++++++++------
 .../components/user_avatar/user_avatar_link.vue    |   1 +
 app/assets/stylesheets/pages/diff.scss             |  47 ++++++---
 23 files changed, 549 insertions(+), 169 deletions(-)
 create mode 100644 app/assets/javascripts/diffs/components/image_diff_overlay.vue

(limited to 'app/assets')

diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 59680959bb1..9c0f72f177f 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/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"
diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue
index fb5556e3cd7..f677999a268 100644
--- a/app/assets/javascripts/diffs/components/diff_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_content.vue
@@ -1,15 +1,22 @@
 <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>
diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue
index cddbe554fbd..e19207bdc95 100644
--- a/app/assets/javascripts/diffs/components/diff_discussions.vue
+++ b/app/assets/javascripts/diffs/components/diff_discussions.vue
@@ -1,24 +1,40 @@
 <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>
diff --git a/app/assets/javascripts/diffs/components/image_diff_overlay.vue b/app/assets/javascripts/diffs/components/image_diff_overlay.vue
new file mode 100644
index 00000000000..4b99f38d293
--- /dev/null
+++ b/app/assets/javascripts/diffs/components/image_diff_overlay.vue
@@ -0,0 +1,115 @@
+<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>
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index 6a50d2c1426..78a39baa4cb 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/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';
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index ca8ae605cb4..d3e9c7c88f0 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/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 () => {};
diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js
index d4c205882ff..2bf0ad99c22 100644
--- a/app/assets/javascripts/diffs/store/getters.js
+++ b/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 () => {};
diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js
index 1c5c35071de..085e255f1d3 100644
--- a/app/assets/javascripts/diffs/store/modules/diff_state.js
+++ b/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: [],
 });
diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js
index 6474ee628e2..e011031e72c 100644
--- a/app/assets/javascripts/diffs/store/mutation_types.js
+++ b/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';
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 38a65f111a2..a7eea2c1449 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/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);
+  },
 };
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index a482a2b82c0..178745cffc2 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -1,5 +1,6 @@
 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;
+};
diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
index 2950c2299ab..d8255181574 100644
--- a/app/assets/javascripts/merge_request_tabs.js
+++ b/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);
diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue
index eaa0cded224..b209f736c3f 100644
--- a/app/assets/javascripts/notes/components/diff_with_note.vue
+++ b/app/assets/javascripts/notes/components/diff_with_note.vue
@@ -1,15 +1,18 @@
 <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>
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index c5fdfa1d47c..2d40acbd3dc 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/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 }"
diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue
index f391ed848a4..40222ac4a80 100644
--- a/app/assets/javascripts/notes/components/noteable_note.vue
+++ b/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">
diff --git a/app/assets/javascripts/vue_shared/components/content_viewer/viewers/image_viewer.vue b/app/assets/javascripts/vue_shared/components/content_viewer/viewers/image_viewer.vue
index 8163947cd0c..6f2f0f98690 100644
--- a/app/assets/javascripts/vue_shared/components/content_viewer/viewers/image_viewer.vue
+++ b/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>
diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue
index cfc5343217c..9c3f3e7f7a9 100644
--- a/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue
+++ b/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>
diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/onion_skin_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/onion_skin_viewer.vue
index 38e881d17a2..cd0c1e850af 100644
--- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/onion_skin_viewer.vue
+++ b/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>
diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue
index 86366c799a2..c3cfe54eb4d 100644
--- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue
+++ b/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"
diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/two_up_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/two_up_viewer.vue
index 9c19266ecdf..9806d65e940 100644
--- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/two_up_viewer.vue
+++ b/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>
diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer.vue
index 1af85283277..e68a2aa73fa 100644
--- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer.vue
+++ b/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>
diff --git a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue
index 86c7498a092..ce8efe5b574 100644
--- a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue
+++ b/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>
diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss
index 52c91266ff4..19bc4262e21 100644
--- a/app/assets/stylesheets/pages/diff.scss
+++ b/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;
+  }
+}
-- 
cgit v1.2.3