Commit e1e315ee authored by Filipa Lacerda's avatar Filipa Lacerda

Merge branch 'mr-image-commenting' into 'master'

Re-implemented image commenting on diffs

Closes #48956

See merge request gitlab-org/gitlab-ce!22443
parents 54ba4fdb 0485c047
...@@ -223,7 +223,10 @@ export default { ...@@ -223,7 +223,10 @@ export default {
:commit="commit" :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 <div
v-show="showTreeList" v-show="showTreeList"
class="diff-tree-list" class="diff-tree-list"
......
<script> <script>
import { mapGetters, mapState } from 'vuex'; import { mapActions, mapGetters, mapState } from 'vuex';
import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue'; import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
import { diffModes } from '~/ide/constants';
import InlineDiffView from './inline_diff_view.vue'; import InlineDiffView from './inline_diff_view.vue';
import ParallelDiffView from './parallel_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 { export default {
components: { components: {
InlineDiffView, InlineDiffView,
ParallelDiffView, ParallelDiffView,
DiffViewer, DiffViewer,
NoteForm,
DiffDiscussions,
ImageDiffOverlay,
}, },
props: { props: {
diffFile: { diffFile: {
...@@ -23,13 +30,38 @@ export default { ...@@ -23,13 +30,38 @@ export default {
endpoint: state => state.diffs.endpoint, endpoint: state => state.diffs.endpoint,
}), }),
...mapGetters('diffs', ['isInlineView', 'isParallelView']), ...mapGetters('diffs', ['isInlineView', 'isParallelView']),
...mapGetters('diffs', ['getCommentFormForDiffFile']),
...mapGetters(['getNoteableData', 'noteableType']),
diffMode() { diffMode() {
const diffModeKey = Object.keys(diffModes).find(key => this.diffFile[`${key}File`]); return getDiffMode(this.diffFile);
return diffModes[diffModeKey] || diffModes.replaced;
}, },
isTextFile() { isTextFile() {
return this.diffFile.viewer.name === 'text'; 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,
width: this.diffFileCommentForm.width,
height: this.diffFileCommentForm.height,
},
});
},
}, },
}; };
</script> </script>
...@@ -56,7 +88,37 @@ export default { ...@@ -56,7 +88,37 @@ export default {
:new-sha="diffFile.diffRefs.headSha" :new-sha="diffFile.diffRefs.headSha"
:old-path="diffFile.oldPath" :old-path="diffFile.oldPath"
:old-sha="diffFile.diffRefs.baseSha" :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>
</div> </div>
</template> </template>
<script> <script>
import { mapActions } from 'vuex'; import { mapActions } from 'vuex';
import Icon from '~/vue_shared/components/icon.vue';
import noteableDiscussion from '../../notes/components/noteable_discussion.vue'; import noteableDiscussion from '../../notes/components/noteable_discussion.vue';
export default { export default {
components: { components: {
noteableDiscussion, noteableDiscussion,
Icon,
}, },
props: { props: {
discussions: { discussions: {
type: Array, type: Array,
required: true, required: true,
}, },
shouldCollapseDiscussions: {
type: Boolean,
required: false,
default: false,
},
renderAvatarBadge: {
type: Boolean,
required: false,
default: false,
},
}, },
methods: { methods: {
...mapActions(['toggleDiscussion']),
...mapActions('diffs', ['removeDiscussionsFromDiff']), ...mapActions('diffs', ['removeDiscussionsFromDiff']),
deleteNoteHandler(discussion) { deleteNoteHandler(discussion) {
if (discussion.notes.length <= 1) { if (discussion.notes.length <= 1) {
this.removeDiscussionsFromDiff(discussion); this.removeDiscussionsFromDiff(discussion);
} }
}, },
isExpanded(discussion) {
return this.shouldCollapseDiscussions ? discussion.expanded : true;
},
}, },
}; };
</script> </script>
...@@ -26,22 +42,54 @@ export default { ...@@ -26,22 +42,54 @@ export default {
<template> <template>
<div> <div>
<div <div
v-for="discussion in discussions" v-for="(discussion, index) in discussions"
:key="discussion.id" :key="discussion.id"
class="discussion-notes diff-discussions" :class="{
collapsed: !isExpanded(discussion)
}"
class="discussion-notes diff-discussions position-relative"
> >
<ul <ul
:data-discussion-id="discussion.id" :data-discussion-id="discussion.id"
class="notes" 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 <noteable-discussion
v-show="isExpanded(discussion)"
:discussion="discussion" :discussion="discussion"
:render-header="false" :render-header="false"
:render-diff-file="false" :render-diff-file="false"
:always-expanded="true" :always-expanded="true"
:discussions-by-diff-order="true" :discussions-by-diff-order="true"
@noteDeleted="deleteNoteHandler" @noteDeleted="deleteNoteHandler"
/> >
<span
v-if="renderAvatarBadge"
slot="avatar-badge"
class="badge badge-pill"
>
{{ index + 1 }}
</span>
</noteable-discussion>
</ul> </ul>
</div> </div>
</div> </div>
......
<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']),
getImageDimensions() {
return {
width: this.$parent.width,
height: this.$parent.height,
};
},
getPositionForObject(meta) {
const { x, y, width, height } = meta;
const imageWidth = this.getImageDimensions().width;
const imageHeight = this.getImageDimensions().height;
const widthRatio = imageWidth / width;
const heightRatio = imageHeight / height;
return {
x: Math.round(x * widthRatio),
y: Math.round(y * heightRatio),
};
},
getPosition(discussion) {
const { x, y } = this.getPositionForObject(discussion.position);
return {
left: `${x}px`,
top: `${y}px`,
};
},
clickedImage(x, y) {
const { width, height } = this.getImageDimensions();
this.openDiffFileCommentForm({
fileHash: this.fileHash,
width,
height,
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>
...@@ -12,6 +12,7 @@ export const NOTE_TYPE = 'Note'; ...@@ -12,6 +12,7 @@ export const NOTE_TYPE = 'Note';
export const NEW_LINE_TYPE = 'new'; export const NEW_LINE_TYPE = 'new';
export const OLD_LINE_TYPE = 'old'; export const OLD_LINE_TYPE = 'old';
export const TEXT_DIFF_POSITION_TYPE = 'text'; export const TEXT_DIFF_POSITION_TYPE = 'text';
export const IMAGE_DIFF_POSITION_TYPE = 'image';
export const LINE_POSITION_LEFT = 'left'; export const LINE_POSITION_LEFT = 'left';
export const LINE_POSITION_RIGHT = 'right'; export const LINE_POSITION_RIGHT = 'right';
......
...@@ -50,8 +50,8 @@ export const assignDiscussionsToDiff = ( ...@@ -50,8 +50,8 @@ export const assignDiscussionsToDiff = (
}; };
export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => { export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => {
const { fileHash, line_code } = removeDiscussion; const { fileHash, line_code, id } = removeDiscussion;
commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code }); commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code, id });
}; };
export const startRenderDiffsQueue = ({ state, commit }) => { export const startRenderDiffsQueue = ({ state, commit }) => {
...@@ -189,6 +189,7 @@ export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { ...@@ -189,6 +189,7 @@ export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => {
return dispatch('saveNote', postData, { root: true }) return dispatch('saveNote', postData, { root: true })
.then(result => dispatch('updateDiscussion', result.discussion, { root: true })) .then(result => dispatch('updateDiscussion', result.discussion, { root: true }))
.then(discussion => dispatch('assignDiscussionsToDiff', [discussion])) .then(discussion => dispatch('assignDiscussionsToDiff', [discussion]))
.then(() => dispatch('closeDiffFileCommentForm', formData.diffFile.fileHash))
.catch(() => createFlash(s__('MergeRequests|Saving the comment failed'))); .catch(() => createFlash(s__('MergeRequests|Saving the comment failed')));
}; };
...@@ -210,5 +211,19 @@ export const toggleShowTreeList = ({ commit, state }) => { ...@@ -210,5 +211,19 @@ export const toggleShowTreeList = ({ commit, state }) => {
localStorage.setItem(MR_TREE_SHOW_KEY, state.showTreeList); 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 // prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {}; export default () => {};
...@@ -114,5 +114,8 @@ export const allBlobs = state => Object.values(state.treeEntries).filter(f => f. ...@@ -114,5 +114,8 @@ export const allBlobs = state => Object.values(state.treeEntries).filter(f => f.
export const diffFilesLength = state => state.diffFiles.length; 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 // prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {}; export default () => {};
...@@ -24,4 +24,6 @@ export default () => ({ ...@@ -24,4 +24,6 @@ export default () => ({
showTreeList: showTreeList:
storedTreeShow === null ? bp.getBreakpointSize() !== 'xs' : storedTreeShow === 'true', storedTreeShow === null ? bp.getBreakpointSize() !== 'xs' : storedTreeShow === 'true',
currentDiffFileId: '', currentDiffFileId: '',
projectPath: '',
commentForms: [],
}); });
...@@ -14,3 +14,7 @@ export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FIL ...@@ -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_FOLDER_OPEN = 'TOGGLE_FOLDER_OPEN';
export const TOGGLE_SHOW_TREE_LIST = 'TOGGLE_SHOW_TREE_LIST'; export const TOGGLE_SHOW_TREE_LIST = 'TOGGLE_SHOW_TREE_LIST';
export const UPDATE_CURRENT_DIFF_FILE_ID = 'UPDATE_CURRENT_DIFF_FILE_ID'; 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';
...@@ -153,20 +153,22 @@ export default { ...@@ -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); const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
if (selectedFile) { if (selectedFile) {
const targetLine = selectedFile.parallelDiffLines.find( if (selectedFile.parallelDiffLines) {
line => const targetLine = selectedFile.parallelDiffLines.find(
(line.left && line.left.lineCode === lineCode) || line =>
(line.right && line.right.lineCode === lineCode), (line.left && line.left.lineCode === lineCode) ||
); (line.right && line.right.lineCode === lineCode),
if (targetLine) { );
const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right'; if (targetLine) {
const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right';
Object.assign(targetLine[side], {
discussions: [], Object.assign(targetLine[side], {
}); discussions: [],
});
}
} }
if (selectedFile.highlightedDiffLines) { if (selectedFile.highlightedDiffLines) {
...@@ -180,6 +182,12 @@ export default { ...@@ -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) { [types.TOGGLE_FOLDER_OPEN](state, path) {
...@@ -191,4 +199,25 @@ export default { ...@@ -191,4 +199,25 @@ export default {
[types.UPDATE_CURRENT_DIFF_FILE_ID](state, fileId) { [types.UPDATE_CURRENT_DIFF_FILE_ID](state, fileId) {
state.currentDiffFileId = 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);
},
}; };
import _ from 'underscore'; import _ from 'underscore';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { diffModes } from '~/ide/constants';
import { import {
LINE_POSITION_LEFT, LINE_POSITION_LEFT,
LINE_POSITION_RIGHT, LINE_POSITION_RIGHT,
...@@ -34,6 +35,7 @@ export function getFormData(params) { ...@@ -34,6 +35,7 @@ export function getFormData(params) {
noteTargetLine, noteTargetLine,
diffViewType, diffViewType,
linePosition, linePosition,
positionType,
} = params; } = params;
const position = JSON.stringify({ const position = JSON.stringify({
...@@ -42,9 +44,13 @@ export function getFormData(params) { ...@@ -42,9 +44,13 @@ export function getFormData(params) {
head_sha: diffFile.diffRefs.headSha, head_sha: diffFile.diffRefs.headSha,
old_path: diffFile.oldPath, old_path: diffFile.oldPath,
new_path: diffFile.newPath, new_path: diffFile.newPath,
position_type: TEXT_DIFF_POSITION_TYPE, position_type: positionType || TEXT_DIFF_POSITION_TYPE,
old_line: noteTargetLine.oldLine, old_line: noteTargetLine ? noteTargetLine.oldLine : null,
new_line: noteTargetLine.newLine, new_line: noteTargetLine ? noteTargetLine.newLine : null,
x: params.x,
y: params.y,
width: params.width,
height: params.height,
}); });
const postData = { const postData = {
...@@ -66,7 +72,7 @@ export function getFormData(params) { ...@@ -66,7 +72,7 @@ export function getFormData(params) {
diffFile.diffRefs.startSha && diffFile.diffRefs.headSha diffFile.diffRefs.startSha && diffFile.diffRefs.headSha
? DIFF_NOTE_TYPE ? DIFF_NOTE_TYPE
: LEGACY_DIFF_NOTE_TYPE, : LEGACY_DIFF_NOTE_TYPE,
line_code: noteTargetLine.lineCode, line_code: noteTargetLine ? noteTargetLine.lineCode : null,
}, },
}; };
...@@ -225,6 +231,7 @@ export function prepareDiffData(diffData) { ...@@ -225,6 +231,7 @@ export function prepareDiffData(diffData) {
Object.assign(file, { Object.assign(file, {
renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
discussions: [],
}); });
} }
} }
...@@ -320,3 +327,8 @@ export const generateTreeList = files => ...@@ -320,3 +327,8 @@ export const generateTreeList = files =>
}, },
{ treeEntries: {}, tree: [] }, { treeEntries: {}, tree: [] },
); );
export const getDiffMode = diffFile => {
const diffModeKey = Object.keys(diffModes).find(key => diffFile[`${key}File`]);
return diffModes[diffModeKey] || diffModes.replaced;
};
...@@ -11,7 +11,6 @@ import bp from './breakpoints'; ...@@ -11,7 +11,6 @@ import bp from './breakpoints';
import { parseUrlPathname, handleLocationHash, isMetaClick } from './lib/utils/common_utils'; import { parseUrlPathname, handleLocationHash, isMetaClick } from './lib/utils/common_utils';
import { isInVueNoteablePage } from './lib/utils/dom_utils'; import { isInVueNoteablePage } from './lib/utils/dom_utils';
import { getLocationHash } from './lib/utils/url_utility'; import { getLocationHash } from './lib/utils/url_utility';
import initDiscussionTab from './image_diff/init_discussion_tab';
import Diff from './diff'; import Diff from './diff';
import { localTimeAgo } from './lib/utils/datetime_utility'; import { localTimeAgo } from './lib/utils/datetime_utility';
import syntaxHighlight from './syntax_highlight'; import syntaxHighlight from './syntax_highlight';
...@@ -207,8 +206,6 @@ export default class MergeRequestTabs { ...@@ -207,8 +206,6 @@ export default class MergeRequestTabs {
} }
this.resetViewContainer(); this.resetViewContainer();
this.destroyPipelinesView(); this.destroyPipelinesView();
initDiscussionTab();
} }
if (this.setUrl) { if (this.setUrl) {
this.setCurrentAction(action); this.setCurrentAction(action);
......
<script> <script>
import { mapState, mapActions } from 'vuex'; import { mapState, mapActions } from 'vuex';
import imageDiffHelper from '~/image_diff/helpers/index';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import DiffFileHeader from '~/diffs/components/diff_file_header.vue'; 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 { GlSkeletonLoading } from '@gitlab-org/gitlab-ui';
import { trimFirstCharOfLineContent } from '~/diffs/store/utils'; import { trimFirstCharOfLineContent, getDiffMode } from '~/diffs/store/utils';
export default { export default {
components: { components: {
DiffFileHeader, DiffFileHeader,
GlSkeletonLoading, GlSkeletonLoading,
DiffViewer,
ImageDiffOverlay,
}, },
props: { props: {
discussion: { discussion: {
...@@ -25,7 +28,11 @@ export default { ...@@ -25,7 +28,11 @@ export default {
computed: { computed: {
...mapState({ ...mapState({
noteableData: state => state.notes.noteableData, noteableData: state => state.notes.noteableData,
projectPath: state => state.diffs.projectPath,
}), }),
diffMode() {
return getDiffMode(this.diffFile);
},
hasTruncatedDiffLines() { hasTruncatedDiffLines() {
return this.discussion.truncatedDiffLines && this.discussion.truncatedDiffLines.length !== 0; return this.discussion.truncatedDiffLines && this.discussion.truncatedDiffLines.length !== 0;
}, },
...@@ -62,11 +69,7 @@ export default { ...@@ -62,11 +69,7 @@ export default {
}, },
}, },
mounted() { mounted() {
if (this.isImageDiff) { if (!this.hasTruncatedDiffLines) {
const canCreateNote = false;
const renderCommentBadge = true;
imageDiffHelper.initImageDiff(this.$refs.fileHolder, canCreateNote, renderCommentBadge);
} else if (!this.hasTruncatedDiffLines) {
this.fetchDiff(); this.fetchDiff();
} }
}, },
...@@ -160,7 +163,24 @@ export default { ...@@ -160,7 +163,24 @@ export default {
<div <div
v-else 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> <slot></slot>
</div> </div>
</div> </div>
......
...@@ -350,11 +350,18 @@ Please check your network connection and try again.`; ...@@ -350,11 +350,18 @@ Please check your network connection and try again.`;
<ul class="notes"> <ul class="notes">
<component <component
:is="componentName(note)" :is="componentName(note)"
v-for="note in discussion.notes" v-for="(note, index) in discussion.notes"
:key="note.id" :key="note.id"
:note="componentData(note)" :note="componentData(note)"
@handleDeleteNote="deleteNoteHandler" @handleDeleteNote="deleteNoteHandler"
/> >
<slot
v-if="index === 0"
slot="avatar-badge"
name="avatar-badge"
>
</slot>
</component>
</ul> </ul>
<div <div
:class="{ 'is-replying': isReplying }" :class="{ 'is-replying': isReplying }"
......
...@@ -182,7 +182,13 @@ export default { ...@@ -182,7 +182,13 @@ export default {
:img-src="author.avatar_url" :img-src="author.avatar_url"
:img-alt="author.name" :img-alt="author.name"
:img-size="40" :img-size="40"
/> >
<slot
slot="avatar-badge"
name="avatar-badge"
>
</slot>
</user-avatar-link>
</div> </div>
<div class="timeline-content"> <div class="timeline-content">
<div class="note-header"> <div class="note-header">
......
...@@ -17,19 +17,37 @@ export default { ...@@ -17,19 +17,37 @@ export default {
type: Boolean, type: Boolean,
default: true, default: true,
}, },
innerCssClasses: {
type: [Array, Object, String],
required: false,
default: '',
},
}, },
data() { data() {
return { return {
width: 0, width: 0,
height: 0, height: 0,
isZoomable: false, isLoaded: false,
isZoomed: false,
}; };
}, },
computed: { computed: {
fileSizeReadable() { fileSizeReadable() {
return numberToHumanSize(this.fileSize); 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() { beforeDestroy() {
window.removeEventListener('resize', this.resizeThrottled, false); window.removeEventListener('resize', this.resizeThrottled, false);
...@@ -48,51 +66,52 @@ export default { ...@@ -48,51 +66,52 @@ export default {
const { contentImg } = this.$refs; const { contentImg } = this.$refs;
if (contentImg) { if (contentImg) {
this.isZoomable =
contentImg.naturalWidth > contentImg.width ||
contentImg.naturalHeight > contentImg.height;
this.width = contentImg.naturalWidth; this.width = contentImg.naturalWidth;
this.height = contentImg.naturalHeight; this.height = contentImg.naturalHeight;
this.$emit('imgLoaded', { this.$nextTick(() => {
width: this.width, this.isLoaded = true;
height: this.height,
renderedWidth: contentImg.clientWidth, this.$emit('imgLoaded', {
renderedHeight: contentImg.clientHeight, width: this.width,
height: this.height,
renderedWidth: contentImg.clientWidth,
renderedHeight: contentImg.clientHeight,
});
}); });
} }
}, },
onImgClick() {
if (this.isZoomable) this.isZoomed = !this.isZoomed;
},
}, },
}; };
</script> </script>
<template> <template>
<div class="file-container"> <div>
<div class="file-content image_file"> <div
:class="innerCssClasses"
:style="dimensionStyles"
class="position-relative"
>
<img <img
ref="contentImg" ref="contentImg"
:class="{ 'is-zoomable': isZoomable, 'is-zoomed': isZoomed }"
:src="path" :src="path"
:alt="path"
@load="onImgLoad" @load="onImgLoad"
@click="onImgClick"/> />
<p <slot name="image-overlay"></slot>
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>
</div> </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> </div>
</template> </template>
...@@ -69,6 +69,13 @@ export default { ...@@ -69,6 +69,13 @@ export default {
:new-path="fullNewPath" :new-path="fullNewPath"
:old-path="fullOldPath" :old-path="fullOldPath"
:project-path="projectPath" :project-path="projectPath"
/> >
<slot
slot="image-overlay"
name="image-overlay"
>
</slot>
</component>
<slot></slot>
</div> </div>
</template> </template>
...@@ -15,11 +15,6 @@ export default { ...@@ -15,11 +15,6 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
projectPath: {
type: String,
required: false,
default: '',
},
}, },
data() { data() {
return { return {
...@@ -120,7 +115,6 @@ export default { ...@@ -120,7 +115,6 @@ export default {
key="onionOldImg" key="onionOldImg"
:render-info="false" :render-info="false"
:path="oldPath" :path="oldPath"
:project-path="projectPath"
@imgLoaded="onionOldImgLoaded" @imgLoaded="onionOldImgLoaded"
/> />
</div> </div>
...@@ -136,9 +130,14 @@ export default { ...@@ -136,9 +130,14 @@ export default {
key="onionNewImg" key="onionNewImg"
:render-info="false" :render-info="false"
:path="newPath" :path="newPath"
:project-path="projectPath"
@imgLoaded="onionNewImgLoaded" @imgLoaded="onionNewImgLoaded"
/> >
<slot
slot="image-overlay"
name="image-overlay"
>
</slot>
</image-viewer>
</div> </div>
<div class="controls"> <div class="controls">
<div class="transparent"></div> <div class="transparent"></div>
......
...@@ -16,11 +16,6 @@ export default { ...@@ -16,11 +16,6 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
projectPath: {
type: String,
required: false,
default: '',
},
}, },
data() { data() {
return { return {
...@@ -117,16 +112,14 @@ export default { ...@@ -117,16 +112,14 @@ export default {
'height': swipeMaxPixelHeight, 'height': swipeMaxPixelHeight,
}" }"
class="swipe-frame"> class="swipe-frame">
<div class="frame deleted"> <image-viewer
<image-viewer key="swipeOldImg"
key="swipeOldImg" ref="swipeOldImg"
ref="swipeOldImg" :render-info="false"
:render-info="false" :path="oldPath"
:path="oldPath" class="frame deleted"
:project-path="projectPath" @imgLoaded="swipeOldImgLoaded"
@imgLoaded="swipeOldImgLoaded" />
/>
</div>
<div <div
ref="swipeWrap" ref="swipeWrap"
:style="{ :style="{
...@@ -134,15 +127,19 @@ export default { ...@@ -134,15 +127,19 @@ export default {
'height': swipeMaxPixelHeight, 'height': swipeMaxPixelHeight,
}" }"
class="swipe-wrap"> class="swipe-wrap">
<div class="frame added"> <image-viewer
<image-viewer key="swipeNewImg"
key="swipeNewImg" :render-info="false"
:render-info="false" :path="newPath"
:path="newPath" class="frame added"
:project-path="projectPath" @imgLoaded="swipeNewImgLoaded"
@imgLoaded="swipeNewImgLoaded" >
/> <slot
</div> slot="image-overlay"
name="image-overlay"
>
</slot>
</image-viewer>
</div> </div>
<span <span
ref="swipeBar" ref="swipeBar"
......
...@@ -14,28 +14,29 @@ export default { ...@@ -14,28 +14,29 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
projectPath: {
type: String,
required: false,
default: '',
},
}, },
}; };
</script> </script>
<template> <template>
<div class="two-up view row"> <div class="two-up view">
<div class="col-sm-6 frame deleted"> <image-viewer
<image-viewer :path="oldPath"
:path="oldPath" :render-info="true"
:project-path="projectPath" inner-css-classes="frame deleted"
/> class="wrap"
</div> />
<div class="col-sm-6 frame added"> <image-viewer
<image-viewer :path="newPath"
:path="newPath" :render-info="true"
:project-path="projectPath" :inner-css-classes="['frame', 'added']"
/> class="wrap"
</div> >
<slot
slot="image-overlay"
name="image-overlay"
>
</slot>
</image-viewer>
</div> </div>
</template> </template>
...@@ -8,9 +8,6 @@ import { diffModes, imageViewMode } from '../constants'; ...@@ -8,9 +8,6 @@ import { diffModes, imageViewMode } from '../constants';
export default { export default {
components: { components: {
ImageViewer, ImageViewer,
TwoUpViewer,
SwipeViewer,
OnionSkinViewer,
}, },
props: { props: {
diffMode: { diffMode: {
...@@ -25,17 +22,32 @@ export default { ...@@ -25,17 +22,32 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
projectPath: {
type: String,
required: false,
default: '',
},
}, },
data() { data() {
return { return {
mode: imageViewMode.twoup, 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: { methods: {
changeMode(newMode) { changeMode(newMode) {
this.mode = newMode; this.mode = newMode;
...@@ -52,15 +64,16 @@ export default { ...@@ -52,15 +64,16 @@ export default {
v-if="diffMode === $options.diffModes.replaced" v-if="diffMode === $options.diffModes.replaced"
class="diff-viewer"> class="diff-viewer">
<div class="image js-replaced-image"> <div class="image js-replaced-image">
<two-up-viewer <component
v-if="mode === $options.imageViewMode.twoup" :is="imageViewComponent"
v-bind="$props"/> v-bind="$props"
<swipe-viewer >
v-else-if="mode === $options.imageViewMode.swipe" <slot
v-bind="$props"/> slot="image-overlay"
<onion-skin-viewer name="image-overlay"
v-else-if="mode === $options.imageViewMode.onion" >
v-bind="$props"/> </slot>
</component>
</div> </div>
<div class="view-modes"> <div class="view-modes">
<ul class="view-modes-menu"> <ul class="view-modes-menu">
...@@ -87,23 +100,27 @@ export default { ...@@ -87,23 +100,27 @@ export default {
</li> </li>
</ul> </ul>
</div> </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>
<div <div
v-else v-else
class="diff-viewer deleted"> class="diff-viewer"
<image-viewer >
:path="oldPath" <div class="image">
:project-path="projectPath" <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>
</div> </div>
</template> </template>
...@@ -99,6 +99,6 @@ export default { ...@@ -99,6 +99,6 @@ export default {
v-tooltip v-tooltip
:title="tooltipText" :title="tooltipText"
:tooltip-placement="tooltipPlacement" :tooltip-placement="tooltipPlacement"
>{{ username }}</span> >{{ username }}</span><slot name="avatar-badge"></slot>
</gl-link> </gl-link>
</template> </template>
...@@ -421,21 +421,13 @@ ...@@ -421,21 +421,13 @@
.diff-file-container { .diff-file-container {
.frame.deleted { .frame.deleted {
border: 0; border: 1px solid $deleted;
background-color: inherit; background-color: inherit;
.image_file img {
border: 1px solid $deleted;
}
} }
.frame.added { .frame.added {
border: 0; border: 1px solid $added;
background-color: inherit; background-color: inherit;
.image_file img {
border: 1px solid $added;
}
} }
.swipe.view, .swipe.view,
...@@ -481,6 +473,11 @@ ...@@ -481,6 +473,11 @@
bottom: -25px; bottom: -25px;
} }
} }
.discussion-notes .discussion-notes {
margin-left: 0;
border-left: 0;
}
} }
.file-content .diff-file { .file-content .diff-file {
...@@ -804,7 +801,7 @@ ...@@ -804,7 +801,7 @@
// double jagged line divider // double jagged line divider
.discussion-notes + .discussion-notes::before, .discussion-notes + .discussion-notes::before,
.discussion-notes + .discussion-form::before { .diff-file-discussions + .discussion-form::before {
content: ''; content: '';
position: relative; position: relative;
display: block; display: block;
...@@ -844,6 +841,13 @@ ...@@ -844,6 +841,13 @@
background-repeat: repeat; background-repeat: repeat;
} }
.diff-file-discussions + .discussion-form::before {
width: auto;
margin-left: -16px;
margin-right: -16px;
margin-bottom: 16px;
}
.notes { .notes {
position: relative; position: relative;
} }
...@@ -870,11 +874,13 @@ ...@@ -870,11 +874,13 @@
} }
} }
.files:not([data-can-create-note]) .frame { .files:not([data-can-create-note="true"]) .frame {
cursor: auto; cursor: auto;
} }
.frame.click-to-comment { .frame,
.frame.click-to-comment,
.btn-transparent.image-diff-overlay-add-comment {
position: relative; position: relative;
cursor: image-url('illustrations/image_comment_light_cursor.svg') cursor: image-url('illustrations/image_comment_light_cursor.svg')
$image-comment-cursor-left-offset $image-comment-cursor-top-offset, $image-comment-cursor-left-offset $image-comment-cursor-top-offset,
...@@ -910,6 +916,7 @@ ...@@ -910,6 +916,7 @@
.frame .badge.badge-pill, .frame .badge.badge-pill,
.image-diff-avatar-link .badge.badge-pill, .image-diff-avatar-link .badge.badge-pill,
.user-avatar-link .badge.badge-pill,
.notes > .badge.badge-pill { .notes > .badge.badge-pill {
position: absolute; position: absolute;
background-color: $blue-400; background-color: $blue-400;
...@@ -944,7 +951,8 @@ ...@@ -944,7 +951,8 @@
} }
} }
.image-diff-avatar-link { .image-diff-avatar-link,
.user-avatar-link {
position: relative; position: relative;
.badge.badge-pill, .badge.badge-pill,
...@@ -1073,3 +1081,14 @@ ...@@ -1073,3 +1081,14 @@
top: 0; top: 0;
} }
} }
.image-diff-overlay,
.image-diff-overlay-add-comment {
top: 0;
left: 0;
&:active,
&:focus {
outline: 0;
}
}
---
title: Reimplemented image commenting in merge request diffs
merge_request:
author:
type: added
...@@ -336,6 +336,9 @@ msgstr "" ...@@ -336,6 +336,9 @@ msgstr ""
msgid "Add a table" msgid "Add a table"
msgstr "" msgstr ""
msgid "Add image comment"
msgstr ""
msgid "Add license" msgid "Add license"
msgstr "" msgstr ""
...@@ -1718,12 +1721,18 @@ msgstr "" ...@@ -1718,12 +1721,18 @@ msgstr ""
msgid "Collapse sidebar" msgid "Collapse sidebar"
msgstr "" msgstr ""
msgid "Comment"
msgstr ""
msgid "Comment & resolve discussion" msgid "Comment & resolve discussion"
msgstr "" msgstr ""
msgid "Comment & unresolve discussion" msgid "Comment & unresolve discussion"
msgstr "" msgstr ""
msgid "Comment form position"
msgstr ""
msgid "Comments" msgid "Comments"
msgstr "" msgstr ""
......
...@@ -114,10 +114,9 @@ describe 'Merge request > User creates image diff notes', :js do ...@@ -114,10 +114,9 @@ describe 'Merge request > User creates image diff notes', :js do
create_image_diff_note create_image_diff_note
end end
# TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 it 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes' do
xit 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes' do
indicator = find('.js-image-badge', match: :first) 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(indicator).to have_content('1')
expect(badge).to have_content('1') expect(badge).to have_content('1')
...@@ -157,8 +156,7 @@ describe 'Merge request > User creates image diff notes', :js do ...@@ -157,8 +156,7 @@ describe 'Merge request > User creates image diff notes', :js do
visit project_merge_request_path(project, merge_request) visit project_merge_request_path(project, merge_request)
end end
# TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 it 'render diff indicators within the image frame' do
xit 'render diff indicators within the image frame' do
diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position)
wait_for_requests wait_for_requests
...@@ -200,7 +198,6 @@ describe 'Merge request > User creates image diff notes', :js do ...@@ -200,7 +198,6 @@ describe 'Merge request > User creates image diff notes', :js do
def create_image_diff_note def create_image_diff_note
find('.js-add-image-diff-note-button', match: :first).click 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') find('.diff-content .note-textarea').native.send_keys('image diff test comment')
click_button 'Comment' click_button 'Comment'
wait_for_requests wait_for_requests
......
import Vue from 'vue'; import Vue from 'vue';
import DiffContentComponent from '~/diffs/components/diff_content.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 { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import { GREEN_BOX_IMAGE_URL, RED_BOX_IMAGE_URL } from 'spec/test_constants'; 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 diffFileMockData from '../mock_data/diff_file';
import discussionsMockData from '../mock_data/diff_discussions';
describe('DiffContent', () => { describe('DiffContent', () => {
const Component = Vue.extend(DiffContentComponent); const Component = Vue.extend(DiffContentComponent);
let vm; let vm;
beforeEach(() => { beforeEach(() => {
const store = createStore();
store.state.notes.noteableData = {
current_user: {
can_create_note: false,
},
};
vm = mountComponentWithStore(Component, { vm = mountComponentWithStore(Component, {
store, store,
props: { props: {
...@@ -46,21 +55,57 @@ describe('DiffContent', () => { ...@@ -46,21 +55,57 @@ describe('DiffContent', () => {
}); });
describe('image diff', () => { describe('image diff', () => {
beforeEach(() => { beforeEach(done => {
vm.diffFile.newPath = GREEN_BOX_IMAGE_URL; vm.diffFile.newPath = GREEN_BOX_IMAGE_URL;
vm.diffFile.newSha = 'DEF'; vm.diffFile.newSha = 'DEF';
vm.diffFile.oldPath = RED_BOX_IMAGE_URL; vm.diffFile.oldPath = RED_BOX_IMAGE_URL;
vm.diffFile.oldSha = 'ABC'; vm.diffFile.oldSha = 'ABC';
vm.diffFile.viewPath = ''; vm.diffFile.viewPath = '';
vm.diffFile.discussions = [{ ...discussionsMockData }];
vm.$store.state.diffs.commentForms.push({
fileHash: vm.diffFile.fileHash,
x: 10,
y: 20,
width: 100,
height: 200,
});
vm.$nextTick(done);
}); });
it('should have image diff view in place', done => { it('should have image diff view in place', () => {
vm.$nextTick(() => { expect(vm.$el.querySelectorAll('.js-diff-inline-view').length).toEqual(0);
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);
});
done(); it('renders image diff overlay', () => {
expect(vm.$el.querySelector('.image-diff-overlay')).not.toBe(null);
});
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,
width: 100,
height: 200,
},
});
}); });
}); });
}); });
......
import Vue from 'vue'; import Vue from 'vue';
import DiffDiscussions from '~/diffs/components/diff_discussions.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 { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import '~/behaviors/markdown/render_gfm';
import discussionsMockData from '../mock_data/diff_discussions'; import discussionsMockData from '../mock_data/diff_discussions';
describe('DiffDiscussions', () => { describe('DiffDiscussions', () => {
let component; let vm;
const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
beforeEach(() => { function createComponent(props = {}) {
component = createComponentWithStore(Vue.extend(DiffDiscussions), store, { const store = createStore();
vm = createComponentWithStore(Vue.extend(DiffDiscussions), store, {
discussions: getDiscussionsMockData(), discussions: getDiscussionsMockData(),
...props,
}).$mount(); }).$mount();
}
afterEach(() => {
vm.$destroy();
}); });
describe('template', () => { describe('template', () => {
it('should have notes list', () => { 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');
}); });
}); });
}); });
import Vue from 'vue';
import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue';
import { createStore } from '~/mr_notes/stores';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import { imageDiffDiscussions } from '../mock_data/diff_discussions';
describe('Diffs image diff overlay component', () => {
const dimensions = {
width: 100,
height: 200,
};
let Component;
let vm;
function createComponent(props = {}, extendStore = () => {}) {
const store = createStore();
extendStore(store);
vm = createComponentWithStore(Component, store, {
discussions: [...imageDiffDiscussions],
fileHash: 'ABC',
...props,
});
}
beforeAll(() => {
Component = Vue.extend(ImageDiffOverlay);
});
afterEach(() => {
vm.$destroy();
});
it('renders comment badges', () => {
createComponent();
spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
vm.$mount();
expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(2);
});
it('renders index of discussion in badge', () => {
createComponent();
spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
vm.$mount();
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 });
spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
vm.$mount();
expect(vm.$el.querySelector('.js-image-badge svg')).not.toBe(null);
});
it('sets badge comment positions', () => {
createComponent();
spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
vm.$mount();
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],
},
});
spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
vm.$mount();
expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(1);
});
it('dispatches openDiffFileCommentForm when clicking overlay', () => {
createComponent({ canComment: true });
spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
vm.$mount();
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,
width: 100,
height: 200,
});
});
describe('toggle discussion', () => {
it('disables buttons when shouldToggleDiscussion is false', () => {
createComponent({ shouldToggleDiscussion: false });
spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
vm.$mount();
expect(vm.$el.querySelector('.js-image-badge').hasAttribute('disabled')).toBe(true);
});
it('dispatches toggleDiscussion when clicking image badge', () => {
createComponent();
spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
vm.$mount();
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,
});
});
spyOn(vm, 'getImageDimensions').and.returnValue(dimensions);
vm.$mount();
});
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');
});
});
});
...@@ -492,3 +492,24 @@ export default { ...@@ -492,3 +492,24 @@ export default {
image_diff_html: 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', '<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,
width: 100,
height: 200,
},
},
{
id: '2',
position: {
x: 5,
y: 5,
width: 100,
height: 200,
},
},
];
...@@ -237,4 +237,5 @@ export default { ...@@ -237,4 +237,5 @@ export default {
}, },
}, },
], ],
discussions: [],
}; };
...@@ -218,6 +218,7 @@ describe('DiffsStoreActions', () => { ...@@ -218,6 +218,7 @@ describe('DiffsStoreActions', () => {
], ],
}; };
const singleDiscussion = { const singleDiscussion = {
id: '1',
fileHash: 'ABC', fileHash: 'ABC',
line_code: 'ABC_1_1', line_code: 'ABC_1_1',
}; };
...@@ -230,6 +231,7 @@ describe('DiffsStoreActions', () => { ...@@ -230,6 +231,7 @@ describe('DiffsStoreActions', () => {
{ {
type: types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, type: types.REMOVE_LINE_DISCUSSIONS_FOR_FILE,
payload: { payload: {
id: '1',
fileHash: 'ABC', fileHash: 'ABC',
lineCode: 'ABC_1_1', lineCode: 'ABC_1_1',
}, },
......
import Vue from 'vue'; import Vue from 'vue';
import DiffWithNote from '~/notes/components/diff_with_note.vue'; import DiffWithNote from '~/notes/components/diff_with_note.vue';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import createStore from '~/notes/stores'; import { createStore } from '~/mr_notes/stores';
import { mountComponentWithStore } from 'spec/helpers'; import { mountComponentWithStore } from 'spec/helpers';
const discussionFixture = 'merge_requests/diff_discussion.json'; const discussionFixture = 'merge_requests/diff_discussion.json';
......
...@@ -47,7 +47,7 @@ describe('ContentViewer', () => { ...@@ -47,7 +47,7 @@ describe('ContentViewer', () => {
}); });
setTimeout(() => { 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(); done();
}); });
......
...@@ -30,11 +30,11 @@ describe('DiffViewer', () => { ...@@ -30,11 +30,11 @@ describe('DiffViewer', () => {
}); });
setTimeout(() => { 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}`, `//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}`, `//raw/ABC/${GREEN_BOX_IMAGE_URL}`,
); );
......
...@@ -52,13 +52,9 @@ describe('ImageDiffViewer', () => { ...@@ -52,13 +52,9 @@ describe('ImageDiffViewer', () => {
}); });
setTimeout(() => { setTimeout(() => {
expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe( expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
GREEN_BOX_IMAGE_URL,
);
expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe( expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(RED_BOX_IMAGE_URL);
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.active').textContent.trim()).toBe('2-up');
expect(vm.$el.querySelector('.view-modes-menu li:nth-child(2)').textContent.trim()).toBe( expect(vm.$el.querySelector('.view-modes-menu li:nth-child(2)').textContent.trim()).toBe(
...@@ -81,9 +77,7 @@ describe('ImageDiffViewer', () => { ...@@ -81,9 +77,7 @@ describe('ImageDiffViewer', () => {
}); });
setTimeout(() => { setTimeout(() => {
expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe( expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
GREEN_BOX_IMAGE_URL,
);
done(); done();
}); });
...@@ -97,9 +91,7 @@ describe('ImageDiffViewer', () => { ...@@ -97,9 +91,7 @@ describe('ImageDiffViewer', () => {
}); });
setTimeout(() => { setTimeout(() => {
expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe( expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(RED_BOX_IMAGE_URL);
RED_BOX_IMAGE_URL,
);
done(); done();
}); });
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment