diff options
13 files changed, 598 insertions, 646 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue index 8999fd2ac96..a74ea4bfaaf 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -4,14 +4,7 @@ import { s__ } from '~/locale'; import { mapState, mapGetters, mapActions } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; -import { - MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, - OLD_NO_NEW_LINE_TYPE, - NEW_NO_NEW_LINE_TYPE, - LINE_POSITION_RIGHT, - UNFOLD_COUNT, -} from '../constants'; +import { LINE_POSITION_RIGHT, UNFOLD_COUNT } from '../constants'; import * as utils from '../store/utils'; export default { @@ -63,6 +56,21 @@ export default { required: false, default: false, }, + isMatchLine: { + type: Boolean, + required: false, + default: false, + }, + isMetaLine: { + type: Boolean, + required: false, + default: false, + }, + isContextLine: { + type: Boolean, + required: false, + default: false, + }, }, computed: { ...mapState({ @@ -70,15 +78,6 @@ export default { diffFiles: state => state.diffs.diffFiles, }), ...mapGetters(['isLoggedIn', 'discussionsByLineCode']), - isMatchLine() { - return this.lineType === MATCH_LINE_TYPE; - }, - isContextLine() { - return this.lineType === CONTEXT_LINE_TYPE; - }, - isMetaLine() { - return this.lineType === OLD_NO_NEW_LINE_TYPE || this.lineType === NEW_NO_NEW_LINE_TYPE; - }, lineHref() { return this.lineCode ? `#${this.lineCode}` : '#'; }, @@ -109,9 +108,9 @@ export default { }, }, methods: { - ...mapActions(['loadMoreLines']), + ...mapActions(['loadMoreLines', 'showCommentForm']), handleCommentButton() { - this.$emit('showCommentForm', { lineCode: this.lineCode }); + this.showCommentForm({ lineCode: this.lineCode }); }, handleLoadMoreLines() { if (this.isRequesting) { diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue new file mode 100644 index 00000000000..68fe6787f9b --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -0,0 +1,131 @@ +<script> +import { mapGetters } from 'vuex'; +import DiffLineGutterContent from './diff_line_gutter_content.vue'; +import { + MATCH_LINE_TYPE, + CONTEXT_LINE_TYPE, + EMPTY_CELL_TYPE, + OLD_LINE_TYPE, + OLD_NO_NEW_LINE_TYPE, + NEW_NO_NEW_LINE_TYPE, + LINE_HOVER_CLASS_NAME, + LINE_UNFOLD_CLASS_NAME, +} from '../constants'; + +export default { + components: { + DiffLineGutterContent, + }, + props: { + line: { + type: Object, + required: true, + }, + diffFile: { + type: Object, + required: true, + }, + showCommentButton: { + type: Boolean, + required: false, + default: false, + }, + linePosition: { + type: String, + required: false, + default: '', + }, + lineType: { + type: String, + required: false, + default: '', + }, + isContentLine: { + type: Boolean, + required: false, + default: false, + }, + isBottom: { + type: Boolean, + required: false, + default: false, + }, + isHover: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + ...mapGetters(['isLoggedIn', 'isInlineView']), + normalizedLine() { + if (this.isInlineView) { + return this.line; + } + + return this.lineType === OLD_LINE_TYPE ? this.line.left : this.line.right; + }, + isMatchLine() { + return this.normalizedLine.type === MATCH_LINE_TYPE; + }, + isContextLine() { + return this.normalizedLine.type === CONTEXT_LINE_TYPE; + }, + isMetaLine() { + return ( + this.normalizedLine.type === OLD_NO_NEW_LINE_TYPE || + this.normalizedLine.type === NEW_NO_NEW_LINE_TYPE || + this.normalizedLine.type === EMPTY_CELL_TYPE + ); + }, + classNameMap() { + const { type } = this.normalizedLine; + + return { + [type]: type, + [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine, + [LINE_HOVER_CLASS_NAME]: + this.isLoggedIn && + this.isHover && + !this.isMatchLine && + !this.isContextLine && + !this.isMetaLine, + }; + }, + lineNumber() { + const { lineType, normalizedLine } = this; + + return lineType === OLD_LINE_TYPE ? normalizedLine.oldLine : normalizedLine.newLine; + }, + }, +}; +</script> + +<template> + <td + v-if="isContentLine" + :class="lineType" + class="line_content" + v-html="normalizedLine.richText" + > + </td> + <td + v-else + :class="classNameMap" + > + <diff-line-gutter-content + :file-hash="diffFile.fileHash" + :line-type="normalizedLine.type" + :line-code="normalizedLine.lineCode" + :line-position="linePosition" + :line-number="lineNumber" + :meta-data="normalizedLine.metaData" + :show-comment-button="showCommentButton" + :context-lines-path="diffFile.contextLinesPath" + :is-bottom="isBottom" + :is-match-line="isMatchLine" + :is-context-line="isContentLine" + :is-meta-line="isMetaLine" + /> + </td> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_table_row.vue b/app/assets/javascripts/diffs/components/diff_table_row.vue new file mode 100644 index 00000000000..8716fdaf44d --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_table_row.vue @@ -0,0 +1,191 @@ +<script> +import $ from 'jquery'; +import { mapGetters } from 'vuex'; +import DiffTableCell from './diff_table_cell.vue'; +import { + NEW_LINE_TYPE, + OLD_LINE_TYPE, + CONTEXT_LINE_TYPE, + CONTEXT_LINE_CLASS_NAME, + OLD_NO_NEW_LINE_TYPE, + PARALLEL_DIFF_VIEW_TYPE, + NEW_NO_NEW_LINE_TYPE, + LINE_POSITION_LEFT, + LINE_POSITION_RIGHT, +} from '../constants'; + +export default { + components: { + DiffTableCell, + }, + props: { + diffFile: { + type: Object, + required: true, + }, + line: { + type: Object, + required: true, + }, + isBottom: { + type: Boolean, + required: false, + default: false, + }, + }, + data() { + return { + isHover: false, + isLeftHover: false, + isRightHover: false, + }; + }, + computed: { + ...mapGetters(['isInlineView', 'isParallelView']), + isContextLine() { + return this.line.left + ? this.line.left.type === CONTEXT_LINE_TYPE + : this.line.type === CONTEXT_LINE_TYPE; + }, + classNameMap() { + return { + [this.line.type]: this.line.type, + [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, + [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView, + }; + }, + inlineRowId() { + const { lineCode, oldLine, newLine } = this.line; + + return lineCode || `${this.diffFile.fileHash}_${oldLine}_${newLine}`; + }, + parallelViewLeftLineType() { + if (this.line.right.type === NEW_NO_NEW_LINE_TYPE) { + return OLD_NO_NEW_LINE_TYPE; + } + + return this.line.left.type; + }, + }, + created() { + this.newLineType = NEW_LINE_TYPE; + this.oldLineType = OLD_LINE_TYPE; + this.linePositionLeft = LINE_POSITION_LEFT; + this.linePositionRight = LINE_POSITION_RIGHT; + }, + methods: { + handleMouseMove(e) { + const isHover = e.type === 'mouseover'; + + if (this.isInlineView) { + this.isHover = isHover; + } else { + const hoveringCell = e.target.closest('td'); + const allCellsInHoveringRow = Array.from(e.currentTarget.children); + const hoverIndex = allCellsInHoveringRow.indexOf(hoveringCell); + + if (hoverIndex >= 2) { + this.isRightHover = isHover; + } else { + this.isLeftHover = isHover; + } + } + }, + // Prevent text selecting on both sides of parallel diff view + // Backport of the same code from legacy diff notes. + handleParallelLineMouseDown(e) { + const line = $(e.currentTarget); + const table = line.closest('table'); + + table.removeClass('left-side-selected right-side-selected'); + const [lineClass] = ['left-side', 'right-side'].filter(name => line.hasClass(name)); + + if (lineClass) { + table.addClass(`${lineClass}-selected`); + } + }, + }, +}; +</script> + +<template> + <tr + v-if="isInlineView" + :id="inlineRowId" + :class="classNameMap" + class="line_holder" + @mouseover="handleMouseMove" + @mouseout="handleMouseMove" + > + <diff-table-cell + :diff-file="diffFile" + :line="line" + :line-type="oldLineType" + :is-bottom="isBottom" + :is-hover="isHover" + :show-comment-button="true" + class="diff-line-num old_line" + /> + <diff-table-cell + :diff-file="diffFile" + :line="line" + :line-type="newLineType" + :is-bottom="isBottom" + :is-hover="isHover" + class="diff-line-num new_line" + /> + <diff-table-cell + :class="line.type" + :diff-file="diffFile" + :line="line" + :is-content-line="true" + /> + </tr> + + <tr + v-else + :class="classNameMap" + class="line_holder" + @mouseover="handleMouseMove" + @mouseout="handleMouseMove" + > + <diff-table-cell + :diff-file="diffFile" + :line="line" + :line-type="oldLineType" + :line-position="linePositionLeft" + :is-bottom="isBottom" + :is-hover="isLeftHover" + :show-comment-button="true" + class="diff-line-num old_line" + /> + <diff-table-cell + :id="line.left.lineCode" + :diff-file="diffFile" + :line="line" + :is-content-line="true" + :line-type="parallelViewLeftLineType" + class="line_content parallel left-side" + @mousedown.native="handleParallelLineMouseDown" + /> + <diff-table-cell + :diff-file="diffFile" + :line="line" + :line-type="newLineType" + :line-position="linePositionRight" + :is-bottom="isBottom" + :is-hover="isRightHover" + :show-comment-button="true" + class="diff-line-num new_line" + /> + <diff-table-cell + :id="line.right.lineCode" + :diff-file="diffFile" + :line="line" + :is-content-line="true" + :line-type="line.right.type" + class="line_content parallel right-side" + @mousedown.native="handleParallelLineMouseDown" + /> + </tr> +</template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue new file mode 100644 index 00000000000..0e935f1d68e --- /dev/null +++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue @@ -0,0 +1,82 @@ +<script> +import { mapState, mapGetters } from 'vuex'; +import diffDiscussions from './diff_discussions.vue'; +import diffLineNoteForm from './diff_line_note_form.vue'; + +export default { + components: { + diffDiscussions, + diffLineNoteForm, + }, + props: { + line: { + type: Object, + required: true, + }, + diffFile: { + type: Object, + required: true, + }, + diffLines: { + type: Array, + required: true, + }, + lineIndex: { + type: Number, + required: true, + }, + }, + computed: { + ...mapState({ + diffLineCommentForms: state => state.diffs.diffLineCommentForms, + }), + ...mapGetters(['discussionsByLineCode']), + isDiscussionExpanded() { + if (!this.discussions.length) { + return false; + } + + return this.discussions.every(discussion => discussion.expanded); + }, + hasCommentForm() { + return this.diffLineCommentForms[this.line.lineCode]; + }, + discussions() { + return this.discussionsByLineCode[this.line.lineCode] || []; + }, + shouldRender() { + return this.isDiscussionExpanded || this.hasCommentForm; + }, + className() { + return this.discussions.length ? '' : 'js-temp-notes-holder'; + }, + }, +}; +</script> + +<template> + <tr + v-if="shouldRender" + :class="className" + class="notes_holder" + > + <td + class="notes_line" + colspan="2" + ></td> + <td class="notes_content"> + <div class="content"> + <diff-discussions + :discussions="discussions" + /> + <diff-line-note-form + v-if="diffLineCommentForms[line.lineCode]" + :diff-file="diffFile" + :diff-lines="diffLines" + :line="line" + :note-target-line="diffLines[lineIndex]" + /> + </div> + </td> + </tr> +</template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index 21376117bef..e72f85df77a 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -1,34 +1,12 @@ <script> import diffContentMixin from '../mixins/diff_content'; -import { - MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, - OLD_NO_NEW_LINE_TYPE, - NEW_NO_NEW_LINE_TYPE, - LINE_HOVER_CLASS_NAME, - LINE_UNFOLD_CLASS_NAME, -} from '../constants'; +import inlineDiffCommentRow from './inline_diff_comment_row.vue'; export default { - mixins: [diffContentMixin], - methods: { - handleMouse(lineCode, isOver) { - this.hoveredLineCode = isOver ? lineCode : null; - }, - getLineClass(line) { - const isSameLine = this.hoveredLineCode && this.hoveredLineCode === line.lineCode; - const isMatchLine = line.type === MATCH_LINE_TYPE; - const isContextLine = line.type === CONTEXT_LINE_TYPE; - const isMetaLine = line.type === OLD_NO_NEW_LINE_TYPE || line.type === NEW_NO_NEW_LINE_TYPE; - - return { - [line.type]: line.type, - [LINE_UNFOLD_CLASS_NAME]: isMatchLine, - [LINE_HOVER_CLASS_NAME]: - this.isLoggedIn && isSameLine && !isMatchLine && !isContextLine && !isMetaLine, - }; - }, + components: { + inlineDiffCommentRow, }, + mixins: [diffContentMixin], }; </script> @@ -41,76 +19,19 @@ export default { <template v-for="(line, index) in normalizedDiffLines" > - <tr - :id="line.lineCode || `${fileHash}_${line.oldLine}_${line.newLine}`" + <diff-table-row + :diff-file="diffFile" + :line="line" + :is-bottom="index + 1 === diffLinesLength" :key="line.lineCode" - :class="getRowClass(line)" - class="line_holder" - @mouseover="handleMouse(line.lineCode, true)" - @mouseout="handleMouse(line.lineCode, false)" - > - <td - :class="getLineClass(line)" - class="diff-line-num old_line" - > - <diff-line-gutter-content - :file-hash="fileHash" - :line-type="line.type" - :line-code="line.lineCode" - :line-number="line.oldLine" - :meta-data="line.metaData" - :show-comment-button="true" - :context-lines-path="diffFile.contextLinesPath" - :is-bottom="index + 1 === diffLinesLength" - @showCommentForm="handleShowCommentForm" - /> - </td> - <td - :class="getLineClass(line)" - class="diff-line-num new_line" - > - <diff-line-gutter-content - :file-hash="fileHash" - :line-type="line.type" - :line-code="line.lineCode" - :line-number="line.newLine" - :meta-data="line.metaData" - :is-bottom="index + 1 === diffLinesLength" - :context-lines-path="diffFile.contextLinesPath" - /> - </td> - <td - :class="line.type" - class="line_content" - v-html="line.richText" - > - </td> - </tr> - <tr - v-if="isDiscussionExpanded(line.lineCode) || diffLineCommentForms[line.lineCode]" + /> + <inline-diff-comment-row + :diff-file="diffFile" + :diff-lines="normalizedDiffLines" + :line="line" + :line-index="index" :key="index" - :class="discussionsByLineCode[line.lineCode] ? '' : 'js-temp-notes-holder'" - class="notes_holder" - > - <td - class="notes_line" - colspan="2" - ></td> - <td class="notes_content"> - <div class="content"> - <diff-discussions - :discussions="discussionsByLineCode[line.lineCode] || []" - /> - <diff-line-note-form - v-if="diffLineCommentForms[line.lineCode]" - :diff-file="diffFile" - :diff-lines="diffLines" - :line="line" - :note-target-line="diffLines[index]" - /> - </div> - </td> - </tr> + /> </template> </tbody> </table> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue new file mode 100644 index 00000000000..5f33ec7a3c2 --- /dev/null +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -0,0 +1,129 @@ +<script> +import { mapState, mapGetters } from 'vuex'; +import diffDiscussions from './diff_discussions.vue'; +import diffLineNoteForm from './diff_line_note_form.vue'; + +export default { + components: { + diffDiscussions, + diffLineNoteForm, + }, + props: { + line: { + type: Object, + required: true, + }, + diffFile: { + type: Object, + required: true, + }, + diffLines: { + type: Array, + required: true, + }, + lineIndex: { + type: Number, + required: true, + }, + }, + computed: { + ...mapState({ + diffLineCommentForms: state => state.diffs.diffLineCommentForms, + }), + ...mapGetters(['discussionsByLineCode']), + leftLineCode() { + return this.line.left.lineCode; + }, + rightLineCode() { + return this.line.right.lineCode; + }, + hasDiscussion() { + const discussions = this.discussionsByLineCode; + + return discussions[this.leftLineCode] || discussions[this.rightLineCode]; + }, + hasExpandedDiscussionOnLeft() { + const discussions = this.discussionsByLineCode[this.leftLineCode]; + + return discussions ? discussions.every(discussion => discussion.expanded) : false; + }, + hasExpandedDiscussionOnRight() { + const discussions = this.discussionsByLineCode[this.rightLineCode]; + + return discussions ? discussions.every(discussion => discussion.expanded) : false; + }, + hasAnyExpandedDiscussion() { + return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; + }, + shouldRenderDiscussionsRow() { + const hasDiscussion = this.hasDiscussion && this.hasAnyExpandedDiscussion; + const hasCommentFormOnLeft = this.diffLineCommentForms[this.leftLineCode]; + const hasCommentFormOnRight = this.diffLineCommentForms[this.rightLineCode]; + + return hasDiscussion || hasCommentFormOnLeft || hasCommentFormOnRight; + }, + shouldRenderDiscussionsOnLeft() { + return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft; + }, + shouldRenderDiscussionsOnRight() { + return ( + this.discussionsByLineCode[this.rightLineCode] && + this.hasExpandedDiscussionOnRight && + this.line.right.type + ); + }, + className() { + return this.hasDiscussion ? '' : 'js-temp-notes-holder'; + }, + }, +}; +</script> + +<template> + <tr + v-if="shouldRenderDiscussionsRow" + :class="className" + class="notes_holder" + > + <td class="notes_line old"></td> + <td class="notes_content parallel old"> + <div + v-if="shouldRenderDiscussionsOnLeft" + class="content" + > + <diff-discussions + :discussions="discussionsByLineCode[leftLineCode]" + /> + </div> + <diff-line-note-form + v-if="diffLineCommentForms[leftLineCode] && + diffLineCommentForms[leftLineCode]" + :diff-file="diffFile" + :diff-lines="diffLines" + :line="line.left" + :note-target-line="diffLines[lineIndex].left" + position="left" + /> + </td> + <td class="notes_line new"></td> + <td class="notes_content parallel new"> + <div + v-if="shouldRenderDiscussionsOnRight" + class="content" + > + <diff-discussions + :discussions="discussionsByLineCode[rightLineCode]" + /> + </div> + <diff-line-note-form + v-if="diffLineCommentForms[rightLineCode] && + diffLineCommentForms[rightLineCode] && line.right.type" + :diff-file="diffFile" + :diff-lines="diffLines" + :line="line.right" + :note-target-line="diffLines[lineIndex].right" + position="right" + /> + </td> + </tr> +</template> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 60edbcbbda8..ed92b4ee249 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -1,17 +1,12 @@ <script> import diffContentMixin from '../mixins/diff_content'; -import { - EMPTY_CELL_TYPE, - MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, - OLD_NO_NEW_LINE_TYPE, - NEW_NO_NEW_LINE_TYPE, - LINE_HOVER_CLASS_NAME, - LINE_UNFOLD_CLASS_NAME, - LINE_POSITION_RIGHT, -} from '../constants'; +import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; +import { EMPTY_CELL_TYPE } from '../constants'; export default { + components: { + parallelDiffCommentRow, + }, mixins: [diffContentMixin], computed: { parallelDiffLines() { @@ -26,77 +21,6 @@ export default { }); }, }, - methods: { - hasDiscussion(line) { - const discussions = this.discussionsByLineCode; - const hasDiscussion = discussions[line.left.lineCode] || discussions[line.right.lineCode]; - - return hasDiscussion; - }, - getClassName(line, position) { - const { type, lineCode } = line[position]; - const isMatchLine = type === MATCH_LINE_TYPE; - const isContextLine = !isMatchLine && type !== EMPTY_CELL_TYPE && type !== CONTEXT_LINE_TYPE; - const isMetaLine = type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE; - const isSameLine = this.hoveredLineCode && this.hoveredLineCode === lineCode; - const isSameSection = position === this.hoveredSection; - - return { - [type]: type, - [LINE_UNFOLD_CLASS_NAME]: isMatchLine, - [LINE_HOVER_CLASS_NAME]: - this.isLoggedIn && isContextLine && isSameLine && isSameSection && !isMetaLine, - }; - }, - handleMouse(e, line, isHover) { - if (isHover) { - const cell = e.target.closest('td'); - - if (this.$refs.leftLines.indexOf(cell) > -1) { - this.hoveredLineCode = line.left.lineCode; - this.hoveredSection = 'left'; - } else if (this.$refs.rightLines.indexOf(cell) > -1) { - this.hoveredLineCode = line.right.lineCode; - this.hoveredSection = 'right'; - } - } else { - this.hoveredLineCode = null; - this.hoveredSection = null; - } - }, - shouldRenderDiscussionsRow(line) { - const hasDiscussion = this.hasDiscussion(line) && this.hasAnyExpandedDiscussion(line); - const hasCommentFormOnLeft = this.diffLineCommentForms[line.left.lineCode]; - const hasCommentFormOnRight = this.diffLineCommentForms[line.right.lineCode]; - - return hasDiscussion || hasCommentFormOnLeft || hasCommentFormOnRight; - }, - shouldRenderDiscussions(line, position) { - const { lineCode } = line[position]; - let render = this.discussionsByLineCode[lineCode] && this.isDiscussionExpanded(lineCode); - - // Avoid rendering context line discussions on the right side in parallel view - if (position === LINE_POSITION_RIGHT) { - render = render && line.right.type; - } - - return render; - }, - hasAnyExpandedDiscussion(line) { - const isLeftExpanded = this.isDiscussionExpanded(line.left.lineCode); - const isRightExpanded = this.isDiscussionExpanded(line.right.lineCode); - - return isLeftExpanded || isRightExpanded; - }, - getLineCode(line, side) { - const { lineCode } = side; - if (lineCode) { - return lineCode; - } - - return `${this.fileHash}_${line.left.oldLine}_${line.right.newLine}`; - }, - }, }; </script> @@ -104,119 +28,26 @@ export default { <div :class="userColorScheme" :data-commit-id="commitId" - class="code diff-wrap-lines js-syntax-highlight text-file"> + class="code diff-wrap-lines js-syntax-highlight text-file" + > <table> <tbody> <template v-for="(line, index) in parallelDiffLines" > - <tr + <diff-table-row + :diff-file="diffFile" + :line="line" + :is-bottom="index + 1 === diffLinesLength" :key="index" - :class="getRowClass(line)" - class="line_holder parallel" - @mouseover="handleMouse($event, line, true)" - @mouseout="handleMouse($event, line, false)" - > - <td - ref="leftLines" - :class="getClassName(line, 'left')" - class="diff-line-num old_line" - > - <diff-line-gutter-content - :file-hash="fileHash" - :line-type="line.left.type" - :line-code="line.left.lineCode" - :line-number="line.left.oldLine" - :meta-data="line.left.metaData" - :show-comment-button="true" - :context-lines-path="diffFile.contextLinesPath" - :is-bottom="index + 1 === diffLinesLength" - line-position="left" - @showCommentForm="handleShowCommentForm" - /> - </td> - <td - ref="leftLines" - :class="getClassName(line, 'left')" - :id="getLineCode(line, line.left)" - class="line_content parallel left-side" - v-html="line.left.richText" - > - </td> - <td - ref="rightLines" - :class="getClassName(line, 'right')" - class="diff-line-num new_line" - > - <diff-line-gutter-content - :file-hash="fileHash" - :line-type="line.right.type" - :line-code="line.right.lineCode" - :line-number="line.right.newLine" - :meta-data="line.right.metaData" - :show-comment-button="true" - :context-lines-path="diffFile.contextLinesPath" - :is-bottom="index + 1 === diffLinesLength" - line-position="right" - @showCommentForm="handleShowCommentForm" - /> - </td> - <td - ref="rightLines" - :class="getClassName(line, 'right')" - :id="getLineCode(line, line.right)" - class="line_content parallel right-side" - v-html="line.right.richText" - > - </td> - </tr> - <tr - v-if="shouldRenderDiscussionsRow(line)" + /> + <parallel-diff-comment-row :key="line.left.lineCode || line.right.lineCode" - :class="hasDiscussion(line) ? '' : 'js-temp-notes-holder'" - class="notes_holder" - > - <td class="notes_line old"></td> - <td class="notes_content parallel old"> - <div - v-if="shouldRenderDiscussions(line, 'left')" - class="content" - > - <diff-discussions - :discussions="discussionsByLineCode[line.left.lineCode]" - /> - </div> - <diff-line-note-form - v-if="diffLineCommentForms[line.left.lineCode] && - diffLineCommentForms[line.left.lineCode]" - :diff-file="diffFile" - :diff-lines="diffLines" - :line="line.left" - :note-target-line="diffLines[index].left" - position="left" - /> - </td> - <td class="notes_line new"></td> - <td class="notes_content parallel new"> - <div - v-if="shouldRenderDiscussions(line, 'right')" - class="content" - > - <diff-discussions - :discussions="discussionsByLineCode[line.right.lineCode]" - /> - </div> - <diff-line-note-form - v-if="diffLineCommentForms[line.right.lineCode] && - diffLineCommentForms[line.right.lineCode] && line.right.type" - :diff-file="diffFile" - :diff-lines="diffLines" - :line="line.right" - :note-target-line="diffLines[index].right" - position="right" - /> - </td> - </tr> + :line="line" + :diff-file="diffFile" + :diff-lines="parallelDiffLines" + :line-index="index" + /> </template> </tbody> </table> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index d314f08e60e..2fa8367f528 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -14,6 +14,8 @@ export const TEXT_DIFF_POSITION_TYPE = 'text'; export const LINE_POSITION_LEFT = 'left'; export const LINE_POSITION_RIGHT = 'right'; +export const LINE_SIDE_LEFT = 'left-side'; +export const LINE_SIDE_RIGHT = 'right-side'; export const DIFF_VIEW_COOKIE_NAME = 'diff_view'; export const LINE_HOVER_CLASS_NAME = 'is-over'; diff --git a/app/assets/javascripts/diffs/mixins/diff_content.js b/app/assets/javascripts/diffs/mixins/diff_content.js index bef06ad2b52..ebb511d3a7e 100644 --- a/app/assets/javascripts/diffs/mixins/diff_content.js +++ b/app/assets/javascripts/diffs/mixins/diff_content.js @@ -1,9 +1,9 @@ -import { mapState, mapGetters, mapActions } from 'vuex'; +import { mapGetters } from 'vuex'; import diffDiscussions from '../components/diff_discussions.vue'; import diffLineGutterContent from '../components/diff_line_gutter_content.vue'; import diffLineNoteForm from '../components/diff_line_note_form.vue'; +import diffTableRow from '../components/diff_table_row.vue'; import { trimFirstCharOfLineContent } from '../store/utils'; -import { CONTEXT_LINE_TYPE, CONTEXT_LINE_CLASS_NAME } from '../constants'; export default { props: { @@ -16,22 +16,14 @@ export default { required: true, }, }, - data() { - return { - hoveredLineCode: null, - hoveredSection: null, - }; - }, components: { diffDiscussions, + diffTableRow, diffLineNoteForm, diffLineGutterContent, }, computed: { - ...mapState({ - diffLineCommentForms: state => state.diffs.diffLineCommentForms, - }), - ...mapGetters(['discussionsByLineCode', 'isLoggedIn', 'commit']), + ...mapGetters(['commit']), commitId() { return this.commit && this.commit.id; }, @@ -41,15 +33,15 @@ export default { normalizedDiffLines() { return this.diffLines.map(line => { if (line.richText) { - return this.trimFirstChar(line); + return trimFirstCharOfLineContent(line); } if (line.left) { - Object.assign(line, { left: this.trimFirstChar(line.left) }); + Object.assign(line, { left: trimFirstCharOfLineContent(line.left) }); } if (line.right) { - Object.assign(line, { right: this.trimFirstChar(line.right) }); + Object.assign(line, { right: trimFirstCharOfLineContent(line.right) }); } return line; @@ -62,28 +54,4 @@ export default { return this.diffFile.fileHash; }, }, - methods: { - ...mapActions(['showCommentForm', 'cancelCommentForm']), - getRowClass(line) { - const isContextLine = line.left - ? line.left.type === CONTEXT_LINE_TYPE - : line.type === CONTEXT_LINE_TYPE; - - return { - [line.type]: line.type, - [CONTEXT_LINE_CLASS_NAME]: isContextLine, - }; - }, - trimFirstChar(line) { - return trimFirstCharOfLineContent(line); - }, - handleShowCommentForm(params) { - this.showCommentForm({ lineCode: params.lineCode }); - }, - isDiscussionExpanded(lineCode) { - const discussions = this.discussionsByLineCode[lineCode]; - - return discussions ? discussions.every(discussion => discussion.expanded) : false; - }, - }, }; diff --git a/app/assets/stylesheets/highlight/white_base.scss b/app/assets/stylesheets/highlight/white_base.scss index 8cc5252648d..90a5250c247 100644 --- a/app/assets/stylesheets/highlight/white_base.scss +++ b/app/assets/stylesheets/highlight/white_base.scss @@ -102,7 +102,9 @@ pre.code, // Diff line .line_holder { - &.match .line_content { + &.match .line_content, + .new-nonewline.line_content, + .old-nonewline.line_content { @include matchLine; } diff --git a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js index cce10c4083c..2d136a63c52 100644 --- a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js +++ b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js @@ -2,12 +2,6 @@ import Vue from 'vue'; import DiffLineGutterContent from '~/diffs/components/diff_line_gutter_content.vue'; import store from '~/mr_notes/stores'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; -import { - MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, - OLD_NO_NEW_LINE_TYPE, - NEW_NO_NEW_LINE_TYPE, -} from '~/diffs/constants'; import discussionsMockData from '../mock_data/diff_discussions'; import diffFileMockData from '../mock_data/diff_file'; @@ -31,45 +25,6 @@ describe('DiffLineGutterContent', () => { }; describe('computed', () => { - describe('isMatchLine', () => { - it('should return true for match line type', () => { - const component = createComponent({ lineType: MATCH_LINE_TYPE }); - expect(component.isMatchLine).toEqual(true); - }); - - it('should return false for non-match line type', () => { - const component = createComponent({ lineType: CONTEXT_LINE_TYPE }); - expect(component.isMatchLine).toEqual(false); - }); - }); - - describe('isContextLine', () => { - it('should return true for context line type', () => { - const component = createComponent({ lineType: CONTEXT_LINE_TYPE }); - expect(component.isContextLine).toEqual(true); - }); - - it('should return false for non-context line type', () => { - const component = createComponent({ lineType: MATCH_LINE_TYPE }); - expect(component.isContextLine).toEqual(false); - }); - }); - - describe('isMetaLine', () => { - it('should return true for meta line type', () => { - const component = createComponent({ lineType: NEW_NO_NEW_LINE_TYPE }); - expect(component.isMetaLine).toEqual(true); - - const component2 = createComponent({ lineType: OLD_NO_NEW_LINE_TYPE }); - expect(component2.isMetaLine).toEqual(true); - }); - - it('should return false for non-meta line type', () => { - const component = createComponent({ lineType: MATCH_LINE_TYPE }); - expect(component.isMetaLine).toEqual(false); - }); - }); - describe('lineHref', () => { it('should prepend # to lineCode', () => { const lineCode = 'LC_42'; @@ -109,7 +64,7 @@ describe('DiffLineGutterContent', () => { describe('template', () => { it('should render three dots for context lines', () => { const component = createComponent({ - lineType: MATCH_LINE_TYPE, + isMatchLine: true, }); expect(component.$el.querySelector('span').classList.contains('context-cell')).toEqual(true); diff --git a/spec/javascripts/diffs/components/inline_diff_view_spec.js b/spec/javascripts/diffs/components/inline_diff_view_spec.js index 0d5a3576204..e1adf60962e 100644 --- a/spec/javascripts/diffs/components/inline_diff_view_spec.js +++ b/spec/javascripts/diffs/components/inline_diff_view_spec.js @@ -1,7 +1,6 @@ import Vue from 'vue'; import InlineDiffView from '~/diffs/components/inline_diff_view.vue'; import store from '~/mr_notes/stores'; -import * as constants from '~/diffs/constants'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import diffFileMockData from '../mock_data/diff_file'; import discussionsMockData from '../mock_data/diff_discussions'; @@ -14,58 +13,13 @@ describe('InlineDiffView', () => { beforeEach(() => { const diffFile = getDiffFileMock(); + store.dispatch('setInlineDiffViewType'); component = createComponentWithStore(Vue.extend(InlineDiffView), store, { diffFile, diffLines: diffFile.highlightedDiffLines, }).$mount(); }); - describe('methods', () => { - describe('handleMouse', () => { - it('should set hoveredLineCode', () => { - expect(component.hoveredLineCode).toEqual(null); - - component.handleMouse('lineCode1', true); - expect(component.hoveredLineCode).toEqual('lineCode1'); - - component.handleMouse('lineCode1', false); - expect(component.hoveredLineCode).toEqual(null); - }); - }); - - describe('getLineClass', () => { - it('should return line class object', () => { - const { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME } = constants; - const { MATCH_LINE_TYPE, NEW_LINE_TYPE } = constants; - - expect(component.getLineClass(component.diffLines[0])).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: false, - }); - - component.handleMouse(component.diffLines[0].lineCode, true); - Object.defineProperty(component, 'isLoggedIn', { - get() { - return true; - }, - }); - - expect(component.getLineClass(component.diffLines[0])).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: true, - }); - - expect(component.getLineClass(component.diffLines[5])).toEqual({ - [MATCH_LINE_TYPE]: MATCH_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: true, - [LINE_HOVER_CLASS_NAME]: false, - }); - }); - }); - }); - describe('template', () => { it('should have rendered diff lines', () => { const el = component.$el; @@ -89,23 +43,5 @@ describe('InlineDiffView', () => { done(); }); }); - - it('should render new discussion forms', done => { - const el = component.$el; - const lines = getDiffFileMock().highlightedDiffLines; - - component.handleShowCommentForm({ lineCode: lines[0].lineCode }); - component.handleShowCommentForm({ lineCode: lines[1].lineCode }); - - Vue.nextTick(() => { - expect(el.querySelectorAll('.js-vue-markdown-field').length).toEqual(2); - expect(el.querySelectorAll('tr')[1].classList.contains('notes_holder')).toEqual(true); - expect(el.querySelectorAll('tr')[3].classList.contains('notes_holder')).toEqual(true); - - store.state.diffs.diffLineCommentForms = {}; - - done(); - }); - }); }); }); diff --git a/spec/javascripts/diffs/components/parallel_diff_view_spec.js b/spec/javascripts/diffs/components/parallel_diff_view_spec.js index cab533217c0..165e4b69b6c 100644 --- a/spec/javascripts/diffs/components/parallel_diff_view_spec.js +++ b/spec/javascripts/diffs/components/parallel_diff_view_spec.js @@ -4,12 +4,10 @@ import store from '~/mr_notes/stores'; import * as constants from '~/diffs/constants'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import diffFileMockData from '../mock_data/diff_file'; -import discussionsMockData from '../mock_data/diff_discussions'; describe('ParallelDiffView', () => { let component; const getDiffFileMock = () => Object.assign({}, diffFileMockData); - const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; beforeEach(() => { const diffFile = getDiffFileMock(); @@ -28,197 +26,4 @@ describe('ParallelDiffView', () => { }); }); }); - - describe('methods', () => { - describe('hasDiscussion', () => { - it('it should return true if there is a discussion either for left or right section', () => { - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { line_42: true }; - }, - }); - - expect(component.hasDiscussion({ left: {}, right: {} })).toEqual(undefined); - expect(component.hasDiscussion({ left: { lineCode: 'line_42' }, right: {} })).toEqual(true); - expect(component.hasDiscussion({ left: {}, right: { lineCode: 'line_42' } })).toEqual(true); - }); - }); - - describe('getClassName', () => { - it('should return line class object', () => { - const { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME } = constants; - const { MATCH_LINE_TYPE, NEW_LINE_TYPE, LINE_POSITION_RIGHT } = constants; - - expect(component.getClassName(component.diffLines[1], LINE_POSITION_RIGHT)).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: false, - }); - - const eventMock = { - target: component.$refs.rightLines[1], - }; - - component.handleMouse(eventMock, component.diffLines[1], true); - Object.defineProperty(component, 'isLoggedIn', { - get() { - return true; - }, - }); - - expect(component.getClassName(component.diffLines[1], LINE_POSITION_RIGHT)).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: true, - }); - - expect(component.getClassName(component.diffLines[5], LINE_POSITION_RIGHT)).toEqual({ - [MATCH_LINE_TYPE]: MATCH_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: true, - [LINE_HOVER_CLASS_NAME]: false, - }); - }); - }); - - describe('handleMouse', () => { - it('should set hovered line code and line section to null when isHover is false', () => { - const rightLineEventMock = { target: component.$refs.rightLines[1] }; - expect(component.hoveredLineCode).toEqual(null); - expect(component.hoveredSection).toEqual(null); - - component.handleMouse(rightLineEventMock, null, false); - expect(component.hoveredLineCode).toEqual(null); - expect(component.hoveredSection).toEqual(null); - }); - - it('should set hovered line code and line section for right section', () => { - const rightLineEventMock = { target: component.$refs.rightLines[1] }; - component.handleMouse(rightLineEventMock, component.diffLines[1], true); - expect(component.hoveredLineCode).toEqual(component.diffLines[1].right.lineCode); - expect(component.hoveredSection).toEqual(constants.LINE_POSITION_RIGHT); - }); - - it('should set hovered line code and line section for left section', () => { - const leftLineEventMock = { target: component.$refs.leftLines[2] }; - component.handleMouse(leftLineEventMock, component.diffLines[2], true); - expect(component.hoveredLineCode).toEqual(component.diffLines[2].left.lineCode); - expect(component.hoveredSection).toEqual(constants.LINE_POSITION_LEFT); - }); - }); - - describe('shouldRenderDiscussions', () => { - it('should return true if there is a discussion on left side and it is expanded', () => { - const line = { left: { lineCode: 'lineCode1' } }; - spyOn(component, 'isDiscussionExpanded').and.returnValue(true); - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { - [line.left.lineCode]: true, - }; - }, - }); - - expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_LEFT)).toEqual(true); - expect(component.isDiscussionExpanded).toHaveBeenCalledWith(line.left.lineCode); - }); - - it('should return false if there is a discussion on left side but it is collapsed', () => { - const line = { left: { lineCode: 'lineCode1' } }; - spyOn(component, 'isDiscussionExpanded').and.returnValue(false); - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { - [line.left.lineCode]: true, - }; - }, - }); - - expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_LEFT)).toEqual( - false, - ); - }); - - it('should return false for discussions on the right side if there is no line type', () => { - const CUSTOM_RIGHT_LINE_TYPE = 'CUSTOM_RIGHT_LINE_TYPE'; - const line = { right: { lineCode: 'lineCode1', type: CUSTOM_RIGHT_LINE_TYPE } }; - spyOn(component, 'isDiscussionExpanded').and.returnValue(true); - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { - [line.right.lineCode]: true, - }; - }, - }); - - expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_RIGHT)).toEqual( - CUSTOM_RIGHT_LINE_TYPE, - ); - }); - }); - - describe('hasAnyExpandedDiscussion', () => { - const LINE_CODE_LEFT = 'LINE_CODE_LEFT'; - const LINE_CODE_RIGHT = 'LINE_CODE_RIGHT'; - - it('should return true if there is a discussion either on the left or the right side', () => { - const mockLineOne = { - right: { lineCode: LINE_CODE_RIGHT }, - left: {}, - }; - const mockLineTwo = { - left: { lineCode: LINE_CODE_LEFT }, - right: {}, - }; - - spyOn(component, 'isDiscussionExpanded').and.callFake(lc => lc === LINE_CODE_RIGHT); - expect(component.hasAnyExpandedDiscussion(mockLineOne)).toEqual(true); - expect(component.hasAnyExpandedDiscussion(mockLineTwo)).toEqual(false); - }); - }); - }); - - describe('template', () => { - it('should have rendered diff lines', () => { - const el = component.$el; - - expect(el.querySelectorAll('tr.line_holder.parallel').length).toEqual(6); - expect(el.querySelectorAll('td.empty-cell').length).toEqual(4); - expect(el.querySelectorAll('td.line_content.parallel.right-side').length).toEqual(6); - expect(el.querySelectorAll('td.line_content.parallel.left-side').length).toEqual(6); - expect(el.querySelectorAll('td.match').length).toEqual(4); - expect(el.textContent.indexOf('Bad dates') > -1).toEqual(true); - }); - - it('should render discussions', done => { - const el = component.$el; - component.$store.dispatch('setInitialNotes', getDiscussionsMockData()); - - Vue.nextTick(() => { - expect(el.querySelectorAll('.notes_holder').length).toEqual(1); - expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(5); - expect(el.innerText.indexOf('comment 5') > -1).toEqual(true); - component.$store.dispatch('setInitialNotes', []); - - done(); - }); - }); - - it('should render new discussion forms', done => { - const el = component.$el; - const lines = getDiffFileMock().parallelDiffLines; - - component.handleShowCommentForm({ lineCode: lines[0].lineCode }); - component.handleShowCommentForm({ lineCode: lines[1].lineCode }); - - Vue.nextTick(() => { - expect(el.querySelectorAll('.js-vue-markdown-field').length).toEqual(2); - expect(el.querySelectorAll('tr')[1].classList.contains('notes_holder')).toEqual(true); - expect(el.querySelectorAll('tr')[3].classList.contains('notes_holder')).toEqual(true); - - store.state.diffs.diffLineCommentForms = {}; - - done(); - }); - }); - }); }); |