diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-19 08:27:35 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-19 08:27:35 +0000 |
commit | 7e9c479f7de77702622631cff2628a9c8dcbc627 (patch) | |
tree | c8f718a08e110ad7e1894510980d2155a6549197 /app/assets/javascripts/diffs | |
parent | e852b0ae16db4052c1c567d9efa4facc81146e88 (diff) | |
download | gitlab-ce-7e9c479f7de77702622631cff2628a9c8dcbc627.tar.gz |
Add latest changes from gitlab-org/gitlab@13-6-stable-eev13.6.0-rc42
Diffstat (limited to 'app/assets/javascripts/diffs')
31 files changed, 1215 insertions, 665 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 085f951147f..9d8d184a3f6 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -20,6 +20,8 @@ import HiddenFilesWarning from './hidden_files_warning.vue'; import MergeConflictWarning from './merge_conflict_warning.vue'; import CollapsedFilesWarning from './collapsed_files_warning.vue'; +import { diffsApp } from '../utils/performance'; + import { TREE_LIST_WIDTH_STORAGE_KEY, INITIAL_TREE_WIDTH, @@ -124,7 +126,6 @@ export default { return { treeWidth, diffFilesLength: 0, - collapsedWarningDismissed: false, }; }, computed: { @@ -153,7 +154,7 @@ export default { 'canMerge', 'hasConflicts', ]), - ...mapGetters('diffs', ['hasCollapsedFile', 'isParallelView', 'currentDiffIndex']), + ...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']), ...mapGetters(['isNotesFetched', 'getNoteableData']), diffs() { if (!this.viewDiffsFileByFile) { @@ -206,11 +207,7 @@ export default { visible = this.$options.alerts.ALERT_OVERFLOW_HIDDEN; } else if (this.isDiffHead && this.hasConflicts) { visible = this.$options.alerts.ALERT_MERGE_CONFLICT; - } else if ( - this.hasCollapsedFile && - !this.collapsedWarningDismissed && - !this.viewDiffsFileByFile - ) { + } else if (this.whichCollapsedTypes.automatic && !this.viewDiffsFileByFile) { visible = this.$options.alerts.ALERT_COLLAPSED_FILES; } @@ -277,8 +274,12 @@ export default { ); } }, + beforeCreate() { + diffsApp.instrument(); + }, created() { this.adjustView(); + eventHub.$once('fetchDiffData', this.fetchData); eventHub.$on('refetchDiffData', this.refetchDiffData); this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES; @@ -299,6 +300,8 @@ export default { ); }, beforeDestroy() { + diffsApp.deinstrument(); + eventHub.$off('fetchDiffData', this.fetchData); eventHub.$off('refetchDiffData', this.refetchDiffData); this.removeEventListeners(); @@ -429,9 +432,6 @@ export default { this.toggleShowTreeList(false); } }, - dismissCollapsedWarning() { - this.collapsedWarningDismissed = true; - }, }, minTreeWidth: MIN_TREE_WIDTH, maxTreeWidth: MAX_TREE_WIDTH, @@ -464,7 +464,6 @@ export default { <collapsed-files-warning v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES" :limited="isLimitedContainer" - @dismiss="dismissCollapsedWarning" /> <div @@ -496,9 +495,11 @@ export default { <div v-if="isBatchLoading" class="loading"><gl-loading-icon size="lg" /></div> <template v-else-if="renderDiffFiles"> <diff-file - v-for="file in diffs" + v-for="(file, index) in diffs" :key="file.newPath" :file="file" + :is-first-file="index === 0" + :is-last-file="index === diffs.length - 1" :help-page-path="helpPagePath" :can-current-user-fork="canCurrentUserFork" :view-diffs-file-by-file="viewDiffsFileByFile" diff --git a/app/assets/javascripts/diffs/components/collapsed_files_warning.vue b/app/assets/javascripts/diffs/components/collapsed_files_warning.vue index 270bbfb99b7..0cf1cdb17f8 100644 --- a/app/assets/javascripts/diffs/components/collapsed_files_warning.vue +++ b/app/assets/javascripts/diffs/components/collapsed_files_warning.vue @@ -1,9 +1,8 @@ <script> -import { mapActions } from 'vuex'; - import { GlAlert, GlButton } from '@gitlab/ui'; -import { CENTERED_LIMITED_CONTAINER_CLASSES } from '../constants'; +import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '../constants'; +import eventHub from '../event_hub'; export default { components: { @@ -36,13 +35,12 @@ export default { }, methods: { - ...mapActions('diffs', ['expandAllFiles']), dismiss() { this.isDismissed = true; this.$emit('dismiss'); }, expand() { - this.expandAllFiles(); + eventHub.$emit(EVT_EXPAND_ALL_FILES); this.dismiss(); }, }, diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index b1ebd8e6ebc..700d5ec86c8 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -6,7 +6,8 @@ import { polyfillSticky } from '~/lib/utils/sticky'; import CompareDropdownLayout from './compare_dropdown_layout.vue'; import SettingsDropdown from './settings_dropdown.vue'; import DiffStats from './diff_stats.vue'; -import { CENTERED_LIMITED_CONTAINER_CLASSES } from '../constants'; +import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '../constants'; +import eventHub from '../event_hub'; export default { components: { @@ -38,7 +39,7 @@ export default { }, computed: { ...mapGetters('diffs', [ - 'hasCollapsedFile', + 'whichCollapsedTypes', 'diffCompareDropdownTargetVersions', 'diffCompareDropdownSourceVersions', ]), @@ -67,9 +68,11 @@ export default { ...mapActions('diffs', [ 'setInlineDiffViewType', 'setParallelDiffViewType', - 'expandAllFiles', 'toggleShowTreeList', ]), + expandAllFiles() { + eventHub.$emit(EVT_EXPAND_ALL_FILES); + }, }, }; </script> @@ -129,7 +132,7 @@ export default { {{ __('Show latest version') }} </gl-button> <gl-button - v-show="hasCollapsedFile" + v-show="whichCollapsedTypes.any" variant="default" class="gl-mr-3" @click="expandAllFiles" diff --git a/app/assets/javascripts/diffs/components/diff_comment_cell.vue b/app/assets/javascripts/diffs/components/diff_comment_cell.vue new file mode 100644 index 00000000000..4b0b603f5a5 --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_comment_cell.vue @@ -0,0 +1,69 @@ +<script> +import { mapActions } from 'vuex'; +import DiffDiscussions from './diff_discussions.vue'; +import DiffLineNoteForm from './diff_line_note_form.vue'; +import DiffDiscussionReply from './diff_discussion_reply.vue'; + +export default { + components: { + DiffDiscussions, + DiffLineNoteForm, + DiffDiscussionReply, + }, + props: { + line: { + type: Object, + required: true, + }, + diffFileHash: { + type: String, + required: true, + }, + helpPagePath: { + type: String, + required: false, + default: '', + }, + hasDraft: { + type: Boolean, + required: false, + default: false, + }, + linePosition: { + type: String, + required: false, + default: '', + }, + }, + methods: { + ...mapActions('diffs', ['showCommentForm']), + }, +}; +</script> + +<template> + <div class="content"> + <diff-discussions + v-if="line.renderDiscussion" + :line="line" + :discussions="line.discussions" + :help-page-path="helpPagePath" + /> + <diff-discussion-reply + v-if="!hasDraft" + :has-form="line.hasCommentForm" + :render-reply-placeholder="Boolean(line.discussions.length)" + @showNewDiscussionForm="showCommentForm({ lineCode: line.line_code, fileHash: diffFileHash })" + > + <template #form> + <diff-line-note-form + :diff-file-hash="diffFileHash" + :line="line" + :note-target-line="line" + :help-page-path="helpPagePath" + :line-position="linePosition" + /> + </template> + </diff-discussion-reply> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index e68260b3e62..401064fb18f 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -10,6 +10,7 @@ import NoPreviewViewer from '~/vue_shared/components/diff_viewer/viewers/no_prev import DiffFileDrafts from '~/batch_comments/components/diff_file_drafts.vue'; import InlineDiffView from './inline_diff_view.vue'; import ParallelDiffView from './parallel_diff_view.vue'; +import DiffView from './diff_view.vue'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import NoteForm from '../../notes/components/note_form.vue'; import ImageDiffOverlay from './image_diff_overlay.vue'; @@ -18,12 +19,14 @@ import eventHub from '../../notes/event_hub'; import { IMAGE_DIFF_POSITION_TYPE } from '../constants'; import { getDiffMode } from '../store/utils'; import { diffViewerModes } from '~/ide/constants'; +import { mapInline, mapParallel } from './diff_row_utils'; export default { components: { GlLoadingIcon, InlineDiffView, ParallelDiffView, + DiffView, DiffViewer, NoteForm, DiffDiscussions, @@ -83,6 +86,19 @@ export default { author() { return this.getUserData; }, + mappedLines() { + if (this.glFeatures.unifiedDiffLines && this.glFeatures.unifiedDiffComponents) { + return this.diffLines(this.diffFile, true).map(mapParallel(this)) || []; + } + + // TODO: Everything below this line can be deleted when unifiedDiffComponents FF is removed + if (this.isInlineView) { + return this.diffFile.highlighted_diff_lines.map(mapInline(this)); + } + return this.glFeatures.unifiedDiffLines + ? this.diffLines(this.diffFile).map(mapParallel(this)) + : this.diffFile.parallel_diff_lines.map(mapParallel(this)) || []; + }, }, updated() { this.$nextTick(() => { @@ -113,19 +129,28 @@ export default { <template> <div class="diff-content"> <div class="diff-viewer"> - <template v-if="isTextFile"> + <template + v-if="isTextFile && glFeatures.unifiedDiffLines && glFeatures.unifiedDiffComponents" + > + <diff-view + :diff-file="diffFile" + :diff-lines="mappedLines" + :help-page-path="helpPagePath" + :inline="isInlineView" + /> + <gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" /> + </template> + <template v-else-if="isTextFile"> <inline-diff-view v-if="isInlineView" :diff-file="diffFile" - :diff-lines="diffFile.highlighted_diff_lines" + :diff-lines="mappedLines" :help-page-path="helpPagePath" /> <parallel-diff-view v-else-if="isParallelView" :diff-file="diffFile" - :diff-lines=" - glFeatures.unifiedDiffLines ? diffLines(diffFile) : diffFile.parallel_diff_lines || [] - " + :diff-lines="mappedLines" :help-page-path="helpPagePath" /> <gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" /> diff --git a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue index 0094b4f8707..4c49dfb5de9 100644 --- a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue @@ -6,7 +6,6 @@ import { s__, sprintf } from '~/locale'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { UNFOLD_COUNT, INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '../constants'; import * as utils from '../store/utils'; -import tooltip from '../../vue_shared/directives/tooltip'; const EXPAND_ALL = 0; const EXPAND_UP = 1; @@ -28,9 +27,6 @@ const i18n = { export default { i18n, - directives: { - tooltip, - }, components: { GlIcon, }, @@ -58,11 +54,6 @@ export default { required: false, default: false, }, - colspan: { - type: Number, - required: false, - default: 4, - }, }, computed: { ...mapState({ @@ -235,28 +226,26 @@ export default { </script> <template> - <td :colspan="colspan" class="text-center gl-font-regular"> - <div class="content js-line-expansion-content"> - <a - v-if="canExpandDown" - class="gl-mx-2 gl-cursor-pointer js-unfold-down gl-display-inline-block gl-py-4" - @click="handleExpandLines(EXPAND_DOWN)" - > - <gl-icon :size="12" name="expand-down" aria-hidden="true" /> - <span>{{ $options.i18n.showMore }}</span> - </a> - <a class="gl-mx-2 cursor-pointer js-unfold-all" @click="handleExpandLines()"> - <gl-icon :size="12" name="expand" aria-hidden="true" /> - <span>{{ $options.i18n.showAll }}</span> - </a> - <a - v-if="canExpandUp" - class="gl-mx-2 gl-cursor-pointer js-unfold gl-display-inline-block gl-py-4" - @click="handleExpandLines(EXPAND_UP)" - > - <gl-icon :size="12" name="expand-up" aria-hidden="true" /> - <span>{{ $options.i18n.showMore }}</span> - </a> - </div> - </td> + <div class="content js-line-expansion-content"> + <a + v-if="canExpandDown" + class="gl-mx-2 gl-cursor-pointer js-unfold-down gl-display-inline-block gl-py-4" + @click="handleExpandLines(EXPAND_DOWN)" + > + <gl-icon :size="12" name="expand-down" aria-hidden="true" /> + <span>{{ $options.i18n.showMore }}</span> + </a> + <a class="gl-mx-2 cursor-pointer js-unfold-all" @click="handleExpandLines()"> + <gl-icon :size="12" name="expand" aria-hidden="true" /> + <span>{{ $options.i18n.showAll }}</span> + </a> + <a + v-if="canExpandUp" + class="gl-mx-2 gl-cursor-pointer js-unfold gl-display-inline-block gl-py-4" + @click="handleExpandLines(EXPAND_UP)" + > + <gl-icon :size="12" name="expand-up" aria-hidden="true" /> + <span>{{ $options.i18n.showMore }}</span> + </a> + </div> </template> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 529723a349d..32191d7e309 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -1,20 +1,31 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; import { escape } from 'lodash'; -import { GlLoadingIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; +import { GlButton, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import { __, sprintf } from '~/locale'; +import { sprintf } from '~/locale'; import { deprecatedCreateFlash as createFlash } from '~/flash'; import { hasDiff } from '~/helpers/diffs_helper'; -import eventHub from '../../notes/event_hub'; +import notesEventHub from '../../notes/event_hub'; import DiffFileHeader from './diff_file_header.vue'; import DiffContent from './diff_content.vue'; import { diffViewerErrors } from '~/ide/constants'; +import { collapsedType, isCollapsed } from '../diff_file'; +import { + DIFF_FILE_AUTOMATIC_COLLAPSE, + DIFF_FILE_MANUAL_COLLAPSE, + EVT_EXPAND_ALL_FILES, + EVT_PERF_MARK_DIFF_FILES_END, + EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN, +} from '../constants'; +import { DIFF_FILE, GENERIC_ERROR } from '../i18n'; +import eventHub from '../event_hub'; export default { components: { DiffFileHeader, DiffContent, + GlButton, GlLoadingIcon, }, directives: { @@ -26,6 +37,16 @@ export default { type: Object, required: true, }, + isFirstFile: { + type: Boolean, + required: false, + default: false, + }, + isLastFile: { + type: Boolean, + required: false, + default: false, + }, canCurrentUserFork: { type: Boolean, required: true, @@ -44,16 +65,20 @@ export default { return { isLoadingCollapsedDiff: false, forkMessageVisible: false, - isCollapsed: this.file.viewer.automaticallyCollapsed || false, + isCollapsed: isCollapsed(this.file), }; }, + i18n: { + ...DIFF_FILE, + genericError: GENERIC_ERROR, + }, computed: { ...mapState('diffs', ['currentDiffFileId']), ...mapGetters(['isNotesFetched']), ...mapGetters('diffs', ['getDiffFileDiscussions']), viewBlobLink() { return sprintf( - __('You can %{linkStart}view the blob%{linkEnd} instead.'), + this.$options.i18n.blobView, { linkStart: `<a href="${escape(this.file.view_path)}">`, linkEnd: '</a>', @@ -71,13 +96,11 @@ export default { return this.file.viewer.error === diffViewerErrors.too_large; }, errorMessage() { - return this.file.viewer.error_message; + return !this.manuallyCollapsed ? this.file.viewer.error_message : ''; }, forkMessage() { return sprintf( - __( - "You're not allowed to %{tag_start}edit%{tag_end} files in this project directly. Please fork this project, make your changes there, and submit a merge request.", - ), + this.$options.i18n.editInFork, { tag_start: '<span class="js-file-fork-suggestion-section-action">', tag_end: '</span>', @@ -85,62 +108,131 @@ export default { false, ); }, - }, - watch: { - isCollapsed: function fileCollapsedWatch(newVal, oldVal) { - if (!newVal && oldVal && !this.hasDiff) { - this.handleLoadCollapsedDiff(); + hasBodyClasses() { + const domParts = { + header: 'gl-rounded-base!', + contentByHash: '', + content: '', + }; + + if (this.showBody) { + domParts.header = 'gl-rounded-bottom-left-none gl-rounded-bottom-right-none'; + domParts.contentByHash = + 'gl-rounded-none gl-rounded-bottom-left-base gl-rounded-bottom-right-base gl-border-1 gl-border-t-0! gl-border-solid gl-border-gray-100'; + domParts.content = 'gl-rounded-bottom-left-base gl-rounded-bottom-right-base'; } - this.setFileCollapsed({ filePath: this.file.file_path, collapsed: newVal }); + return domParts; }, + automaticallyCollapsed() { + return collapsedType(this.file) === DIFF_FILE_AUTOMATIC_COLLAPSE; + }, + manuallyCollapsed() { + return collapsedType(this.file) === DIFF_FILE_MANUAL_COLLAPSE; + }, + showBody() { + return !this.isCollapsed || this.automaticallyCollapsed; + }, + showWarning() { + return this.isCollapsed && (this.automaticallyCollapsed && !this.viewDiffsFileByFile); + }, + showContent() { + return !this.isCollapsed && !this.isFileTooLarge; + }, + }, + watch: { 'file.file_hash': { - handler: function watchFileHash() { - if (this.viewDiffsFileByFile && this.file.viewer.automaticallyCollapsed) { - this.isCollapsed = false; - this.handleLoadCollapsedDiff(); - } else { - this.isCollapsed = this.file.viewer.automaticallyCollapsed || false; + handler: function hashChangeWatch(newHash, oldHash) { + this.isCollapsed = isCollapsed(this.file); + + if (newHash && oldHash && !this.hasDiff) { + this.requestDiff(); } }, immediate: true, }, - 'file.viewer.automaticallyCollapsed': function setIsCollapsed(newVal) { - if (!this.viewDiffsFileByFile) { - this.isCollapsed = newVal; - } + 'file.viewer.automaticallyCollapsed': { + handler: function autoChangeWatch(automaticValue) { + if (collapsedType(this.file) !== DIFF_FILE_MANUAL_COLLAPSE) { + this.isCollapsed = this.viewDiffsFileByFile ? false : automaticValue; + } + }, + immediate: true, + }, + 'file.viewer.manuallyCollapsed': { + handler: function manualChangeWatch(manualValue) { + if (manualValue !== null) { + this.isCollapsed = manualValue; + } + }, + immediate: true, }, }, created() { - eventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.handleLoadCollapsedDiff); + notesEventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.requestDiff); + eventHub.$on(EVT_EXPAND_ALL_FILES, this.expandAllListener); + }, + mounted() { + if (this.hasDiff) { + this.postRender(); + } + }, + beforeDestroy() { + eventHub.$off(EVT_EXPAND_ALL_FILES, this.expandAllListener); }, methods: { ...mapActions('diffs', [ 'loadCollapsedDiff', 'assignDiscussionsToDiff', 'setRenderIt', - 'setFileCollapsed', + 'setFileCollapsedByUser', ]), + expandAllListener() { + if (this.isCollapsed) { + this.handleToggle(); + } + }, + async postRender() { + const eventsForThisFile = []; + + if (this.isFirstFile) { + eventsForThisFile.push(EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN); + } + + if (this.isLastFile) { + eventsForThisFile.push(EVT_PERF_MARK_DIFF_FILES_END); + } + + await this.$nextTick(); + + eventsForThisFile.forEach(event => { + eventHub.$emit(event); + }); + }, handleToggle() { - if (!this.hasDiff) { - this.handleLoadCollapsedDiff(); - } else { - this.isCollapsed = !this.isCollapsed; - this.setRenderIt(this.file); + const currentCollapsedFlag = this.isCollapsed; + + this.setFileCollapsedByUser({ + filePath: this.file.file_path, + collapsed: !currentCollapsedFlag, + }); + + if (!this.hasDiff && currentCollapsedFlag) { + this.requestDiff(); } }, - handleLoadCollapsedDiff() { + requestDiff() { this.isLoadingCollapsedDiff = true; this.loadCollapsedDiff(this.file) .then(() => { this.isLoadingCollapsedDiff = false; - this.isCollapsed = false; this.setRenderIt(this.file); }) .then(() => { requestIdleCallback( () => { + this.postRender(); this.assignDiscussionsToDiff(this.getDiffFileDiscussions(this.file)); }, { timeout: 1000 }, @@ -148,7 +240,7 @@ export default { }) .catch(() => { this.isLoadingCollapsedDiff = false; - createFlash(__('Something went wrong on our end. Please try again!')); + createFlash(this.$options.i18n.genericError); }); }, showForkMessage() { @@ -167,9 +259,10 @@ export default { :class="{ 'is-active': currentDiffFileId === file.file_hash, 'comments-disabled': Boolean(file.brokenSymlink), + 'has-body': showBody, }" :data-path="file.new_path" - class="diff-file file-holder" + class="diff-file file-holder gl-border-none" > <diff-file-header :can-current-user-fork="canCurrentUserFork" @@ -178,7 +271,8 @@ export default { :expanded="!isCollapsed" :add-merge-request-buttons="true" :view-diffs-file-by-file="viewDiffsFileByFile" - class="js-file-title file-title" + class="js-file-title file-title gl-border-1 gl-border-solid gl-border-gray-100" + :class="hasBodyClasses.header" @toggleFile="handleToggle" @showForkMessage="showForkMessage" /> @@ -188,31 +282,50 @@ export default { <a :href="file.fork_path" class="js-fork-suggestion-button btn btn-grouped btn-inverted btn-success" - >{{ __('Fork') }}</a + >{{ $options.i18n.fork }}</a > <button class="js-cancel-fork-suggestion-button btn btn-grouped" type="button" @click="hideForkMessage" > - {{ __('Cancel') }} + {{ $options.i18n.cancel }} </button> </div> - <gl-loading-icon v-if="showLoadingIcon" class="diff-content loading" /> <template v-else> - <div :id="`diff-content-${file.file_hash}`"> - <div v-if="errorMessage" class="diff-viewer"> + <div + :id="`diff-content-${file.file_hash}`" + :class="hasBodyClasses.contentByHash" + data-testid="content-area" + > + <gl-loading-icon + v-if="showLoadingIcon" + class="diff-content loading gl-my-0 gl-pt-3" + data-testid="loader-icon" + /> + <div v-else-if="errorMessage" class="diff-viewer"> <div v-safe-html="errorMessage" class="nothing-here-block"></div> </div> <template v-else> - <div v-show="isCollapsed" class="nothing-here-block diff-collapsed"> - {{ __('This diff is collapsed.') }} - <a class="click-to-expand js-click-to-expand" href="#" @click.prevent="handleToggle">{{ - __('Click to expand it.') - }}</a> + <div + v-show="showWarning" + class="collapsed-file-warning gl-p-7 gl-bg-orange-50 gl-text-center gl-rounded-bottom-left-base gl-rounded-bottom-right-base" + > + <p class="gl-mb-8"> + {{ $options.i18n.autoCollapsed }} + </p> + <gl-button + data-testid="expand-button" + category="secondary" + variant="warning" + @click.prevent="handleToggle" + > + {{ $options.i18n.expand }} + </gl-button> </div> <diff-content - v-show="!isCollapsed && !isFileTooLarge" + v-show="showContent" + :class="hasBodyClasses.content" :diff-file="file" :help-page-path="helpPagePath" /> diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 9f451cd759a..0d99a2e8a60 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -10,6 +10,7 @@ import { GlDropdown, GlDropdownItem, GlDropdownDivider, + GlLoadingIcon, } from '@gitlab/ui'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue'; @@ -18,6 +19,7 @@ import { __, s__, sprintf } from '~/locale'; import { diffViewerModes } from '~/ide/constants'; import DiffStats from './diff_stats.vue'; import { scrollToElement } from '~/lib/utils/common_utils'; +import { isCollapsed } from '../diff_file'; import { DIFF_FILE_HEADER } from '../i18n'; export default { @@ -31,6 +33,7 @@ export default { GlDropdown, GlDropdownItem, GlDropdownDivider, + GlLoadingIcon, }, directives: { GlTooltip: GlTooltipDirective, @@ -125,6 +128,9 @@ export default { isUsingLfs() { return this.diffFile.stored_externally && this.diffFile.external_storage === 'lfs'; }, + isCollapsed() { + return isCollapsed(this.diffFile, { fileByFile: this.viewDiffsFileByFile }); + }, collapseIcon() { return this.expanded ? 'chevron-down' : 'chevron-right'; }, @@ -209,7 +215,7 @@ export default { class="js-file-title file-title file-title-flex-parent" @click.self="handleToggleFile" > - <div class="file-header-content gl-display-flex gl-align-items-center gl-pr-0!"> + <div class="file-header-content"> <gl-icon v-if="collapsible" ref="collapseIcon" @@ -222,11 +228,17 @@ export default { <a ref="titleWrapper" :v-once="!viewDiffsFileByFile" - class="gl-mr-2 gl-text-decoration-none! gl-text-truncate" + class="gl-mr-2 gl-text-decoration-none! gl-word-break-all" :href="titleLink" @click="handleFileNameClick" > - <file-icon :file-name="filePath" :size="18" aria-hidden="true" css-classes="gl-mr-2" /> + <file-icon + :file-name="filePath" + :size="18" + aria-hidden="true" + css-classes="gl-mr-2" + :submodule="diffFile.submodule" + /> <span v-if="isFileRenamed"> <strong v-gl-tooltip @@ -270,12 +282,12 @@ export default { {{ diffFile.a_mode }} → {{ diffFile.b_mode }} </small> - <span v-if="isUsingLfs" class="label label-lfs gl-mr-2"> {{ __('LFS') }} </span> + <span v-if="isUsingLfs" class="badge label label-lfs gl-mr-2"> {{ __('LFS') }} </span> </div> <div v-if="!diffFile.submodule && addMergeRequestButtons" - class="file-actions d-flex align-items-center flex-wrap" + class="file-actions d-flex align-items-center gl-ml-auto gl-align-self-start" > <diff-stats :added-lines="diffFile.added_lines" :removed-lines="diffFile.removed_lines" /> <gl-button-group class="gl-pt-0!"> @@ -334,7 +346,7 @@ export default { </gl-dropdown-item> </template> - <template v-if="!diffFile.viewer.automaticallyCollapsed"> + <template v-if="!isCollapsed"> <gl-dropdown-divider v-if="!diffFile.is_fully_expanded || diffHasDiscussions(diffFile)" /> @@ -355,8 +367,10 @@ export default { <gl-dropdown-item v-if="!diffFile.is_fully_expanded" ref="expandDiffToFullFileButton" + :disabled="diffFile.isLoadingFullFile" @click="toggleFullDiff(diffFile.file_path)" > + <gl-loading-icon v-if="diffFile.isLoadingFullFile" inline /> {{ expandDiffToFullFileTitle }} </gl-dropdown-item> </template> diff --git a/app/assets/javascripts/diffs/components/diff_file_row.vue b/app/assets/javascripts/diffs/components/diff_file_row.vue index 2856e6ae8eb..3888eb781fb 100644 --- a/app/assets/javascripts/diffs/components/diff_file_row.vue +++ b/app/assets/javascripts/diffs/components/diff_file_row.vue @@ -62,6 +62,7 @@ export default { v-bind="$attrs" :class="{ 'is-active': isActive }" class="diff-file-row" + truncate-middle :file-classes="fileClasses" v-on="$listeners" > 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 700e6302102..55f5a736cdf 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -7,7 +7,7 @@ import noteForm from '../../notes/components/note_form.vue'; import MultilineCommentForm from '../../notes/components/multiline_comment_form.vue'; import autosave from '../../notes/mixins/autosave'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; -import { DIFF_NOTE_TYPE } from '../constants'; +import { DIFF_NOTE_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '../constants'; import { commentLineOptions, formatLineRange, @@ -60,7 +60,7 @@ export default { diffViewType: state => state.diffs.diffViewType, }), ...mapState('diffs', ['showSuggestPopover']), - ...mapGetters('diffs', ['getDiffFileByHash']), + ...mapGetters('diffs', ['getDiffFileByHash', 'diffLines']), ...mapGetters([ 'isLoggedIn', 'noteableType', @@ -88,16 +88,30 @@ export default { commentLineOptions() { const combineSides = (acc, { left, right }) => { // ignore null values match lines - if (left && left.type !== 'match') acc.push(left); + if (left) acc.push(left); // if the line_codes are identically, return to avoid duplicates - if (left?.line_code === right?.line_code) return acc; + if ( + left?.line_code === right?.line_code || + left?.type === 'old-nonewline' || + right?.type === 'new-nonewline' + ) { + return acc; + } if (right && right.type !== 'match') acc.push(right); return acc; }; + const getDiffLines = () => { + if (this.diffViewType === PARALLEL_DIFF_VIEW_TYPE) { + return (this.glFeatures.unifiedDiffLines + ? this.diffLines(this.diffFile) + : this.diffFile.parallel_diff_lines + ).reduce(combineSides, []); + } + + return this.diffFile.highlighted_diff_lines; + }; const side = this.line.type === 'new' ? 'right' : 'left'; - const lines = this.diffFile.highlighted_diff_lines.length - ? this.diffFile.highlighted_diff_lines - : this.diffFile.parallel_diff_lines.reduce(combineSides, []); + const lines = getDiffLines(); return commentLineOptions(lines, this.line, this.line.line_code, side); }, }, diff --git a/app/assets/javascripts/diffs/components/diff_row.vue b/app/assets/javascripts/diffs/components/diff_row.vue new file mode 100644 index 00000000000..77a97c67f3b --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_row.vue @@ -0,0 +1,271 @@ +<script> +import { mapActions, mapGetters, mapState } from 'vuex'; +import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; +import { CONTEXT_LINE_CLASS_NAME, PARALLEL_DIFF_VIEW_TYPE } from '../constants'; +import DiffGutterAvatars from './diff_gutter_avatars.vue'; +import * as utils from './diff_row_utils'; + +export default { + components: { + GlIcon, + DiffGutterAvatars, + }, + directives: { + GlTooltip: GlTooltipDirective, + SafeHtml, + }, + props: { + fileHash: { + type: String, + required: true, + }, + filePath: { + type: String, + required: true, + }, + line: { + type: Object, + required: true, + }, + isCommented: { + type: Boolean, + required: false, + default: false, + }, + inline: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + ...mapGetters('diffs', ['fileLineCoverage']), + ...mapGetters(['isLoggedIn']), + ...mapState({ + isHighlighted(state) { + const line = this.line.left?.line_code ? this.line.left : this.line.right; + return utils.isHighlighted(state, line, this.isCommented); + }, + }), + classNameMap() { + return { + [CONTEXT_LINE_CLASS_NAME]: this.line.isContextLineLeft, + [PARALLEL_DIFF_VIEW_TYPE]: true, + }; + }, + parallelViewLeftLineType() { + return utils.parallelViewLeftLineType(this.line, this.isHighlighted); + }, + coverageState() { + return this.fileLineCoverage(this.filePath, this.line.right.new_line); + }, + classNameMapCellLeft() { + return utils.classNameMapCell(this.line.left, this.isHighlighted, this.isLoggedIn); + }, + classNameMapCellRight() { + return utils.classNameMapCell(this.line.right, this.isHighlighted, this.isLoggedIn); + }, + addCommentTooltipLeft() { + return utils.addCommentTooltip(this.line.left); + }, + addCommentTooltipRight() { + return utils.addCommentTooltip(this.line.right); + }, + shouldRenderCommentButton() { + return ( + this.isLoggedIn && + !this.line.isContextLineLeft && + !this.line.isMetaLineLeft && + !this.line.hasDiscussionsLeft && + !this.line.hasDiscussionsRight + ); + }, + }, + mounted() { + this.scrollToLineIfNeededParallel(this.line); + }, + methods: { + ...mapActions('diffs', [ + 'scrollToLineIfNeededParallel', + 'showCommentForm', + 'setHighlightedRow', + 'toggleLineDiscussions', + ]), + // 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('.diff-table'); + + table.classList.remove('left-side-selected', 'right-side-selected'); + const [lineClass] = ['left-side', 'right-side'].filter(name => line.classList.contains(name)); + + if (lineClass) { + table.classList.add(`${lineClass}-selected`); + } + }, + handleCommentButton(line) { + this.showCommentForm({ lineCode: line.line_code, fileHash: this.fileHash }); + }, + }, +}; +</script> + +<template> + <div :class="classNameMap" class="diff-grid-row diff-tr line_holder"> + <div class="diff-grid-left left-side"> + <template v-if="line.left"> + <div + :class="classNameMapCellLeft" + data-testid="leftLineNumber" + class="diff-td diff-line-num old_line" + > + <span + v-if="shouldRenderCommentButton" + v-gl-tooltip + data-testid="leftCommentButton" + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltipLeft" + > + <button + type="button" + class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + :disabled="line.left.commentsDisabled" + @click="handleCommentButton(line.left)" + > + <gl-icon :size="12" name="comment" /> + </button> + </span> + <a + v-if="line.left.old_line" + :data-linenumber="line.left.old_line" + :href="line.lineHrefOld" + @click="setHighlightedRow(line.lineCode)" + > + </a> + <diff-gutter-avatars + v-if="line.hasDiscussionsLeft" + :discussions="line.left.discussions" + :discussions-expanded="line.left.discussionsExpanded" + data-testid="leftDiscussions" + @toggleLineDiscussions=" + toggleLineDiscussions({ + lineCode: line.left.line_code, + fileHash, + expanded: !line.left.discussionsExpanded, + }) + " + /> + </div> + <div :class="classNameMapCellLeft" class="diff-td diff-line-num old_line"> + <a + v-if="line.left.old_line" + :data-linenumber="line.left.old_line" + :href="line.lineHrefOld" + @click="setHighlightedRow(line.lineCode)" + > + </a> + </div> + <div :class="parallelViewLeftLineType" class="diff-td line-coverage left-side"></div> + <div + :id="line.left.line_code" + :key="line.left.line_code" + v-safe-html="line.left.rich_text" + :class="parallelViewLeftLineType" + class="diff-td line_content with-coverage parallel left-side" + data-testid="leftContent" + @mousedown="handleParallelLineMouseDown" + ></div> + </template> + <template v-else> + <div data-testid="leftEmptyCell" class="diff-td diff-line-num old_line empty-cell"></div> + <div class="diff-td diff-line-num old_line empty-cell"></div> + <div class="diff-td line-coverage left-side empty-cell"></div> + <div class="diff-td line_content with-coverage parallel left-side empty-cell"></div> + </template> + </div> + <div + v-if="!inline || (line.right && Boolean(line.right.type))" + class="diff-grid-right right-side" + > + <template v-if="line.right"> + <div + :class="classNameMapCellRight" + data-testid="rightLineNumber" + class="diff-td diff-line-num new_line" + > + <span + v-if="shouldRenderCommentButton" + v-gl-tooltip + data-testid="rightCommentButton" + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltipRight" + > + <button + type="button" + class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + :disabled="line.right.commentsDisabled" + @click="handleCommentButton(line.right)" + > + <gl-icon :size="12" name="comment" /> + </button> + </span> + <a + v-if="line.right.new_line" + :data-linenumber="line.right.new_line" + :href="line.lineHrefNew" + @click="setHighlightedRow(line.lineCode)" + > + </a> + <diff-gutter-avatars + v-if="line.hasDiscussionsRight" + :discussions="line.right.discussions" + :discussions-expanded="line.right.discussionsExpanded" + data-testid="rightDiscussions" + @toggleLineDiscussions=" + toggleLineDiscussions({ + lineCode: line.right.line_code, + fileHash, + expanded: !line.right.discussionsExpanded, + }) + " + /> + </div> + <div :class="classNameMapCellRight" class="diff-td diff-line-num new_line"> + <a + v-if="line.right.new_line" + :data-linenumber="line.right.new_line" + :href="line.lineHrefNew" + @click="setHighlightedRow(line.lineCode)" + > + </a> + </div> + <div + v-gl-tooltip.hover + :title="coverageState.text" + :class="[line.right.type, coverageState.class, { hll: isHighlighted }]" + class="diff-td line-coverage right-side" + ></div> + <div + :id="line.right.line_code" + :key="line.right.rich_text" + v-safe-html="line.right.rich_text" + :class="[ + line.right.type, + { + hll: isHighlighted, + }, + ]" + class="diff-td line_content with-coverage parallel right-side" + @mousedown="handleParallelLineMouseDown" + ></div> + </template> + <template v-else> + <div data-testid="rightEmptyCell" class="diff-td diff-line-num old_line empty-cell"></div> + <div class="diff-td diff-line-num old_line empty-cell"></div> + <div class="diff-td line-coverage right-side empty-cell"></div> + <div class="diff-td line_content with-coverage parallel right-side empty-cell"></div> + </template> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_row_utils.js b/app/assets/javascripts/diffs/components/diff_row_utils.js index 08b87a4bade..d5491d3cd56 100644 --- a/app/assets/javascripts/diffs/components/diff_row_utils.js +++ b/app/assets/javascripts/diffs/components/diff_row_utils.js @@ -83,3 +83,76 @@ export const parallelViewLeftLineType = (line, hll) => { export const shouldShowCommentButton = (hover, context, meta, discussions) => { return hover && !context && !meta && !discussions; }; + +export const mapParallel = content => line => { + let { left, right } = line; + + // Dicussions/Comments + const hasExpandedDiscussionOnLeft = + left?.discussions?.length > 0 ? left?.discussionsExpanded : false; + const hasExpandedDiscussionOnRight = + right?.discussions?.length > 0 ? right?.discussionsExpanded : false; + + const renderCommentRow = + hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight || left?.hasForm || right?.hasForm; + + if (left) { + left = { + ...left, + renderDiscussion: hasExpandedDiscussionOnLeft, + hasDraft: content.hasParallelDraftLeft(content.diffFile.file_hash, line), + lineDraft: content.draftForLine(content.diffFile.file_hash, line, 'left'), + hasCommentForm: left.hasForm, + }; + } + if (right) { + right = { + ...right, + renderDiscussion: Boolean(hasExpandedDiscussionOnRight && right.type), + hasDraft: content.hasParallelDraftRight(content.diffFile.file_hash, line), + lineDraft: content.draftForLine(content.diffFile.file_hash, line, 'right'), + hasCommentForm: Boolean(right.hasForm && right.type), + }; + } + + return { + ...line, + left, + right, + isMatchLineLeft: isMatchLine(left?.type), + isMatchLineRight: isMatchLine(right?.type), + isContextLineLeft: isContextLine(left?.type), + isContextLineRight: isContextLine(right?.type), + hasDiscussionsLeft: hasDiscussions(left), + hasDiscussionsRight: hasDiscussions(right), + lineHrefOld: lineHref(left), + lineHrefNew: lineHref(right), + lineCode: lineCode(line), + isMetaLineLeft: isMetaLine(left?.type), + isMetaLineRight: isMetaLine(right?.type), + draftRowClasses: left?.lineDraft > 0 || right?.lineDraft > 0 ? '' : 'js-temp-notes-holder', + renderCommentRow, + commentRowClasses: hasDiscussions(left) || hasDiscussions(right) ? '' : 'js-temp-notes-holder', + }; +}; + +// TODO: Delete this function when unifiedDiffComponents FF is removed +export const mapInline = content => line => { + // Discussions/Comments + const renderCommentRow = line.hasForm || (line.discussions?.length && line.discussionsExpanded); + + return { + ...line, + renderDiscussion: Boolean(line.discussions?.length), + isMatchLine: isMatchLine(line.type), + commentRowClasses: line.discussions?.length ? '' : 'js-temp-notes-holder', + renderCommentRow, + hasDraft: content.shouldRenderDraftRow(content.diffFile.file_hash, line), + hasCommentForm: line.hasForm, + isMetaLine: isMetaLine(line.type), + isContextLine: isContextLine(line.type), + hasDiscussions: hasDiscussions(line), + lineHref: lineHref(line), + lineCode: lineCode(line), + }; +}; diff --git a/app/assets/javascripts/diffs/components/diff_view.vue b/app/assets/javascripts/diffs/components/diff_view.vue new file mode 100644 index 00000000000..84429f62a1c --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_view.vue @@ -0,0 +1,151 @@ +<script> +import { mapGetters, mapState } from 'vuex'; +import draftCommentsMixin from '~/diffs/mixins/draft_comments'; +import DraftNote from '~/batch_comments/components/draft_note.vue'; +import DiffRow from './diff_row.vue'; +import DiffCommentCell from './diff_comment_cell.vue'; +import DiffExpansionCell from './diff_expansion_cell.vue'; +import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; + +export default { + components: { + DiffExpansionCell, + DiffRow, + DiffCommentCell, + DraftNote, + }, + mixins: [draftCommentsMixin], + props: { + diffFile: { + type: Object, + required: true, + }, + diffLines: { + type: Array, + required: true, + }, + helpPagePath: { + type: String, + required: false, + default: '', + }, + inline: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + ...mapGetters('diffs', ['commitId']), + ...mapState({ + selectedCommentPosition: ({ notes }) => notes.selectedCommentPosition, + selectedCommentPositionHover: ({ notes }) => notes.selectedCommentPositionHover, + }), + diffLinesLength() { + return this.diffLines.length; + }, + commentedLines() { + return getCommentedLines( + this.selectedCommentPosition || this.selectedCommentPositionHover, + this.diffLines, + ); + }, + }, + methods: { + showCommentLeft(line) { + return !this.inline || line.left; + }, + showCommentRight(line) { + return !this.inline || (line.right && !line.left); + }, + }, + userColorScheme: window.gon.user_color_scheme, +}; +</script> + +<template> + <div + :class="[$options.userColorScheme, { inline }]" + :data-commit-id="commitId" + class="diff-grid diff-table code diff-wrap-lines js-syntax-highlight text-file" + > + <template v-for="(line, index) in diffLines"> + <div + v-if="line.isMatchLineLeft || line.isMatchLineRight" + :key="`expand-${index}`" + class="diff-tr line_expansion match" + > + <div class="diff-td text-center gl-font-regular"> + <diff-expansion-cell + :file-hash="diffFile.file_hash" + :context-lines-path="diffFile.context_lines_path" + :line="line.left" + :is-top="index === 0" + :is-bottom="index + 1 === diffLinesLength" + /> + </div> + </div> + <diff-row + v-if="!line.isMatchLineLeft && !line.isMatchLineRight" + :key="line.line_code" + :file-hash="diffFile.file_hash" + :file-path="diffFile.file_path" + :line="line" + :is-bottom="index + 1 === diffLinesLength" + :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" + :inline="inline" + /> + <div + v-if="line.renderCommentRow" + :key="`dcr-${line.line_code || index}`" + :class="line.commentRowClasses" + class="diff-grid-comments diff-tr notes_holder" + > + <div v-if="showCommentLeft(line)" class="diff-td notes-content parallel old"> + <diff-comment-cell + v-if="line.left" + :line="line.left" + :diff-file-hash="diffFile.file_hash" + :help-page-path="helpPagePath" + :has-draft="line.left.hasDraft" + line-position="left" + /> + </div> + <div v-if="showCommentRight(line)" class="diff-td notes-content parallel new"> + <diff-comment-cell + v-if="line.right" + :line="line.right" + :diff-file-hash="diffFile.file_hash" + :line-index="index" + :help-page-path="helpPagePath" + :has-draft="line.right.hasDraft" + line-position="right" + /> + </div> + </div> + <div + v-if="shouldRenderParallelDraftRow(diffFile.file_hash, line)" + :key="`drafts-${index}`" + :class="line.draftRowClasses" + class="diff-grid-drafts diff-tr notes_holder" + > + <div + v-if="!inline || (line.left && line.left.lineDraft.isDraft)" + class="diff-td notes-content parallel old" + > + <div v-if="line.left && line.left.lineDraft.isDraft" class="content"> + <draft-note :draft="line.left.lineDraft" :line="line.left" /> + </div> + </div> + <div + v-if="!inline || (line.right && line.right.lineDraft.isDraft)" + class="diff-td notes-content parallel new" + > + <div v-if="line.right && line.right.lineDraft.isDraft" class="content"> + <draft-note :draft="line.right.lineDraft" :line="line.right" /> + </div> + </div> + </div> + </template> + </div> +</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 deleted file mode 100644 index 87f0396cf72..00000000000 --- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue +++ /dev/null @@ -1,82 +0,0 @@ -<script> -import { mapActions } from 'vuex'; -import DiffDiscussions from './diff_discussions.vue'; -import DiffLineNoteForm from './diff_line_note_form.vue'; -import DiffDiscussionReply from './diff_discussion_reply.vue'; - -export default { - components: { - DiffDiscussions, - DiffLineNoteForm, - DiffDiscussionReply, - }, - props: { - line: { - type: Object, - required: true, - }, - diffFileHash: { - type: String, - required: true, - }, - helpPagePath: { - type: String, - required: false, - default: '', - }, - hasDraft: { - type: Boolean, - required: false, - default: false, - }, - }, - computed: { - className() { - return this.line.discussions.length ? '' : 'js-temp-notes-holder'; - }, - shouldRender() { - if (this.line.hasForm) return true; - - if (!this.line.discussions || !this.line.discussions.length) { - return false; - } - return this.line.discussionsExpanded; - }, - }, - methods: { - ...mapActions('diffs', ['showCommentForm']), - }, -}; -</script> - -<template> - <tr v-if="shouldRender" :class="className" class="notes_holder"> - <td class="notes-content" colspan="4"> - <div class="content"> - <diff-discussions - v-if="line.discussions.length" - :line="line" - :discussions="line.discussions" - :help-page-path="helpPagePath" - /> - <diff-discussion-reply - v-if="!hasDraft" - :has-form="line.hasForm" - :render-reply-placeholder="Boolean(line.discussions.length)" - @showNewDiscussionForm=" - showCommentForm({ lineCode: line.line_code, fileHash: diffFileHash }) - " - > - <template #form> - <diff-line-note-form - :diff-file-hash="diffFileHash" - :line="line" - :note-target-line="line" - :help-page-path="helpPagePath" - /> - </template> - </diff-discussion-reply> - </div> - </td> - </tr> -</template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_expansion_row.vue b/app/assets/javascripts/diffs/components/inline_diff_expansion_row.vue deleted file mode 100644 index 071a988d789..00000000000 --- a/app/assets/javascripts/diffs/components/inline_diff_expansion_row.vue +++ /dev/null @@ -1,51 +0,0 @@ -<script> -import DiffExpansionCell from './diff_expansion_cell.vue'; -import { MATCH_LINE_TYPE } from '../constants'; - -export default { - components: { - DiffExpansionCell, - }, - props: { - fileHash: { - type: String, - required: true, - }, - contextLinesPath: { - type: String, - required: true, - }, - line: { - type: Object, - required: true, - }, - isTop: { - type: Boolean, - required: false, - default: false, - }, - isBottom: { - type: Boolean, - required: false, - default: false, - }, - }, - computed: { - isMatchLine() { - return this.line.type === MATCH_LINE_TYPE; - }, - }, -}; -</script> - -<template> - <tr v-if="isMatchLine" class="line_expansion match"> - <diff-expansion-cell - :file-hash="fileHash" - :context-lines-path="contextLinesPath" - :line="line" - :is-top="isTop" - :is-bottom="isBottom" - /> - </tr> -</template> 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 99cf79a70d4..2d8ffb047ca 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -3,7 +3,13 @@ import { mapActions, mapGetters, mapState } from 'vuex'; import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import { CONTEXT_LINE_CLASS_NAME } from '../constants'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; -import * as utils from './diff_row_utils'; +import { + isHighlighted, + shouldShowCommentButton, + shouldRenderCommentButton, + classNameMapCell, + addCommentTooltip, +} from './diff_row_utils'; export default { components: { @@ -48,60 +54,42 @@ export default { ...mapGetters('diffs', ['fileLineCoverage']), ...mapState({ isHighlighted(state) { - return utils.isHighlighted(state, this.line, this.isCommented); + return isHighlighted(state, this.line, this.isCommented); }, }), - isContextLine() { - return utils.isContextLine(this.line.type); - }, classNameMap() { return [ this.line.type, { - [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, + [CONTEXT_LINE_CLASS_NAME]: this.line.isContextLine, }, ]; }, inlineRowId() { return this.line.line_code || `${this.fileHash}_${this.line.old_line}_${this.line.new_line}`; }, - isMatchLine() { - return utils.isMatchLine(this.line.type); - }, coverageState() { return this.fileLineCoverage(this.filePath, this.line.new_line); }, - isMetaLine() { - return utils.isMetaLine(this.line.type); - }, classNameMapCell() { - return utils.classNameMapCell(this.line, this.isHighlighted, this.isLoggedIn, this.isHover); + return classNameMapCell(this.line, this.isHighlighted, this.isLoggedIn, this.isHover); }, addCommentTooltip() { - return utils.addCommentTooltip(this.line); + return addCommentTooltip(this.line); }, shouldRenderCommentButton() { - return utils.shouldRenderCommentButton(this.isLoggedIn, true); + return shouldRenderCommentButton(this.isLoggedIn, true); }, shouldShowCommentButton() { - return utils.shouldShowCommentButton( + return shouldShowCommentButton( this.isHover, - this.isContextLine, - this.isMetaLine, - this.hasDiscussions, + this.line.isContextLine, + this.line.isMetaLine, + this.line.hasDiscussions, ); }, - hasDiscussions() { - return utils.hasDiscussions(this.line); - }, - lineHref() { - return utils.lineHref(this.line); - }, - lineCode() { - return utils.lineCode(this.line); - }, shouldShowAvatarsOnGutter() { - return this.hasDiscussions; + return this.line.hasDiscussions; }, }, mounted() { @@ -128,7 +116,6 @@ export default { <template> <tr - v-if="!isMatchLine" :id="inlineRowId" :class="classNameMap" class="line_holder" @@ -158,8 +145,8 @@ export default { v-if="line.old_line" ref="lineNumberRefOld" :data-linenumber="line.old_line" - :href="lineHref" - @click="setHighlightedRow(lineCode)" + :href="line.lineHref" + @click="setHighlightedRow(line.lineCode)" > </a> <diff-gutter-avatars @@ -167,7 +154,11 @@ export default { :discussions="line.discussions" :discussions-expanded="line.discussionsExpanded" @toggleLineDiscussions=" - toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded }) + toggleLineDiscussions({ + lineCode: line.lineCode, + fileHash, + expanded: !line.discussionsExpanded, + }) " /> </td> @@ -176,8 +167,8 @@ export default { v-if="line.new_line" ref="lineNumberRefNew" :data-linenumber="line.new_line" - :href="lineHref" - @click="setHighlightedRow(lineCode)" + :href="line.lineHref" + @click="setHighlightedRow(line.lineCode)" > </a> </td> diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index 13805910648..05f5461054f 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -2,18 +2,18 @@ import { mapGetters, mapState } from 'vuex'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import draftCommentsMixin from '~/diffs/mixins/draft_comments'; -import InlineDraftCommentRow from '~/batch_comments/components/inline_draft_comment_row.vue'; +import DraftNote from '~/batch_comments/components/draft_note.vue'; import inlineDiffTableRow from './inline_diff_table_row.vue'; -import inlineDiffCommentRow from './inline_diff_comment_row.vue'; -import inlineDiffExpansionRow from './inline_diff_expansion_row.vue'; +import DiffCommentCell from './diff_comment_cell.vue'; +import DiffExpansionCell from './diff_expansion_cell.vue'; import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; export default { components: { - inlineDiffCommentRow, + DiffCommentCell, inlineDiffTableRow, - InlineDraftCommentRow, - inlineDiffExpansionRow, + DraftNote, + DiffExpansionCell, }, mixins: [draftCommentsMixin, glFeatureFlagsMixin()], props: { @@ -65,15 +65,19 @@ export default { </colgroup> <tbody> <template v-for="(line, index) in diffLines"> - <inline-diff-expansion-row - :key="`expand-${index}`" - :file-hash="diffFile.file_hash" - :context-lines-path="diffFile.context_lines_path" - :line="line" - :is-top="index === 0" - :is-bottom="index + 1 === diffLinesLength" - /> + <tr v-if="line.isMatchLine" :key="`expand-${index}`" class="line_expansion match"> + <td colspan="4" class="text-center gl-font-regular"> + <diff-expansion-cell + :file-hash="diffFile.file_hash" + :context-lines-path="diffFile.context_lines_path" + :line="line" + :is-top="index === 0" + :is-bottom="index + 1 === diffLinesLength" + /> + </td> + </tr> <inline-diff-table-row + v-if="!line.isMatchLine" :key="`${line.line_code || index}`" :file-hash="diffFile.file_hash" :file-path="diffFile.file_path" @@ -81,20 +85,32 @@ export default { :is-bottom="index + 1 === diffLinesLength" :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" /> - <inline-diff-comment-row + <tr + v-if="line.renderCommentRow" :key="`icr-${line.line_code || index}`" - :diff-file-hash="diffFile.file_hash" - :line="line" - :help-page-path="helpPagePath" - :has-draft="shouldRenderDraftRow(diffFile.file_hash, line) || false" - /> - <inline-draft-comment-row - v-if="shouldRenderDraftRow(diffFile.file_hash, line)" - :key="`draft_${index}`" - :draft="draftForLine(diffFile.file_hash, line)" - :diff-file="diffFile" - :line="line" - /> + :class="line.commentRowClasses" + class="notes_holder" + > + <td class="notes-content" colspan="4"> + <diff-comment-cell + :diff-file-hash="diffFile.file_hash" + :line="line" + :help-page-path="helpPagePath" + :has-draft="line.hasDraft" + /> + </td> + </tr> + <tr v-if="line.hasDraft" :key="`draft_${index}`" class="notes_holder js-temp-notes-holder"> + <td class="notes-content" colspan="4"> + <div class="content"> + <draft-note + :draft="draftForLine(diffFile.file_hash, line)" + :diff-file="diffFile" + :line="line" + /> + </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 deleted file mode 100644 index 127e3f214cf..00000000000 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ /dev/null @@ -1,175 +0,0 @@ -<script> -import { mapActions } from 'vuex'; -import DiffDiscussions from './diff_discussions.vue'; -import DiffLineNoteForm from './diff_line_note_form.vue'; -import DiffDiscussionReply from './diff_discussion_reply.vue'; - -export default { - components: { - DiffDiscussions, - DiffLineNoteForm, - DiffDiscussionReply, - }, - props: { - line: { - type: Object, - required: true, - }, - diffFileHash: { - type: String, - required: true, - }, - lineIndex: { - type: Number, - required: true, - }, - helpPagePath: { - type: String, - required: false, - default: '', - }, - hasDraftLeft: { - type: Boolean, - required: false, - default: false, - }, - hasDraftRight: { - type: Boolean, - required: false, - default: false, - }, - }, - computed: { - hasExpandedDiscussionOnLeft() { - return this.line.left && this.line.left.discussions.length - ? this.line.left.discussionsExpanded - : false; - }, - hasExpandedDiscussionOnRight() { - return this.line.right && this.line.right.discussions.length - ? this.line.right.discussionsExpanded - : false; - }, - hasAnyExpandedDiscussion() { - return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; - }, - shouldRenderDiscussionsOnLeft() { - return ( - this.line.left && - this.line.left.discussions && - this.line.left.discussions.length && - this.hasExpandedDiscussionOnLeft - ); - }, - shouldRenderDiscussionsOnRight() { - return ( - this.line.right && - this.line.right.discussions && - this.line.right.discussions.length && - this.hasExpandedDiscussionOnRight && - this.line.right.type - ); - }, - showRightSideCommentForm() { - return this.line.right && this.line.right.type && this.line.right.hasForm; - }, - showLeftSideCommentForm() { - return this.line.left && this.line.left.hasForm; - }, - className() { - return (this.left && this.line.left.discussions.length > 0) || - (this.right && this.line.right.discussions.length > 0) - ? '' - : 'js-temp-notes-holder'; - }, - shouldRender() { - const { line } = this; - const hasDiscussion = - (line.left && line.left.discussions && line.left.discussions.length) || - (line.right && line.right.discussions && line.right.discussions.length); - - if ( - hasDiscussion && - (this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight) - ) { - return true; - } - - const hasCommentFormOnLeft = line.left && line.left.hasForm; - const hasCommentFormOnRight = line.right && line.right.hasForm; - - return hasCommentFormOnLeft || hasCommentFormOnRight; - }, - shouldRenderReplyPlaceholderOnLeft() { - return Boolean( - this.line.left && this.line.left.discussions && this.line.left.discussions.length, - ); - }, - shouldRenderReplyPlaceholderOnRight() { - return Boolean( - this.line.right && this.line.right.discussions && this.line.right.discussions.length, - ); - }, - }, - methods: { - ...mapActions('diffs', ['showCommentForm']), - showNewDiscussionForm(lineCode) { - this.showCommentForm({ lineCode, fileHash: this.diffFileHash }); - }, - }, -}; -</script> - -<template> - <tr v-if="shouldRender" :class="className" class="notes_holder"> - <td class="notes-content parallel old" colspan="3"> - <div v-if="shouldRenderDiscussionsOnLeft" class="content"> - <diff-discussions - :discussions="line.left.discussions" - :line="line.left" - :help-page-path="helpPagePath" - /> - </div> - <diff-discussion-reply - v-if="!hasDraftLeft" - :has-form="showLeftSideCommentForm" - :render-reply-placeholder="shouldRenderReplyPlaceholderOnLeft" - @showNewDiscussionForm="showNewDiscussionForm(line.left.line_code)" - > - <template #form> - <diff-line-note-form - :diff-file-hash="diffFileHash" - :line="line.left" - :note-target-line="line.left" - :help-page-path="helpPagePath" - line-position="left" - /> - </template> - </diff-discussion-reply> - </td> - <td class="notes-content parallel new" colspan="3"> - <div v-if="shouldRenderDiscussionsOnRight" class="content"> - <diff-discussions - :discussions="line.right.discussions" - :line="line.right" - :help-page-path="helpPagePath" - /> - </div> - <diff-discussion-reply - v-if="!hasDraftRight" - :has-form="showRightSideCommentForm" - :render-reply-placeholder="shouldRenderReplyPlaceholderOnRight" - @showNewDiscussionForm="showNewDiscussionForm(line.right.line_code)" - > - <template #form> - <diff-line-note-form - :diff-file-hash="diffFileHash" - :line="line.right" - :note-target-line="line.right" - line-position="right" - /> - </template> - </diff-discussion-reply> - </td> - </tr> -</template> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue deleted file mode 100644 index 0a80107ced4..00000000000 --- a/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue +++ /dev/null @@ -1,56 +0,0 @@ -<script> -import { MATCH_LINE_TYPE } from '../constants'; -import DiffExpansionCell from './diff_expansion_cell.vue'; - -export default { - components: { - DiffExpansionCell, - }, - props: { - fileHash: { - type: String, - required: true, - }, - contextLinesPath: { - type: String, - required: true, - }, - line: { - type: Object, - required: true, - }, - isTop: { - type: Boolean, - required: false, - default: false, - }, - isBottom: { - type: Boolean, - required: false, - default: false, - }, - }, - computed: { - isMatchLineLeft() { - return this.line.left && this.line.left.type === MATCH_LINE_TYPE; - }, - isMatchLineRight() { - return this.line.right && this.line.right.type === MATCH_LINE_TYPE; - }, - }, -}; -</script> -<template> - <tr class="line_expansion match"> - <template v-if="isMatchLineLeft || isMatchLineRight"> - <diff-expansion-cell - :file-hash="fileHash" - :context-lines-path="contextLinesPath" - :line="line.left" - :is-top="isTop" - :is-bottom="isBottom" - :colspan="6" - /> - </template> - </tr> -</template> 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 cdc6db791f0..13cd0651ff2 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -55,27 +55,15 @@ export default { return utils.isHighlighted(state, line, this.isCommented); }, }), - isContextLineLeft() { - return utils.isContextLine(this.line.left?.type); - }, - isContextLineRight() { - return utils.isContextLine(this.line.right?.type); - }, classNameMap() { return { - [CONTEXT_LINE_CLASS_NAME]: this.isContextLineLeft, + [CONTEXT_LINE_CLASS_NAME]: this.line.isContextLineLeft, [PARALLEL_DIFF_VIEW_TYPE]: true, }; }, parallelViewLeftLineType() { return utils.parallelViewLeftLineType(this.line, this.isHighlighted); }, - isMatchLineLeft() { - return utils.isMatchLine(this.line.left?.type); - }, - isMatchLineRight() { - return utils.isMatchLine(this.line.right?.type); - }, coverageState() { return this.fileLineCoverage(this.filePath, this.line.right.new_line); }, @@ -107,40 +95,19 @@ export default { shouldShowCommentButtonLeft() { return utils.shouldShowCommentButton( this.isLeftHover, - this.isContextLineLeft, - this.isMetaLineLeft, - this.hasDiscussionsLeft, + this.line.isContextLineLeft, + this.line.isMetaLineLeft, + this.line.hasDiscussionsLeft, ); }, shouldShowCommentButtonRight() { return utils.shouldShowCommentButton( this.isRightHover, - this.isContextLineRight, - this.isMetaLineRight, - this.hasDiscussionsRight, + this.line.isContextLineRight, + this.line.isMetaLineRight, + this.line.hasDiscussionsRight, ); }, - hasDiscussionsLeft() { - return utils.hasDiscussions(this.line.left); - }, - hasDiscussionsRight() { - return utils.hasDiscussions(this.line.right); - }, - lineHrefOld() { - return utils.lineHref(this.line.left); - }, - lineHrefNew() { - return utils.lineHref(this.line.right); - }, - lineCode() { - return utils.lineCode(this.line); - }, - isMetaLineLeft() { - return utils.isMetaLine(this.line.left?.type); - }, - isMetaLineRight() { - return utils.isMetaLine(this.line.right?.type); - }, }, mounted() { this.scrollToLineIfNeededParallel(this.line); @@ -203,7 +170,7 @@ export default { @mouseover="handleMouseMove" @mouseout="handleMouseMove" > - <template v-if="line.left && !isMatchLineLeft"> + <template v-if="line.left && !line.isMatchLineLeft"> <td ref="oldTd" :class="classNameMapCellLeft" class="diff-line-num old_line"> <span v-if="shouldRenderCommentButton" @@ -227,12 +194,12 @@ export default { v-if="line.left.old_line" ref="lineNumberRefOld" :data-linenumber="line.left.old_line" - :href="lineHrefOld" - @click="setHighlightedRow(lineCode)" + :href="line.lineHrefOld" + @click="setHighlightedRow(line.lineCode)" > </a> <diff-gutter-avatars - v-if="hasDiscussionsLeft" + v-if="line.hasDiscussionsLeft" :discussions="line.left.discussions" :discussions-expanded="line.left.discussionsExpanded" @toggleLineDiscussions=" @@ -259,7 +226,7 @@ export default { <td class="line-coverage left-side empty-cell"></td> <td class="line_content with-coverage parallel left-side empty-cell"></td> </template> - <template v-if="line.right && !isMatchLineRight"> + <template v-if="line.right && !line.isMatchLineRight"> <td ref="newTd" :class="classNameMapCellRight" class="diff-line-num new_line"> <span v-if="shouldRenderCommentButton" @@ -283,12 +250,12 @@ export default { v-if="line.right.new_line" ref="lineNumberRefNew" :data-linenumber="line.right.new_line" - :href="lineHrefNew" - @click="setHighlightedRow(lineCode)" + :href="line.lineHrefNew" + @click="setHighlightedRow(line.lineCode)" > </a> <diff-gutter-avatars - v-if="hasDiscussionsRight" + v-if="line.hasDiscussionsRight" :discussions="line.right.discussions" :discussions-expanded="line.right.discussionsExpanded" @toggleLineDiscussions=" diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 46a691ad22d..67b599fe163 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -1,18 +1,18 @@ <script> import { mapGetters, mapState } from 'vuex'; import draftCommentsMixin from '~/diffs/mixins/draft_comments'; -import ParallelDraftCommentRow from '~/batch_comments/components/parallel_draft_comment_row.vue'; +import DraftNote from '~/batch_comments/components/draft_note.vue'; import parallelDiffTableRow from './parallel_diff_table_row.vue'; -import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; -import parallelDiffExpansionRow from './parallel_diff_expansion_row.vue'; +import DiffCommentCell from './diff_comment_cell.vue'; +import DiffExpansionCell from './diff_expansion_cell.vue'; import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; export default { components: { - parallelDiffExpansionRow, + DiffExpansionCell, parallelDiffTableRow, - parallelDiffCommentRow, - ParallelDraftCommentRow, + DiffCommentCell, + DraftNote, }, mixins: [draftCommentsMixin], props: { @@ -66,14 +66,21 @@ export default { </colgroup> <tbody> <template v-for="(line, index) in diffLines"> - <parallel-diff-expansion-row + <tr + v-if="line.isMatchLineLeft || line.isMatchLineRight" :key="`expand-${index}`" - :file-hash="diffFile.file_hash" - :context-lines-path="diffFile.context_lines_path" - :line="line" - :is-top="index === 0" - :is-bottom="index + 1 === diffLinesLength" - /> + class="line_expansion match" + > + <td colspan="6" class="text-center gl-font-regular"> + <diff-expansion-cell + :file-hash="diffFile.file_hash" + :context-lines-path="diffFile.context_lines_path" + :line="line.left" + :is-top="index === 0" + :is-bottom="index + 1 === diffLinesLength" + /> + </td> + </tr> <parallel-diff-table-row :key="line.line_code" :file-hash="diffFile.file_hash" @@ -82,21 +89,53 @@ export default { :is-bottom="index + 1 === diffLinesLength" :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" /> - <parallel-diff-comment-row + <tr + v-if="line.renderCommentRow" :key="`dcr-${line.line_code || index}`" - :line="line" - :diff-file-hash="diffFile.file_hash" - :line-index="index" - :help-page-path="helpPagePath" - :has-draft-left="hasParallelDraftLeft(diffFile.file_hash, line) || false" - :has-draft-right="hasParallelDraftRight(diffFile.file_hash, line) || false" - /> - <parallel-draft-comment-row + :class="line.commentRowClasses" + class="notes_holder" + > + <td class="notes-content parallel old" colspan="3"> + <diff-comment-cell + v-if="line.left" + :line="line.left" + :diff-file-hash="diffFile.file_hash" + :help-page-path="helpPagePath" + :has-draft="line.left.hasDraft" + line-position="left" + /> + </td> + <td class="notes-content parallel new" colspan="3"> + <diff-comment-cell + v-if="line.right" + :line="line.right" + :diff-file-hash="diffFile.file_hash" + :line-index="index" + :help-page-path="helpPagePath" + :has-draft="line.right.hasDraft" + line-position="right" + /> + </td> + </tr> + <tr v-if="shouldRenderParallelDraftRow(diffFile.file_hash, line)" :key="`drafts-${index}`" - :line="line" - :diff-file-content-sha="diffFile.file_hash" - /> + :class="line.draftRowClasses" + class="notes_holder" + > + <td class="notes_line old"></td> + <td class="notes-content parallel old" colspan="2"> + <div v-if="line.left && line.left.lineDraft.isDraft" class="content"> + <draft-note :draft="line.left.lineDraft" :line="line.left" /> + </div> + </td> + <td class="notes_line new"></td> + <td class="notes-content parallel new" colspan="2"> + <div v-if="line.right && line.right.lineDraft.isDraft" class="content"> + <draft-note :draft="line.right.lineDraft" :line="line.right" /> + </div> + </td> + </tr> </template> </tbody> </table> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index dc97d9993da..79f8c08e389 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -73,6 +73,10 @@ export const ALERT_OVERFLOW_HIDDEN = 'overflow'; export const ALERT_MERGE_CONFLICT = 'merge-conflict'; export const ALERT_COLLAPSED_FILES = 'collapsed'; +// Diff File collapse types +export const DIFF_FILE_AUTOMATIC_COLLAPSE = 'automatic'; +export const DIFF_FILE_MANUAL_COLLAPSE = 'manual'; + // State machine states export const STATE_IDLING = 'idle'; export const STATE_LOADING = 'loading'; @@ -91,3 +95,11 @@ export const RENAMED_DIFF_TRANSITIONS = { [`${STATE_ERRORED}:${TRANSITION_LOAD_START}`]: STATE_LOADING, [`${STATE_ERRORED}:${TRANSITION_ACKNOWLEDGE_ERROR}`]: STATE_IDLING, }; + +// MR Diffs known events +export const EVT_EXPAND_ALL_FILES = 'mr:diffs:expandAllFiles'; +export const EVT_PERF_MARK_FILE_TREE_START = 'mr:diffs:perf:fileTreeStart'; +export const EVT_PERF_MARK_FILE_TREE_END = 'mr:diffs:perf:fileTreeEnd'; +export const EVT_PERF_MARK_DIFF_FILES_START = 'mr:diffs:perf:filesStart'; +export const EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN = 'mr:diffs:perf:firstFileShown'; +export const EVT_PERF_MARK_DIFF_FILES_END = 'mr:diffs:perf:filesEnd'; diff --git a/app/assets/javascripts/diffs/diff_file.js b/app/assets/javascripts/diffs/diff_file.js index 933197a2c7f..a14a30b41a9 100644 --- a/app/assets/javascripts/diffs/diff_file.js +++ b/app/assets/javascripts/diffs/diff_file.js @@ -1,4 +1,9 @@ -import { DIFF_FILE_SYMLINK_MODE, DIFF_FILE_DELETED_MODE } from './constants'; +import { + DIFF_FILE_SYMLINK_MODE, + DIFF_FILE_DELETED_MODE, + DIFF_FILE_MANUAL_COLLAPSE, + DIFF_FILE_AUTOMATIC_COLLAPSE, +} from './constants'; function fileSymlinkInformation(file, fileList) { const duplicates = fileList.filter(iteratedFile => iteratedFile.file_hash === file.file_hash); @@ -23,6 +28,7 @@ function collapsed(file) { return { automaticallyCollapsed: viewer.automaticallyCollapsed || viewer.collapsed || false, + manuallyCollapsed: null, }; } @@ -37,3 +43,19 @@ export function prepareRawDiffFile({ file, allFiles }) { return file; } + +export function collapsedType(file) { + const isManual = typeof file.viewer?.manuallyCollapsed === 'boolean'; + + return isManual ? DIFF_FILE_MANUAL_COLLAPSE : DIFF_FILE_AUTOMATIC_COLLAPSE; +} + +export function isCollapsed(file) { + const type = collapsedType(file); + const collapsedStates = { + [DIFF_FILE_AUTOMATIC_COLLAPSE]: file.viewer?.automaticallyCollapsed || false, + [DIFF_FILE_MANUAL_COLLAPSE]: file.viewer?.manuallyCollapsed, + }; + + return collapsedStates[type]; +} diff --git a/app/assets/javascripts/diffs/event_hub.js b/app/assets/javascripts/diffs/event_hub.js new file mode 100644 index 00000000000..3e0c313f5e8 --- /dev/null +++ b/app/assets/javascripts/diffs/event_hub.js @@ -0,0 +1,3 @@ +import eventHubFactory from '~/helpers/event_hub_factory'; + +export default eventHubFactory(); diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index 8699cd88a18..4ec24d452bf 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -1,5 +1,18 @@ import { __ } from '~/locale'; +export const GENERIC_ERROR = __('Something went wrong on our end. Please try again!'); + export const DIFF_FILE_HEADER = { optionsDropdownTitle: __('Options'), }; + +export const DIFF_FILE = { + blobView: __('You can %{linkStart}view the blob%{linkEnd} instead.'), + editInFork: __( + "You're not allowed to %{tag_start}edit%{tag_end} files in this project directly. Please fork this project, make your changes there, and submit a merge request.", + ), + fork: __('Fork'), + cancel: __('Cancel'), + autoCollapsed: __('Files with large changes are collapsed by default.'), + expand: __('Expand file'), +}; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 966b706fc31..91c4c51487f 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -8,7 +8,8 @@ import { __, s__ } from '~/locale'; import { handleLocationHash, historyPushState, scrollToElement } from '~/lib/utils/common_utils'; import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility'; import TreeWorker from '../workers/tree_worker'; -import eventHub from '../../notes/event_hub'; +import notesEventHub from '../../notes/event_hub'; +import eventHub from '../event_hub'; import { getDiffPositionByLineCode, getNoteFormData, @@ -40,8 +41,14 @@ import { DIFF_WHITESPACE_COOKIE_NAME, SHOW_WHITESPACE, NO_SHOW_WHITESPACE, + DIFF_FILE_MANUAL_COLLAPSE, + DIFF_FILE_AUTOMATIC_COLLAPSE, + EVT_PERF_MARK_FILE_TREE_START, + EVT_PERF_MARK_FILE_TREE_END, + EVT_PERF_MARK_DIFF_FILES_START, } from '../constants'; import { diffViewerModes } from '~/ide/constants'; +import { isCollapsed } from '../diff_file'; export const setBaseConfig = ({ commit }, options) => { const { @@ -75,6 +82,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { commit(types.SET_BATCH_LOADING, true); commit(types.SET_RETRIEVING_BATCHES, true); + eventHub.$emit(EVT_PERF_MARK_DIFF_FILES_START); const getBatch = (page = 1) => axios @@ -136,9 +144,11 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { }; commit(types.SET_LOADING, true); + eventHub.$emit(EVT_PERF_MARK_FILE_TREE_START); worker.addEventListener('message', ({ data }) => { commit(types.SET_TREE_DATA, data); + eventHub.$emit(EVT_PERF_MARK_FILE_TREE_END); worker.terminate(); }); @@ -212,7 +222,7 @@ export const assignDiscussionsToDiff = ( } Vue.nextTick(() => { - eventHub.$emit('scrollToDiscussion'); + notesEventHub.$emit('scrollToDiscussion'); }); }; @@ -237,10 +247,17 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi } if (file.viewer.automaticallyCollapsed) { - eventHub.$emit(`loadCollapsedDiff/${file.file_hash}`); + notesEventHub.$emit(`loadCollapsedDiff/${file.file_hash}`); scrollToElement(document.getElementById(file.file_hash)); + } else if (file.viewer.manuallyCollapsed) { + commit(types.SET_FILE_COLLAPSED, { + filePath: file.file_path, + collapsed: false, + trigger: DIFF_FILE_AUTOMATIC_COLLAPSE, + }); + notesEventHub.$emit('scrollToDiscussion'); } else { - eventHub.$emit('scrollToDiscussion'); + notesEventHub.$emit('scrollToDiscussion'); } } } @@ -252,8 +269,7 @@ export const startRenderDiffsQueue = ({ state, commit }) => { const nextFile = state.diffFiles.find( file => !file.renderIt && - (file.viewer && - (!file.viewer.automaticallyCollapsed || file.viewer.name !== diffViewerModes.text)), + (file.viewer && (!isCollapsed(file) || file.viewer.name !== diffViewerModes.text)), ); if (nextFile) { @@ -355,10 +371,6 @@ export const loadCollapsedDiff = ({ commit, getters, state }, file) => }); }); -export const expandAllFiles = ({ commit }) => { - commit(types.EXPAND_ALL_FILES); -}; - /** * Toggles the file discussions after user clicked on the toggle discussions button. * @@ -480,7 +492,7 @@ export const setShowWhitespace = ({ commit }, { showWhitespace, pushState = fals historyPushState(mergeUrlParams({ w }, window.location.href)); } - eventHub.$emit('refetchDiffData'); + notesEventHub.$emit('refetchDiffData'); }; export const toggleFileFinder = ({ commit }, visible) => { @@ -531,15 +543,20 @@ export const setExpandedDiffLines = ({ commit, state }, { file, data }) => { }), }), }; + const unifiedDiffLinesEnabled = window.gon?.features?.unifiedDiffLines; const currentDiffLinesKey = - state.diffViewType === INLINE_DIFF_VIEW_TYPE ? INLINE_DIFF_LINES_KEY : PARALLEL_DIFF_LINES_KEY; + state.diffViewType === INLINE_DIFF_VIEW_TYPE || unifiedDiffLinesEnabled + ? INLINE_DIFF_LINES_KEY + : PARALLEL_DIFF_LINES_KEY; const hiddenDiffLinesKey = state.diffViewType === INLINE_DIFF_VIEW_TYPE ? PARALLEL_DIFF_LINES_KEY : INLINE_DIFF_LINES_KEY; - commit(types.SET_HIDDEN_VIEW_DIFF_FILE_LINES, { - filePath: file.file_path, - lines: expandedDiffLines[hiddenDiffLinesKey], - }); + if (!unifiedDiffLinesEnabled) { + commit(types.SET_HIDDEN_VIEW_DIFF_FILE_LINES, { + filePath: file.file_path, + lines: expandedDiffLines[hiddenDiffLinesKey], + }); + } if (expandedDiffLines[currentDiffLinesKey].length > MAX_RENDERING_DIFF_LINES) { let index = START_RENDERING_INDEX; @@ -621,7 +638,7 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d .then(({ data }) => { const lines = data.map((line, index) => prepareLineForRenamedFile({ - diffViewType: state.diffViewType, + diffViewType: window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType, line, diffFile, index, @@ -633,6 +650,7 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d viewer: { ...diffFile.alternate_viewer, automaticallyCollapsed: false, + manuallyCollapsed: false, }, }); commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: diffFile.file_path, lines }); @@ -641,8 +659,9 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d }); } -export const setFileCollapsed = ({ commit }, { filePath, collapsed }) => - commit(types.SET_FILE_COLLAPSED, { filePath, collapsed }); +export const setFileCollapsedByUser = ({ commit }, { filePath, collapsed }) => { + commit(types.SET_FILE_COLLAPSED, { filePath, collapsed, trigger: DIFF_FILE_MANUAL_COLLAPSE }); +}; export const setSuggestPopoverDismissed = ({ commit, state }) => axios diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 91425c7825b..9ee73998177 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -8,8 +8,16 @@ export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW export const isInlineView = state => state.diffViewType === INLINE_DIFF_VIEW_TYPE; -export const hasCollapsedFile = state => - state.diffFiles.some(file => file.viewer && file.viewer.automaticallyCollapsed); +export const whichCollapsedTypes = state => { + const automatic = state.diffFiles.some(file => file.viewer?.automaticallyCollapsed); + const manual = state.diffFiles.some(file => file.viewer?.manuallyCollapsed); + + return { + any: automatic || manual, + automatic, + manual, + }; +}; export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null); @@ -157,10 +165,13 @@ export const fileLineCoverage = state => (file, line) => { export const currentDiffIndex = state => Math.max(0, state.diffFiles.findIndex(diff => diff.file_hash === state.currentDiffFileId)); -export const diffLines = state => file => { - if (state.diffViewType === INLINE_DIFF_VIEW_TYPE) { +export const diffLines = state => (file, unifiedDiffComponents) => { + if (!unifiedDiffComponents && state.diffViewType === INLINE_DIFF_VIEW_TYPE) { return null; } - return parallelizeDiffLines(file.highlighted_diff_lines || []); + return parallelizeDiffLines( + file.highlighted_diff_lines || [], + state.diffViewType === INLINE_DIFF_VIEW_TYPE, + ); }; diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 5dba2e9d10d..19a9e65edc9 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -13,7 +13,6 @@ export const SET_MERGE_REQUEST_DIFFS = 'SET_MERGE_REQUEST_DIFFS'; export const TOGGLE_LINE_HAS_FORM = 'TOGGLE_LINE_HAS_FORM'; 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 13ecf6a997d..096c4f69439 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -1,6 +1,10 @@ import Vue from 'vue'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; -import { INLINE_DIFF_VIEW_TYPE } from '../constants'; +import { + DIFF_FILE_MANUAL_COLLAPSE, + DIFF_FILE_AUTOMATIC_COLLAPSE, + INLINE_DIFF_VIEW_TYPE, +} from '../constants'; import { findDiffFile, addLineReferences, @@ -16,6 +20,12 @@ function updateDiffFilesInState(state, files) { return Object.assign(state, { diffFiles: files }); } +function renderFile(file) { + Object.assign(file, { + renderIt: true, + }); +} + export default { [types.SET_BASE_CONFIG](state, options) { const { @@ -81,9 +91,7 @@ export default { }, [types.RENDER_FILE](state, file) { - Object.assign(file, { - renderIt: true, - }); + renderFile(file); }, [types.SET_MERGE_REQUEST_DIFFS](state, mergeRequestDiffs) { @@ -168,16 +176,6 @@ export default { Object.assign(selectedFile, { ...newFileData }); }, - [types.EXPAND_ALL_FILES](state) { - state.diffFiles.forEach(file => { - Object.assign(file, { - viewer: Object.assign(file.viewer, { - automaticallyCollapsed: false, - }), - }); - }); - }, - [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode, hash }) { const { latestDiff } = state; @@ -351,11 +349,24 @@ export default { file.isShowingFullFile = true; file.isLoadingFullFile = false; }, - [types.SET_FILE_COLLAPSED](state, { filePath, collapsed }) { + [types.SET_FILE_COLLAPSED]( + state, + { filePath, collapsed, trigger = DIFF_FILE_AUTOMATIC_COLLAPSE }, + ) { const file = state.diffFiles.find(f => f.file_path === filePath); if (file && file.viewer) { - file.viewer.automaticallyCollapsed = collapsed; + if (trigger === DIFF_FILE_MANUAL_COLLAPSE) { + file.viewer.automaticallyCollapsed = false; + file.viewer.manuallyCollapsed = collapsed; + } else if (trigger === DIFF_FILE_AUTOMATIC_COLLAPSE) { + file.viewer.automaticallyCollapsed = collapsed; + file.viewer.manuallyCollapsed = null; + } + } + + if (file && !collapsed) { + renderFile(file); } }, [types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) { @@ -367,8 +378,13 @@ export default { }, [types.SET_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) { const file = state.diffFiles.find(f => f.file_path === filePath); - const currentDiffLinesKey = - state.diffViewType === 'inline' ? 'highlighted_diff_lines' : 'parallel_diff_lines'; + let currentDiffLinesKey; + + if (window.gon?.features?.unifiedDiffLines || state.diffViewType === 'inline') { + currentDiffLinesKey = 'highlighted_diff_lines'; + } else { + currentDiffLinesKey = 'parallel_diff_lines'; + } file[currentDiffLinesKey] = lines; }, diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 69330ffae2f..f87f57c32c3 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -36,9 +36,12 @@ export const isMeta = line => ['match', 'new-nonewline', 'old-nonewline'].includ * * @param {Object[]} diffLines - inline diff lines * + * @param {Boolean} inline - is inline context or not + * * @returns {Object[]} parallel lines */ -export const parallelizeDiffLines = (diffLines = []) => { + +export const parallelizeDiffLines = (diffLines, inline) => { let freeRightIndex = null; const lines = []; @@ -57,7 +60,7 @@ export const parallelizeDiffLines = (diffLines = []) => { } index += 1; } else if (isAdded(line)) { - if (freeRightIndex !== null) { + if (freeRightIndex !== null && !inline) { // If an old line came before this without a line on the right, this // line can be put to the right of it. lines[freeRightIndex].right = line; @@ -664,6 +667,7 @@ export const generateTreeList = files => { addedLines: file.added_lines, removedLines: file.removed_lines, parentPath: parent ? `${parent.path}/` : '/', + submodule: file.submodule, }); } else { Object.assign(entry, { diff --git a/app/assets/javascripts/diffs/utils/performance.js b/app/assets/javascripts/diffs/utils/performance.js new file mode 100644 index 00000000000..dcde6f4ecc4 --- /dev/null +++ b/app/assets/javascripts/diffs/utils/performance.js @@ -0,0 +1,80 @@ +import { performanceMarkAndMeasure } from '~/performance/utils'; +import { + MR_DIFFS_MARK_FILE_TREE_START, + MR_DIFFS_MARK_FILE_TREE_END, + MR_DIFFS_MARK_DIFF_FILES_START, + MR_DIFFS_MARK_FIRST_DIFF_FILE_SHOWN, + MR_DIFFS_MARK_DIFF_FILES_END, + MR_DIFFS_MEASURE_FILE_TREE_DONE, + MR_DIFFS_MEASURE_DIFF_FILES_DONE, +} from '../../performance/constants'; + +import eventHub from '../event_hub'; +import { + EVT_PERF_MARK_FILE_TREE_START, + EVT_PERF_MARK_FILE_TREE_END, + EVT_PERF_MARK_DIFF_FILES_START, + EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN, + EVT_PERF_MARK_DIFF_FILES_END, +} from '../constants'; + +function treeStart() { + performanceMarkAndMeasure({ + mark: MR_DIFFS_MARK_FILE_TREE_START, + }); +} + +function treeEnd() { + performanceMarkAndMeasure({ + mark: MR_DIFFS_MARK_FILE_TREE_END, + measures: [ + { + name: MR_DIFFS_MEASURE_FILE_TREE_DONE, + start: MR_DIFFS_MARK_FILE_TREE_START, + end: MR_DIFFS_MARK_FILE_TREE_END, + }, + ], + }); +} + +function filesStart() { + performanceMarkAndMeasure({ + mark: MR_DIFFS_MARK_DIFF_FILES_START, + }); +} + +function filesEnd() { + performanceMarkAndMeasure({ + mark: MR_DIFFS_MARK_DIFF_FILES_END, + measures: [ + { + name: MR_DIFFS_MEASURE_DIFF_FILES_DONE, + start: MR_DIFFS_MARK_DIFF_FILES_START, + end: MR_DIFFS_MARK_DIFF_FILES_END, + }, + ], + }); +} + +function firstFile() { + performanceMarkAndMeasure({ + mark: MR_DIFFS_MARK_FIRST_DIFF_FILE_SHOWN, + }); +} + +export const diffsApp = { + instrument() { + eventHub.$on(EVT_PERF_MARK_FILE_TREE_START, treeStart); + eventHub.$on(EVT_PERF_MARK_FILE_TREE_END, treeEnd); + eventHub.$on(EVT_PERF_MARK_DIFF_FILES_START, filesStart); + eventHub.$on(EVT_PERF_MARK_DIFF_FILES_END, filesEnd); + eventHub.$on(EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN, firstFile); + }, + deinstrument() { + eventHub.$off(EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN, firstFile); + eventHub.$off(EVT_PERF_MARK_DIFF_FILES_END, filesEnd); + eventHub.$off(EVT_PERF_MARK_DIFF_FILES_START, filesStart); + eventHub.$off(EVT_PERF_MARK_FILE_TREE_END, treeEnd); + eventHub.$off(EVT_PERF_MARK_FILE_TREE_START, treeStart); + }, +}; |