diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
18 files changed, 392 insertions, 325 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index b5b05df4d34..4261a99c52b 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -59,7 +59,7 @@ export default { emailPatchPath: state => state.diffs.emailPatchPath, }), ...mapGetters('diffs', ['isParallelView']), - ...mapGetters(['isNotesFetched']), + ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), targetBranch() { return { branchName: this.targetBranchName, @@ -112,13 +112,26 @@ export default { }, created() { this.adjustView(); + eventHub.$once('fetchedNotesData', this.setDiscussions); }, methods: { - ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles', 'startRenderDiffsQueue']), + ...mapActions('diffs', [ + 'setBaseConfig', + 'fetchDiffFiles', + 'startRenderDiffsQueue', + 'assignDiscussionsToDiff', + ]), + fetchData() { this.fetchDiffFiles() .then(() => { - requestIdleCallback(this.startRenderDiffsQueue, { timeout: 1000 }); + requestIdleCallback( + () => { + this.setDiscussions(); + this.startRenderDiffsQueue(); + }, + { timeout: 1000 }, + ); }) .catch(() => { createFlash(__('Something went wrong on our end. Please try again!')); @@ -128,6 +141,16 @@ export default { eventHub.$emit('fetchNotesData'); } }, + setDiscussions() { + if (this.isNotesFetched) { + requestIdleCallback( + () => { + this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); + }, + { timeout: 1000 }, + ); + } + }, adjustView() { if (this.shouldShow && this.isParallelView) { window.mrTabs.expandViewContainer(); diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index e64d5511d78..cddbe554fbd 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -1,4 +1,5 @@ <script> +import { mapActions } from 'vuex'; import noteableDiscussion from '../../notes/components/noteable_discussion.vue'; export default { @@ -11,6 +12,14 @@ export default { required: true, }, }, + methods: { + ...mapActions('diffs', ['removeDiscussionsFromDiff']), + deleteNoteHandler(discussion) { + if (discussion.notes.length <= 1) { + this.removeDiscussionsFromDiff(discussion); + } + }, + }, }; </script> @@ -31,6 +40,7 @@ export default { :render-diff-file="false" :always-expanded="true" :discussions-by-diff-order="true" + @noteDeleted="deleteNoteHandler" /> </ul> </div> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 59e9ba08b8b..67e85c4eee3 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -1,5 +1,5 @@ <script> -import { mapActions } from 'vuex'; +import { mapActions, mapGetters } from 'vuex'; import _ from 'underscore'; import { __, sprintf } from '~/locale'; import createFlash from '~/flash'; @@ -30,6 +30,7 @@ export default { }; }, computed: { + ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), isCollapsed() { return this.file.collapsed || false; }, @@ -44,23 +45,23 @@ export default { ); }, showExpandMessage() { - return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge; + return ( + !this.isCollapsed && + !this.file.highlightedDiffLines && + !this.isLoadingCollapsedDiff && + !this.file.tooLarge && + this.file.text + ); }, showLoadingIcon() { return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed); }, }, methods: { - ...mapActions('diffs', ['loadCollapsedDiff']), + ...mapActions('diffs', ['loadCollapsedDiff', 'assignDiscussionsToDiff']), handleToggle() { - const { collapsed, highlightedDiffLines, parallelDiffLines } = this.file; - - if ( - collapsed && - !highlightedDiffLines && - parallelDiffLines !== undefined && - !parallelDiffLines.length - ) { + const { highlightedDiffLines, parallelDiffLines } = this.file; + if (!highlightedDiffLines && parallelDiffLines !== undefined && !parallelDiffLines.length) { this.handleLoadCollapsedDiff(); } else { this.file.collapsed = !this.file.collapsed; @@ -76,6 +77,14 @@ export default { this.file.collapsed = false; this.file.renderIt = true; }) + .then(() => { + requestIdleCallback( + () => { + this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); + }, + { timeout: 1000 }, + ); + }) .catch(() => { this.isLoadingCollapsedDiff = false; createFlash(__('Something went wrong on our end. Please try again!')); @@ -136,11 +145,11 @@ export default { :diff-file="file" /> <loading-icon - v-else-if="showLoadingIcon" + v-if="showLoadingIcon" class="diff-content loading" /> <div - v-if="showExpandMessage" + v-else-if="showExpandMessage" class="nothing-here-block diff-collapsed" > {{ __('This diff is collapsed.') }} diff --git a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue index 7e50a0aed84..a02c41f39ab 100644 --- a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue +++ b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue @@ -1,15 +1,11 @@ <script> import { mapActions } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; -import tooltip from '~/vue_shared/directives/tooltip'; import { pluralize, truncate } from '~/lib/utils/text_utility'; import UserAvatarImage from '~/vue_shared/components/user_avatar/user_avatar_image.vue'; import { COUNT_OF_AVATARS_IN_GUTTER, LENGTH_OF_AVATAR_TOOLTIP } from '../constants'; export default { - directives: { - tooltip, - }, components: { Icon, UserAvatarImage, @@ -91,10 +87,10 @@ export default { @click.native="toggleDiscussions" /> <span - v-tooltip + v-gl-tooltip v-if="moreText" :title="moreText" - class="diff-comments-more-count has-tooltip js-diff-comment-avatar js-diff-comment-plus" + class="diff-comments-more-count js-diff-comment-avatar js-diff-comment-plus" data-container="body" data-placement="top" role="button" 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 8ad1ea34245..6eff3013dcd 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -13,6 +13,10 @@ export default { Icon, }, props: { + line: { + type: Object, + required: true, + }, fileHash: { type: String, required: true, @@ -21,31 +25,16 @@ export default { type: String, required: true, }, - lineType: { - type: String, - required: false, - default: '', - }, lineNumber: { type: Number, required: false, default: 0, }, - lineCode: { - type: String, - required: false, - default: '', - }, linePosition: { type: String, required: false, default: '', }, - metaData: { - type: Object, - required: false, - default: () => ({}), - }, showCommentButton: { type: Boolean, required: false, @@ -76,11 +65,6 @@ export default { required: false, default: false, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapState({ @@ -89,7 +73,7 @@ export default { }), ...mapGetters(['isLoggedIn']), lineHref() { - return this.lineCode ? `#${this.lineCode}` : '#'; + return `#${this.line.lineCode || ''}`; }, shouldShowCommentButton() { return ( @@ -103,20 +87,19 @@ export default { ); }, hasDiscussions() { - return this.discussions.length > 0; + return this.line.discussions && this.line.discussions.length > 0; }, shouldShowAvatarsOnGutter() { - if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) { + if (!this.line.type && this.linePosition === LINE_POSITION_RIGHT) { return false; } - return this.showCommentButton && this.hasDiscussions; }, }, methods: { ...mapActions('diffs', ['loadMoreLines', 'showCommentForm']), handleCommentButton() { - this.showCommentForm({ lineCode: this.lineCode }); + this.showCommentForm({ lineCode: this.line.lineCode }); }, handleLoadMoreLines() { if (this.isRequesting) { @@ -125,8 +108,8 @@ export default { this.isRequesting = true; const endpoint = this.contextLinesPath; - const oldLineNumber = this.metaData.oldPos || 0; - const newLineNumber = this.metaData.newPos || 0; + const oldLineNumber = this.line.metaData.oldPos || 0; + const newLineNumber = this.line.metaData.newPos || 0; const offset = newLineNumber - oldLineNumber; const bottom = this.isBottom; const { fileHash } = this; @@ -201,7 +184,7 @@ export default { </a> <diff-gutter-avatars v-if="shouldShowAvatarsOnGutter" - :discussions="discussions" + :discussions="line.discussions" /> </template> </div> diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue index cbe4551d06b..a0dc381ccc7 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -6,6 +6,7 @@ import noteForm from '../../notes/components/note_form.vue'; import { getNoteFormData } from '../store/utils'; import autosave from '../../notes/mixins/autosave'; import { DIFF_NOTE_TYPE } from '../constants'; +import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils'; export default { components: { @@ -52,7 +53,7 @@ export default { } }, methods: { - ...mapActions('diffs', ['cancelCommentForm']), + ...mapActions('diffs', ['cancelCommentForm', 'assignDiscussionsToDiff']), ...mapActions(['saveNote', 'refetchDiscussionById']), handleCancelCommentForm(shouldConfirm, isDirty) { if (shouldConfirm && isDirty) { @@ -88,7 +89,10 @@ export default { const endpoint = this.getNotesDataByProp('discussionsPath'); this.refetchDiscussionById({ path: endpoint, discussionId: result.discussion_id }) - .then(() => { + .then(selectedDiscussion => { + const lineCodeDiscussions = reduceDiscussionsToLineCodes([selectedDiscussion]); + this.assignDiscussionsToDiff(lineCodeDiscussions); + this.handleCancelCommentForm(); }) .catch(() => { diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index 33bc8d9971e..5d9a0b123fe 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -11,8 +11,6 @@ import { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME, INLINE_DIFF_VIEW_TYPE, - LINE_POSITION_LEFT, - LINE_POSITION_RIGHT, } from '../constants'; export default { @@ -67,42 +65,24 @@ export default { required: false, default: false, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapGetters(['isLoggedIn']), - normalizedLine() { - let normalizedLine; - - if (this.diffViewType === INLINE_DIFF_VIEW_TYPE) { - normalizedLine = this.line; - } else if (this.linePosition === LINE_POSITION_LEFT) { - normalizedLine = this.line.left; - } else if (this.linePosition === LINE_POSITION_RIGHT) { - normalizedLine = this.line.right; - } - - return normalizedLine; - }, isMatchLine() { - return this.normalizedLine.type === MATCH_LINE_TYPE; + return this.line.type === MATCH_LINE_TYPE; }, isContextLine() { - return this.normalizedLine.type === CONTEXT_LINE_TYPE; + return this.line.type === CONTEXT_LINE_TYPE; }, isMetaLine() { - const { type } = this.normalizedLine; + const { type } = this.line; return ( type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE || type === EMPTY_CELL_TYPE ); }, classNameMap() { - const { type } = this.normalizedLine; + const { type } = this.line; return { [type]: type, @@ -116,9 +96,9 @@ export default { }; }, lineNumber() { - const { lineType, normalizedLine } = this; + const { lineType } = this; - return lineType === OLD_LINE_TYPE ? normalizedLine.oldLine : normalizedLine.newLine; + return lineType === OLD_LINE_TYPE ? this.line.oldLine : this.line.newLine; }, }, }; @@ -129,20 +109,17 @@ export default { :class="classNameMap" > <diff-line-gutter-content + :line="line" :file-hash="fileHash" :context-lines-path="contextLinesPath" - :line-type="normalizedLine.type" - :line-code="normalizedLine.lineCode" :line-position="linePosition" :line-number="lineNumber" - :meta-data="normalizedLine.metaData" :show-comment-button="showCommentButton" :is-hover="isHover" :is-bottom="isBottom" :is-match-line="isMatchLine" :is-context-line="isContentLine" :is-meta-line="isMetaLine" - :discussions="discussions" /> </td> </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 index 6348f32d36d..46a51859da5 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue @@ -21,18 +21,13 @@ export default { type: Number, required: true, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), className() { - return this.discussions.length ? '' : 'js-temp-notes-holder'; + return this.line.discussions.length ? '' : 'js-temp-notes-holder'; }, }, }; @@ -49,8 +44,8 @@ export default { > <div class="content"> <diff-discussions - v-if="discussions.length" - :discussions="discussions" + v-if="line.discussions.length" + :discussions="line.discussions" /> <diff-line-note-form v-if="diffLineCommentForms[line.lineCode]" diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue index 32d65ff994f..0e306f39a9f 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -33,11 +33,6 @@ export default { required: false, default: false, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, data() { return { @@ -94,7 +89,6 @@ export default { :is-bottom="isBottom" :is-hover="isHover" :show-comment-button="true" - :discussions="discussions" class="diff-line-num old_line" /> <diff-table-cell @@ -104,7 +98,6 @@ export default { :line-type="newLineType" :is-bottom="isBottom" :is-hover="isHover" - :discussions="discussions" class="diff-line-num new_line" /> <td diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index e7d789734c3..947e7c98fae 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -2,7 +2,6 @@ import { mapGetters, mapState } from 'vuex'; import inlineDiffTableRow from './inline_diff_table_row.vue'; import inlineDiffCommentRow from './inline_diff_comment_row.vue'; -import { trimFirstCharOfLineContent } from '../store/utils'; export default { components: { @@ -20,29 +19,17 @@ export default { }, }, computed: { - ...mapGetters('diffs', [ - 'commitId', - 'shouldRenderInlineCommentRow', - 'singleDiscussionByLineCode', - ]), + ...mapGetters('diffs', ['commitId', 'shouldRenderInlineCommentRow']), ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), - normalizedDiffLines() { - return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line)); - }, diffLinesLength() { - return this.normalizedDiffLines.length; + return this.diffLines.length; }, userColorScheme() { return window.gon.user_color_scheme; }, }, - methods: { - discussionsList(line) { - return line.lineCode !== undefined ? this.singleDiscussionByLineCode(line.lineCode) : []; - }, - }, }; </script> @@ -53,7 +40,7 @@ export default { class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view"> <tbody> <template - v-for="(line, index) in normalizedDiffLines" + v-for="(line, index) in diffLines" > <inline-diff-table-row :file-hash="diffFile.fileHash" @@ -61,7 +48,6 @@ export default { :line="line" :is-bottom="index + 1 === diffLinesLength" :key="line.lineCode" - :discussions="discussionsList(line)" /> <inline-diff-comment-row v-if="shouldRenderInlineCommentRow(line)" @@ -69,7 +55,6 @@ export default { :line="line" :line-index="index" :key="index" - :discussions="discussionsList(line)" /> </template> </tbody> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue index 48b8feeb0b4..26417c350cb 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -21,51 +21,49 @@ export default { type: Number, required: true, }, - leftDiscussions: { - type: Array, - required: false, - default: () => [], - }, - rightDiscussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), leftLineCode() { - return this.line.left.lineCode; + return this.line.left && this.line.left.lineCode; }, rightLineCode() { - return this.line.right.lineCode; + return this.line.right && this.line.right.lineCode; }, hasExpandedDiscussionOnLeft() { - const discussions = this.leftDiscussions; - - return discussions ? discussions.every(discussion => discussion.expanded) : false; + return this.line.left && this.line.left.discussions + ? this.line.left.discussions.every(discussion => discussion.expanded) + : false; }, hasExpandedDiscussionOnRight() { - const discussions = this.rightDiscussions; - - return discussions ? discussions.every(discussion => discussion.expanded) : false; + return this.line.right && this.line.right.discussions + ? this.line.right.discussions.every(discussion => discussion.expanded) + : false; }, hasAnyExpandedDiscussion() { return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; }, shouldRenderDiscussionsOnLeft() { - return this.leftDiscussions && this.hasExpandedDiscussionOnLeft; + return this.line.left && this.line.left.discussions && this.hasExpandedDiscussionOnLeft; }, shouldRenderDiscussionsOnRight() { - return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type; + return ( + this.line.right && + this.line.right.discussions && + this.hasExpandedDiscussionOnRight && + this.line.right.type + ); }, showRightSideCommentForm() { - return this.line.right.type && this.diffLineCommentForms[this.rightLineCode]; + return ( + this.line.right && this.line.right.type && this.diffLineCommentForms[this.rightLineCode] + ); }, className() { - return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0 + return (this.left && this.line.left.discussions.length > 0) || + (this.right && this.line.right.discussions.length > 0) ? '' : 'js-temp-notes-holder'; }, @@ -85,8 +83,8 @@ export default { class="content" > <diff-discussions - v-if="leftDiscussions.length" - :discussions="leftDiscussions" + v-if="line.left.discussions.length" + :discussions="line.left.discussions" /> </div> <diff-line-note-form @@ -104,8 +102,8 @@ export default { class="content" > <diff-discussions - v-if="rightDiscussions.length" - :discussions="rightDiscussions" + v-if="line.right.discussions.length" + :discussions="line.right.discussions" /> </div> <diff-line-note-form diff --git a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue index d4e54c2bd00..fb68d191091 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -1,6 +1,5 @@ <script> import $ from 'jquery'; -import { mapGetters } from 'vuex'; import DiffTableCell from './diff_table_cell.vue'; import { NEW_LINE_TYPE, @@ -10,8 +9,7 @@ import { OLD_NO_NEW_LINE_TYPE, PARALLEL_DIFF_VIEW_TYPE, NEW_NO_NEW_LINE_TYPE, - LINE_POSITION_LEFT, - LINE_POSITION_RIGHT, + EMPTY_CELL_TYPE, } from '../constants'; export default { @@ -36,16 +34,6 @@ export default { required: false, default: false, }, - leftDiscussions: { - type: Array, - required: false, - default: () => [], - }, - rightDiscussions: { - type: Array, - required: false, - default: () => [], - }, }, data() { return { @@ -54,29 +42,26 @@ export default { }; }, computed: { - ...mapGetters('diffs', ['isParallelView']), isContextLine() { - return this.line.left.type === CONTEXT_LINE_TYPE; + return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE; }, classNameMap() { return { [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, - [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView, + [PARALLEL_DIFF_VIEW_TYPE]: true, }; }, parallelViewLeftLineType() { - if (this.line.right.type === NEW_NO_NEW_LINE_TYPE) { + if (this.line.right && this.line.right.type === NEW_NO_NEW_LINE_TYPE) { return OLD_NO_NEW_LINE_TYPE; } - return this.line.left.type; + return this.line.left ? this.line.left.type : EMPTY_CELL_TYPE; }, }, created() { this.newLineType = NEW_LINE_TYPE; this.oldLineType = OLD_LINE_TYPE; - this.linePositionLeft = LINE_POSITION_LEFT; - this.linePositionRight = LINE_POSITION_RIGHT; this.parallelDiffViewType = PARALLEL_DIFF_VIEW_TYPE; }, methods: { @@ -116,47 +101,57 @@ export default { @mouseover="handleMouseMove" @mouseout="handleMouseMove" > - <diff-table-cell - :file-hash="fileHash" - :context-lines-path="contextLinesPath" - :line="line" - :line-type="oldLineType" - :line-position="linePositionLeft" - :is-bottom="isBottom" - :is-hover="isLeftHover" - :show-comment-button="true" - :diff-view-type="parallelDiffViewType" - :discussions="leftDiscussions" - class="diff-line-num old_line" - /> - <td - :id="line.left.lineCode" - :class="parallelViewLeftLineType" - class="line_content parallel left-side" - @mousedown.native="handleParallelLineMouseDown" - v-html="line.left.richText" - > - </td> - <diff-table-cell - :file-hash="fileHash" - :context-lines-path="contextLinesPath" - :line="line" - :line-type="newLineType" - :line-position="linePositionRight" - :is-bottom="isBottom" - :is-hover="isRightHover" - :show-comment-button="true" - :diff-view-type="parallelDiffViewType" - :discussions="rightDiscussions" - class="diff-line-num new_line" - /> - <td - :id="line.right.lineCode" - :class="line.right.type" - class="line_content parallel right-side" - @mousedown.native="handleParallelLineMouseDown" - v-html="line.right.richText" - > - </td> + <template v-if="line.left"> + <diff-table-cell + :file-hash="fileHash" + :context-lines-path="contextLinesPath" + :line="line.left" + :line-type="oldLineType" + :is-bottom="isBottom" + :is-hover="isLeftHover" + :show-comment-button="true" + :diff-view-type="parallelDiffViewType" + line-position="left" + class="diff-line-num old_line" + /> + <td + :id="line.left.lineCode" + :class="parallelViewLeftLineType" + class="line_content parallel left-side" + @mousedown.native="handleParallelLineMouseDown" + v-html="line.left.richText" + > + </td> + </template> + <template v-else> + <td class="diff-line-num old_line empty-cell"></td> + <td class="line_content parallel left-side empty-cell"></td> + </template> + <template v-if="line.right"> + <diff-table-cell + :file-hash="fileHash" + :context-lines-path="contextLinesPath" + :line="line.right" + :line-type="newLineType" + :is-bottom="isBottom" + :is-hover="isRightHover" + :show-comment-button="true" + :diff-view-type="parallelDiffViewType" + line-position="right" + class="diff-line-num new_line" + /> + <td + :id="line.right.lineCode" + :class="line.right.type" + class="line_content parallel right-side" + @mousedown.native="handleParallelLineMouseDown" + v-html="line.right.richText" + > + </td> + </template> + <template v-else> + <td class="diff-line-num old_line empty-cell"></td> + <td class="line_content parallel right-side empty-cell"></td> + </template> </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 24ceb52a04a..501bd4450d8 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -2,8 +2,6 @@ import { mapState, mapGetters } from 'vuex'; import parallelDiffTableRow from './parallel_diff_table_row.vue'; import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; -import { EMPTY_CELL_TYPE } from '../constants'; -import { trimFirstCharOfLineContent } from '../store/utils'; export default { components: { @@ -21,46 +19,17 @@ export default { }, }, computed: { - ...mapGetters('diffs', [ - 'commitId', - 'singleDiscussionByLineCode', - 'shouldRenderParallelCommentRow', - ]), + ...mapGetters('diffs', ['commitId', 'shouldRenderParallelCommentRow']), ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), - parallelDiffLines() { - return this.diffLines.map(line => { - const parallelLine = Object.assign({}, line); - - if (line.left) { - parallelLine.left = trimFirstCharOfLineContent(line.left); - } else { - parallelLine.left = { type: EMPTY_CELL_TYPE }; - } - - if (line.right) { - parallelLine.right = trimFirstCharOfLineContent(line.right); - } else { - parallelLine.right = { type: EMPTY_CELL_TYPE }; - } - - return parallelLine; - }); - }, diffLinesLength() { - return this.parallelDiffLines.length; + return this.diffLines.length; }, userColorScheme() { return window.gon.user_color_scheme; }, }, - methods: { - discussionsByLine(line, leftOrRight) { - return line[leftOrRight] && line[leftOrRight].lineCode !== undefined ? - this.singleDiscussionByLineCode(line[leftOrRight].lineCode) : []; - }, - }, }; </script> @@ -73,7 +42,7 @@ export default { <table> <tbody> <template - v-for="(line, index) in parallelDiffLines" + v-for="(line, index) in diffLines" > <parallel-diff-table-row :file-hash="diffFile.fileHash" @@ -81,8 +50,6 @@ export default { :line="line" :is-bottom="index + 1 === diffLinesLength" :key="index" - :left-discussions="discussionsByLine(line, 'left')" - :right-discussions="discussionsByLine(line, 'right')" /> <parallel-diff-comment-row v-if="shouldRenderParallelCommentRow(line)" @@ -90,8 +57,6 @@ export default { :line="line" :diff-file-hash="diffFile.fileHash" :line-index="index" - :left-discussions="discussionsByLine(line, 'left')" - :right-discussions="discussionsByLine(line, 'right')" /> </template> </tbody> diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 4ab6ceb249a..027df2ec841 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -3,6 +3,7 @@ import axios from '~/lib/utils/axios_utils'; import Cookies from 'js-cookie'; import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils'; import { mergeUrlParams } from '~/lib/utils/url_utility'; +import { getDiffPositionByLineCode } from './utils'; import * as types from './mutation_types'; import { PARALLEL_DIFF_VIEW_TYPE, @@ -29,25 +30,53 @@ export const fetchDiffFiles = ({ state, commit }) => { .then(handleLocationHash); }; -export const startRenderDiffsQueue = ({ state, commit }) => { - const checkItem = () => { - const nextFile = state.diffFiles.find( - file => !file.renderIt && (!file.collapsed || !file.text), - ); - if (nextFile) { - requestAnimationFrame(() => { - commit(types.RENDER_FILE, nextFile); +// This is adding line discussions to the actual lines in the diff tree +// once for parallel and once for inline mode +export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => { + const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles); + + Object.values(allLineDiscussions).forEach(discussions => { + if (discussions.length > 0) { + const { fileHash } = discussions[0]; + commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { + fileHash, + discussions, + diffPositionByLineCode, }); - requestIdleCallback( - () => { - checkItem(); - }, - { timeout: 1000 }, - ); } - }; + }); +}; + +export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => { + const { fileHash, line_code } = removeDiscussion; + commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code }); +}; + +export const startRenderDiffsQueue = ({ state, commit }) => { + const checkItem = () => + new Promise(resolve => { + const nextFile = state.diffFiles.find( + file => !file.renderIt && (!file.collapsed || !file.text), + ); + + if (nextFile) { + requestAnimationFrame(() => { + commit(types.RENDER_FILE, nextFile); + }); + requestIdleCallback( + () => { + checkItem() + .then(resolve) + .catch(() => {}); + }, + { timeout: 1000 }, + ); + } else { + resolve(); + } + }); - checkItem(); + return checkItem(); }; export const setInlineDiffViewType = ({ commit }) => { diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 4a47646d7fa..968ba3c5e13 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -17,7 +17,10 @@ export const commitId = state => (state.commit && state.commit.id ? state.commit export const diffHasAllExpandedDiscussions = (state, getters) => diff => { const discussions = getters.getDiffFileDiscussions(diff); - return (discussions.length && discussions.every(discussion => discussion.expanded)) || false; + return ( + (discussions && discussions.length && discussions.every(discussion => discussion.expanded)) || + false + ); }; /** @@ -28,7 +31,10 @@ export const diffHasAllExpandedDiscussions = (state, getters) => diff => { export const diffHasAllCollpasedDiscussions = (state, getters) => diff => { const discussions = getters.getDiffFileDiscussions(diff); - return (discussions.length && discussions.every(discussion => !discussion.expanded)) || false; + return ( + (discussions && discussions.length && discussions.every(discussion => !discussion.expanded)) || + false + ); }; /** @@ -40,7 +46,9 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => { const discussions = getters.getDiffFileDiscussions(diff); return ( - (discussions.length && discussions.find(discussion => discussion.expanded) !== undefined) || + (discussions && + discussions.length && + discussions.find(discussion => discussion.expanded) !== undefined) || false ); }; @@ -64,45 +72,38 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) = discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash), ) || []; -export const singleDiscussionByLineCode = (state, getters, rootState, rootGetters) => lineCode => { - if (!lineCode || lineCode === undefined) return []; - const discussions = rootGetters.discussionsByLineCode; - return discussions[lineCode] || []; -}; - -export const shouldRenderParallelCommentRow = (state, getters) => line => { - const leftLineCode = line.left.lineCode; - const rightLineCode = line.right.lineCode; - const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode); - const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode); - const hasDiscussion = leftDiscussions.length || rightDiscussions.length; +export const shouldRenderParallelCommentRow = state => line => { + const hasDiscussion = + (line.left && line.left.discussions && line.left.discussions.length) || + (line.right && line.right.discussions && line.right.discussions.length); - const hasExpandedDiscussionOnLeft = leftDiscussions.length - ? leftDiscussions.every(discussion => discussion.expanded) - : false; - const hasExpandedDiscussionOnRight = rightDiscussions.length - ? rightDiscussions.every(discussion => discussion.expanded) - : false; + const hasExpandedDiscussionOnLeft = + line.left && line.left.discussions && line.left.discussions.length + ? line.left.discussions.every(discussion => discussion.expanded) + : false; + const hasExpandedDiscussionOnRight = + line.right && line.right.discussions && line.right.discussions.length + ? line.right.discussions.every(discussion => discussion.expanded) + : false; if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) { return true; } - const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode]; - const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode]; + const hasCommentFormOnLeft = line.left && state.diffLineCommentForms[line.left.lineCode]; + const hasCommentFormOnRight = line.right && state.diffLineCommentForms[line.right.lineCode]; return hasCommentFormOnLeft || hasCommentFormOnRight; }; -export const shouldRenderInlineCommentRow = (state, getters) => line => { +export const shouldRenderInlineCommentRow = state => line => { if (state.diffLineCommentForms[line.lineCode]) return true; - const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode); - if (lineDiscussions.length === 0) { + if (!line.discussions || line.discussions.length === 0) { return false; } - return lineDiscussions.every(discussion => discussion.expanded); + return line.discussions.every(discussion => discussion.expanded); }; // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index c999d637d50..f61efbe6e1e 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -9,3 +9,5 @@ export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES'; export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS'; export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES'; export const RENDER_FILE = 'RENDER_FILE'; +export const SET_LINE_DISCUSSIONS_FOR_FILE = 'SET_LINE_DISCUSSIONS_FOR_FILE'; +export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FILE'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index bc69ae30777..6dc5bf16c65 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -1,8 +1,13 @@ import Vue from 'vue'; -import _ from 'underscore'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; -import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils'; -import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants'; +import { + findDiffFile, + addLineReferences, + removeMatchLine, + addContextLines, + prepareDiffData, + isDiscussionApplicableToLine, +} from './utils'; import * as types from './mutation_types'; export default { @@ -17,38 +22,7 @@ export default { [types.SET_DIFF_DATA](state, data) { const diffData = convertObjectPropsToCamelCase(data, { deep: true }); - let showingLines = 0; - const filesLength = diffData.diffFiles.length; - let i; - for (i = 0; i < filesLength; i += 1) { - const file = diffData.diffFiles[i]; - if (file.parallelDiffLines) { - const linesLength = file.parallelDiffLines.length; - let u = 0; - for (u = 0; u < linesLength; u += 1) { - const line = file.parallelDiffLines[u]; - if (line.left) delete line.left.text; - if (line.right) delete line.right.text; - } - } - - if (file.highlightedDiffLines) { - const linesLength = file.highlightedDiffLines.length; - let u; - for (u = 0; u < linesLength; u += 1) { - const line = file.highlightedDiffLines[u]; - delete line.text; - } - } - - if (file.highlightedDiffLines) { - showingLines += file.parallelDiffLines.length; - } - Object.assign(file, { - renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, - collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, - }); - } + prepareDiffData(diffData); Object.assign(state, { ...diffData, @@ -98,12 +72,10 @@ export default { [types.ADD_COLLAPSED_DIFFS](state, { file, data }) { const normalizedData = convertObjectPropsToCamelCase(data, { deep: true }); + prepareDiffData(normalizedData); const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash); - - if (newFileData) { - const index = _.findIndex(state.diffFiles, f => f.fileHash === file.fileHash); - state.diffFiles.splice(index, 1, newFileData); - } + const selectedFile = state.diffFiles.find(f => f.fileHash === file.fileHash); + Object.assign(selectedFile, { ...newFileData }); }, [types.EXPAND_ALL_FILES](state) { @@ -112,4 +84,81 @@ export default { collapsed: false, })); }, + + [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) { + const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); + const firstDiscussion = discussions[0]; + const isDiffDiscussion = firstDiscussion.diff_discussion; + const hasLineCode = firstDiscussion.line_code; + const isResolvable = firstDiscussion.resolvable; + const diffPosition = diffPositionByLineCode[firstDiscussion.line_code]; + + if ( + selectedFile && + isDiffDiscussion && + hasLineCode && + isResolvable && + diffPosition && + isDiscussionApplicableToLine(firstDiscussion, diffPosition) + ) { + const targetLine = selectedFile.parallelDiffLines.find( + line => + (line.left && line.left.lineCode === firstDiscussion.line_code) || + (line.right && line.right.lineCode === firstDiscussion.line_code), + ); + if (targetLine) { + if (targetLine.left && targetLine.left.lineCode === firstDiscussion.line_code) { + Object.assign(targetLine.left, { + discussions, + }); + } else { + Object.assign(targetLine.right, { + discussions, + }); + } + } + + if (selectedFile.highlightedDiffLines) { + const targetInlineLine = selectedFile.highlightedDiffLines.find( + line => line.lineCode === firstDiscussion.line_code, + ); + + if (targetInlineLine) { + Object.assign(targetInlineLine, { + discussions, + }); + } + } + } + }, + + [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) { + const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); + if (selectedFile) { + const targetLine = selectedFile.parallelDiffLines.find( + line => + (line.left && line.left.lineCode === lineCode) || + (line.right && line.right.lineCode === lineCode), + ); + if (targetLine) { + const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right'; + + Object.assign(targetLine[side], { + discussions: [], + }); + } + + if (selectedFile.highlightedDiffLines) { + const targetInlineLine = selectedFile.highlightedDiffLines.find( + line => line.lineCode === lineCode, + ); + + if (targetInlineLine) { + Object.assign(targetInlineLine, { + discussions: [], + }); + } + } + } + }, }; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 82082ac508a..b7e52a8f37f 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -8,6 +8,8 @@ import { NEW_LINE_TYPE, OLD_LINE_TYPE, MATCH_LINE_TYPE, + LINES_TO_BE_RENDERED_DIRECTLY, + MAX_LINES_TO_BE_RENDERED, } from '../constants'; export function findDiffFile(files, hash) { @@ -161,6 +163,11 @@ export function addContextLines(options) { * @returns {Object} */ export function trimFirstCharOfLineContent(line = {}) { + // eslint-disable-next-line no-param-reassign + delete line.text; + // eslint-disable-next-line no-param-reassign + line.discussions = []; + const parsedLine = Object.assign({}, line); if (line.richText) { @@ -174,7 +181,44 @@ export function trimFirstCharOfLineContent(line = {}) { return parsedLine; } -export function getDiffRefsByLineCode(diffFiles) { +// This prepares and optimizes the incoming diff data from the server +// by setting up incremental rendering and removing unneeded data +export function prepareDiffData(diffData) { + const filesLength = diffData.diffFiles.length; + let showingLines = 0; + for (let i = 0; i < filesLength; i += 1) { + const file = diffData.diffFiles[i]; + + if (file.parallelDiffLines) { + const linesLength = file.parallelDiffLines.length; + for (let u = 0; u < linesLength; u += 1) { + const line = file.parallelDiffLines[u]; + if (line.left) { + line.left = trimFirstCharOfLineContent(line.left); + } + if (line.right) { + line.right = trimFirstCharOfLineContent(line.right); + } + } + } + + if (file.highlightedDiffLines) { + const linesLength = file.highlightedDiffLines.length; + for (let u = 0; u < linesLength; u += 1) { + const line = file.highlightedDiffLines[u]; + Object.assign(line, { ...trimFirstCharOfLineContent(line) }); + } + showingLines += file.parallelDiffLines.length; + } + + Object.assign(file, { + renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, + collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, + }); + } +} + +export function getDiffPositionByLineCode(diffFiles) { return diffFiles.reduce((acc, diffFile) => { const { baseSha, headSha, startSha } = diffFile.diffRefs; const { newPath, oldPath } = diffFile; @@ -194,3 +238,12 @@ export function getDiffRefsByLineCode(diffFiles) { return acc; }, {}); } + +// This method will check whether the discussion is still applicable +// to the diff line in question regarding different versions of the MR +export function isDiscussionApplicableToLine(discussion, diffPosition) { + const originalRefs = convertObjectPropsToCamelCase(discussion.original_position.formatter); + const refs = convertObjectPropsToCamelCase(discussion.position.formatter); + + return _.isEqual(refs, diffPosition) || _.isEqual(originalRefs, diffPosition); +} |