diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
33 files changed, 1052 insertions, 467 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 5062006424e..dd5addbf1e3 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -1,6 +1,6 @@ <script> import { mapState, mapGetters, mapActions } from 'vuex'; -import { GlLoadingIcon, GlButtonGroup, GlButton, GlAlert } from '@gitlab/ui'; +import { GlLoadingIcon, GlPagination, GlSprintf } from '@gitlab/ui'; import Mousetrap from 'mousetrap'; import { __ } from '~/locale'; import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; @@ -13,9 +13,13 @@ import eventHub from '../../notes/event_hub'; import CompareVersions from './compare_versions.vue'; import DiffFile from './diff_file.vue'; import NoChanges from './no_changes.vue'; -import HiddenFilesWarning from './hidden_files_warning.vue'; import CommitWidget from './commit_widget.vue'; import TreeList from './tree_list.vue'; + +import HiddenFilesWarning from './hidden_files_warning.vue'; +import MergeConflictWarning from './merge_conflict_warning.vue'; +import CollapsedFilesWarning from './collapsed_files_warning.vue'; + import { TREE_LIST_WIDTH_STORAGE_KEY, INITIAL_TREE_WIDTH, @@ -24,6 +28,9 @@ import { TREE_HIDE_STATS_WIDTH, MR_TREE_SHOW_KEY, CENTERED_LIMITED_CONTAINER_CLASSES, + ALERT_OVERFLOW_HIDDEN, + ALERT_MERGE_CONFLICT, + ALERT_COLLAPSED_FILES, } from '../constants'; export default { @@ -33,15 +40,21 @@ export default { DiffFile, NoChanges, HiddenFilesWarning, + MergeConflictWarning, + CollapsedFilesWarning, CommitWidget, TreeList, GlLoadingIcon, PanelResizer, - GlButtonGroup, - GlButton, - GlAlert, + GlPagination, + GlSprintf, }, mixins: [glFeatureFlagsMixin()], + alerts: { + ALERT_OVERFLOW_HIDDEN, + ALERT_MERGE_CONFLICT, + ALERT_COLLAPSED_FILES, + }, props: { endpoint: { type: String, @@ -111,6 +124,7 @@ export default { return { treeWidth, diffFilesLength: 0, + collapsedWarningDismissed: false, }; }, computed: { @@ -139,7 +153,7 @@ export default { 'canMerge', 'hasConflicts', ]), - ...mapGetters('diffs', ['isParallelView', 'currentDiffIndex']), + ...mapGetters('diffs', ['hasCollapsedFile', 'isParallelView', 'currentDiffIndex']), ...mapGetters(['isNotesFetched', 'getNoteableData']), diffs() { if (!this.viewDiffsFileByFile) { @@ -169,6 +183,39 @@ export default { isDiffHead() { return parseBoolean(getParameterByName('diff_head')); }, + showFileByFileNavigation() { + return this.diffFiles.length > 1 && this.viewDiffsFileByFile; + }, + currentFileNumber() { + return this.currentDiffIndex + 1; + }, + previousFileNumber() { + const { currentDiffIndex } = this; + + return currentDiffIndex >= 1 ? currentDiffIndex : null; + }, + nextFileNumber() { + const { currentFileNumber, diffFiles } = this; + + return currentFileNumber < diffFiles.length ? currentFileNumber + 1 : null; + }, + visibleWarning() { + let visible = false; + + if (this.renderOverflowWarning) { + 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 + ) { + visible = this.$options.alerts.ALERT_COLLAPSED_FILES; + } + + return visible; + }, }, watch: { commit(newCommit, oldCommit) { @@ -186,7 +233,7 @@ export default { } }, diffViewType() { - if (this.needsReload() || this.needsFirstLoad()) { + if (!this.glFeatures.unifiedDiffLines && (this.needsReload() || this.needsFirstLoad())) { this.refetchDiffData(); } this.adjustView(); @@ -212,7 +259,6 @@ export default { projectPath: this.projectPath, dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, - useSingleDiffStyle: this.glFeatures.singleMrDiffView, viewDiffsFileByFile: this.viewDiffsFileByFile, }); @@ -262,7 +308,6 @@ export default { ...mapActions('diffs', [ 'moveToNeighboringCommit', 'setBaseConfig', - 'fetchDiffFiles', 'fetchDiffFilesMeta', 'fetchDiffFilesBatch', 'fetchCoverageFiles', @@ -274,6 +319,9 @@ export default { 'toggleShowTreeList', 'navigateToDiffFileIndex', ]), + navigateToDiffFileNumber(number) { + this.navigateToDiffFileIndex(number - 1); + }, refetchDiffData() { this.fetchData(false); }, @@ -286,60 +334,35 @@ export default { ); }, needsReload() { - return ( - this.glFeatures.singleMrDiffView && - this.diffFiles.length && - isSingleViewStyle(this.diffFiles[0]) - ); + return this.diffFiles.length && isSingleViewStyle(this.diffFiles[0]); }, needsFirstLoad() { - return this.glFeatures.singleMrDiffView && !this.diffFiles.length; + return !this.diffFiles.length; }, fetchData(toggleTree = true) { - if (this.glFeatures.diffsBatchLoad) { - this.fetchDiffFilesMeta() - .then(({ real_size }) => { - this.diffFilesLength = parseInt(real_size, 10); - if (toggleTree) this.hideTreeListIfJustOneFile(); + this.fetchDiffFilesMeta() + .then(({ real_size }) => { + this.diffFilesLength = parseInt(real_size, 10); + if (toggleTree) this.hideTreeListIfJustOneFile(); - this.startDiffRendering(); - }) - .catch(() => { - createFlash(__('Something went wrong on our end. Please try again!')); - }); - - this.fetchDiffFilesBatch() - .then(() => { - // Guarantee the discussions are assigned after the batch finishes. - // Just watching the length of the discussions or the diff files - // isn't enough, because with split diff loading, neither will - // change when loading the other half of the diff files. - this.setDiscussions(); - }) - .then(() => this.startDiffRendering()) - .catch(() => { - createFlash(__('Something went wrong on our end. Please try again!')); - }); - } else { - this.fetchDiffFiles() - .then(({ real_size }) => { - this.diffFilesLength = parseInt(real_size, 10); - if (toggleTree) { - this.hideTreeListIfJustOneFile(); - } + this.startDiffRendering(); + }) + .catch(() => { + createFlash(__('Something went wrong on our end. Please try again!')); + }); - requestIdleCallback( - () => { - this.setDiscussions(); - this.startRenderDiffsQueue(); - }, - { timeout: 1000 }, - ); - }) - .catch(() => { - createFlash(__('Something went wrong on our end. Please try again!')); - }); - } + this.fetchDiffFilesBatch() + .then(() => { + // Guarantee the discussions are assigned after the batch finishes. + // Just watching the length of the discussions or the diff files + // isn't enough, because with split diff loading, neither will + // change when loading the other half of the diff files. + this.setDiscussions(); + }) + .then(() => this.startDiffRendering()) + .catch(() => { + createFlash(__('Something went wrong on our end. Please try again!')); + }); if (this.endpointCoverage) { this.fetchCoverageFiles(); @@ -406,6 +429,9 @@ export default { this.toggleShowTreeList(false); } }, + dismissCollapsedWarning() { + this.collapsedWarningDismissed = true; + }, }, minTreeWidth: MIN_TREE_WIDTH, maxTreeWidth: MAX_TREE_WIDTH, @@ -423,59 +449,27 @@ export default { /> <hidden-files-warning - v-if="renderOverflowWarning" + v-if="visibleWarning == $options.alerts.ALERT_OVERFLOW_HIDDEN" :visible="numVisibleFiles" :total="numTotalFiles" :plain-diff-path="plainDiffPath" :email-patch-path="emailPatchPath" /> - - <div - v-if="isDiffHead && hasConflicts" - :class="{ - [CENTERED_LIMITED_CONTAINER_CLASSES]: isLimitedContainer, - }" - > - <gl-alert - :dismissible="false" - :title="__('There are merge conflicts')" - variant="warning" - class="w-100 mb-3" - > - <p class="mb-1"> - {{ __('The comparison view may be inaccurate due to merge conflicts.') }} - </p> - <p class="mb-0"> - {{ - __( - 'Resolve these conflicts or ask someone with write access to this repository to merge it locally.', - ) - }} - </p> - <template #actions> - <gl-button - v-if="conflictResolutionPath" - :href="conflictResolutionPath" - variant="info" - class="mr-3 gl-alert-action" - > - {{ __('Resolve conflicts') }} - </gl-button> - <gl-button - v-if="canMerge" - class="gl-alert-action" - data-toggle="modal" - data-target="#modal_merge_info" - > - {{ __('Merge locally') }} - </gl-button> - </template> - </gl-alert> - </div> + <merge-conflict-warning + v-if="visibleWarning == $options.alerts.ALERT_MERGE_CONFLICT" + :limited="isLimitedContainer" + :resolution-path="conflictResolutionPath" + :mergeable="canMerge" + /> + <collapsed-files-warning + v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES" + :limited="isLimitedContainer" + @dismiss="dismissCollapsedWarning" + /> <div :data-can-create-note="getNoteableData.current_user.can_create_note" - class="files d-flex" + class="files d-flex gl-mt-2" > <div v-if="showTreeList" @@ -509,23 +503,22 @@ export default { :can-current-user-fork="canCurrentUserFork" :view-diffs-file-by-file="viewDiffsFileByFile" /> - <div v-if="viewDiffsFileByFile" class="d-flex gl-justify-content-center"> - <gl-button-group> - <gl-button - :disabled="currentDiffIndex === 0" - data-testid="singleFilePrevious" - @click="navigateToDiffFileIndex(currentDiffIndex - 1)" - > - {{ __('Prev') }} - </gl-button> - <gl-button - :disabled="currentDiffIndex === diffFiles.length - 1" - data-testid="singleFileNext" - @click="navigateToDiffFileIndex(currentDiffIndex + 1)" - > - {{ __('Next') }} - </gl-button> - </gl-button-group> + <div + v-if="showFileByFileNavigation" + data-testid="file-by-file-navigation" + class="gl-display-grid gl-text-center" + > + <gl-pagination + class="gl-mx-auto" + :value="currentFileNumber" + :prev-page="previousFileNumber" + :next-page="nextFileNumber" + @input="navigateToDiffFileNumber" + /> + <gl-sprintf :message="__('File %{current} of %{total}')"> + <template #current>{{ currentFileNumber }}</template> + <template #total>{{ diffFiles.length }}</template> + </gl-sprintf> </div> </template> <no-changes v-else :changes-empty-state-illustration="changesEmptyStateIllustration" /> diff --git a/app/assets/javascripts/diffs/components/collapsed_files_warning.vue b/app/assets/javascripts/diffs/components/collapsed_files_warning.vue new file mode 100644 index 00000000000..dded3643115 --- /dev/null +++ b/app/assets/javascripts/diffs/components/collapsed_files_warning.vue @@ -0,0 +1,71 @@ +<script> +import { mapActions } from 'vuex'; + +import { GlAlert, GlButton } from '@gitlab/ui'; + +import { CENTERED_LIMITED_CONTAINER_CLASSES } from '../constants'; + +export default { + components: { + GlAlert, + GlButton, + }, + props: { + limited: { + type: Boolean, + required: false, + default: false, + }, + dismissed: { + type: Boolean, + required: false, + default: false, + }, + }, + data() { + return { + isDismissed: this.dismissed, + }; + }, + computed: { + containerClasses() { + return { + [CENTERED_LIMITED_CONTAINER_CLASSES]: this.limited, + }; + }, + }, + + methods: { + ...mapActions('diffs', ['expandAllFiles']), + dismiss() { + this.isDismissed = true; + this.$emit('dismiss'); + }, + expand() { + this.expandAllFiles(); + this.dismiss(); + }, + }, +}; +</script> + +<template> + <div v-if="!isDismissed" data-testid="root" :class="containerClasses"> + <gl-alert + :dismissible="true" + :title="__('Some changes are not shown')" + variant="warning" + class="gl-mb-5" + @dismiss="dismiss" + > + <p class="gl-mb-0"> + {{ __('For a faster browsing experience, some files are collapsed by default.') }} + </p> + <template #actions> + <gl-button category="secondary" variant="warning" class="gl-alert-action" @click="expand"> + {{ __('Expand all files') }} + </gl-button> + </template> + </gl-alert> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index 274a4027e62..23669eecce2 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -1,11 +1,11 @@ <script> +/* eslint-disable vue/no-v-html */ import { mapActions } from 'vuex'; import { GlButtonGroup, GlButton, GlIcon, GlTooltipDirective } from '@gitlab/ui'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; -import Icon from '~/vue_shared/components/icon.vue'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; @@ -39,7 +39,6 @@ import { setUrlParams } from '../../lib/utils/url_utility'; export default { components: { UserAvatarLink, - Icon, ClipboardButton, TimeAgoTooltip, CommitPipelineStatus, @@ -150,14 +149,13 @@ export default { <span class="commit-row-message d-block d-sm-none">· {{ commit.short_id }}</span> - <button + <gl-button v-if="commit.description_html && collapsible" - class="text-expander js-toggle-button" - type="button" + class="js-toggle-button" + size="small" + icon="ellipsis_h" :aria-label="__('Toggle commit description')" - > - <icon :size="12" name="ellipsis_h" /> - </button> + /> <div class="committer"> <a diff --git a/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue b/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue index ed4edabd81c..8263e938e69 100644 --- a/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue +++ b/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue @@ -1,10 +1,10 @@ <script> -import Icon from '~/vue_shared/components/icon.vue'; +import { GlIcon } from '@gitlab/ui'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; export default { components: { - Icon, + GlIcon, TimeAgo, }, props: { @@ -29,7 +29,7 @@ export default { aria-expanded="false" > <span> {{ selectedVersionName }} </span> - <icon :size="12" name="angle-down" class="position-absolute" /> + <gl-icon :size="12" name="angle-down" class="position-absolute" /> </a> <div class="dropdown-menu dropdown-select dropdown-menu-selectable"> <div class="dropdown-content"> diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index 35e4527af69..b94874c5644 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -1,9 +1,8 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; -import { GlTooltipDirective, GlLink, GlDeprecatedButton, GlSprintf } from '@gitlab/ui'; +import { GlTooltipDirective, GlLink, GlButton, GlSprintf } from '@gitlab/ui'; import { __ } from '~/locale'; import { polyfillSticky } from '~/lib/utils/sticky'; -import Icon from '~/vue_shared/components/icon.vue'; import CompareDropdownLayout from './compare_dropdown_layout.vue'; import SettingsDropdown from './settings_dropdown.vue'; import DiffStats from './diff_stats.vue'; @@ -12,9 +11,8 @@ import { CENTERED_LIMITED_CONTAINER_CLASSES } from '../constants'; export default { components: { CompareDropdownLayout, - Icon, GlLink, - GlDeprecatedButton, + GlButton, GlSprintf, SettingsDropdown, DiffStats, @@ -84,18 +82,15 @@ export default { [CENTERED_LIMITED_CONTAINER_CLASSES]: isLimitedContainer, }" > - <button + <gl-button v-gl-tooltip.hover - type="button" - class="btn btn-default gl-mr-3 js-toggle-tree-list" - :class="{ - active: showTreeList, - }" + variant="default" + icon="file-tree" + class="gl-mr-3 js-toggle-tree-list" :title="toggleFileBrowserTitle" + :selected="showTreeList" @click="toggleShowTreeList" - > - <icon name="file-tree" /> - </button> + /> <gl-sprintf v-if="showDropdowns" class="d-flex align-items-center compare-versions-container" @@ -124,16 +119,22 @@ export default { :added-lines="addedLines" :removed-lines="removedLines" /> - <gl-deprecated-button + <gl-button v-if="commit || startVersion" :href="latestVersionPath" + variant="default" class="gl-mr-3 js-latest-version" > {{ __('Show latest version') }} - </gl-deprecated-button> - <gl-deprecated-button v-show="hasCollapsedFile" class="gl-mr-3" @click="expandAllFiles"> + </gl-button> + <gl-button + v-show="hasCollapsedFile" + variant="default" + class="gl-mr-3" + @click="expandAllFiles" + > {{ __('Expand all') }} - </gl-deprecated-button> + </gl-button> <settings-dropdown /> </div> </div> diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index 087a558efdc..9ecb9a44443 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -1,6 +1,7 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; import { GlLoadingIcon } from '@gitlab/ui'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import diffLineNoteFormMixin from '~/notes/mixins/diff_line_note_form'; import draftCommentsMixin from '~/diffs/mixins/draft_comments'; import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue'; @@ -32,7 +33,7 @@ export default { userAvatarLink, DiffFileDrafts, }, - mixins: [diffLineNoteFormMixin, draftCommentsMixin], + mixins: [diffLineNoteFormMixin, draftCommentsMixin, glFeatureFlagsMixin()], props: { diffFile: { type: Object, @@ -48,8 +49,12 @@ export default { ...mapState({ projectPath: state => state.diffs.projectPath, }), - ...mapGetters('diffs', ['isInlineView', 'isParallelView']), - ...mapGetters('diffs', ['getCommentFormForDiffFile']), + ...mapGetters('diffs', [ + 'isInlineView', + 'isParallelView', + 'getCommentFormForDiffFile', + 'diffLines', + ]), ...mapGetters(['getNoteableData', 'noteableType', 'getUserData']), diffMode() { return getDiffMode(this.diffFile); @@ -114,13 +119,15 @@ export default { <inline-diff-view v-if="isInlineView" :diff-file="diffFile" - :diff-lines="diffFile.highlighted_diff_lines || []" + :diff-lines="diffFile.highlighted_diff_lines" :help-page-path="helpPagePath" /> <parallel-diff-view v-else-if="isParallelView" :diff-file="diffFile" - :diff-lines="diffFile.parallel_diff_lines || []" + :diff-lines=" + glFeatures.unifiedDiffLines ? diffLines(diffFile) : diffFile.parallel_diff_lines || [] + " :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_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index b6a0724c201..7b55bd2104d 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -1,12 +1,12 @@ <script> import { mapActions } from 'vuex'; -import Icon from '~/vue_shared/components/icon.vue'; +import { GlIcon } from '@gitlab/ui'; import noteableDiscussion from '../../notes/components/noteable_discussion.vue'; export default { components: { noteableDiscussion, - Icon, + GlIcon, }, props: { discussions: { @@ -70,7 +70,7 @@ export default { class="js-diff-notes-toggle" @click="toggleDiscussion({ discussionId: discussion.id })" > - <icon v-if="discussion.expanded" name="collapse" class="collapse-icon" /> + <gl-icon v-if="discussion.expanded" name="collapse" class="collapse-icon" /> <template v-else> {{ index + 1 }} </template> diff --git a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue index e5e63bdcb43..0094b4f8707 100644 --- a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue @@ -1,8 +1,9 @@ <script> import { mapState, mapActions } from 'vuex'; +import { GlIcon } from '@gitlab/ui'; import { deprecatedCreateFlash as createFlash } from '~/flash'; -import { s__ } from '~/locale'; -import Icon from '~/vue_shared/components/icon.vue'; +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'; @@ -17,17 +18,23 @@ const lineNumberByViewType = (viewType, diffLine) => { [PARALLEL_DIFF_VIEW_TYPE]: line => (line?.right || line?.left)?.new_line, }; const numberGetter = numberGetters[viewType]; - return numberGetter && numberGetter(diffLine); }; +const i18n = { + showMore: sprintf(s__('Diffs|Show %{unfoldCount} lines'), { unfoldCount: UNFOLD_COUNT }), + showAll: s__('Diffs|Show all unchanged lines'), +}; + export default { + i18n, directives: { tooltip, }, components: { - Icon, + GlIcon, }, + mixins: [glFeatureFlagsMixin()], props: { fileHash: { type: String, @@ -59,7 +66,9 @@ export default { }, computed: { ...mapState({ - diffViewType: state => state.diffs.diffViewType, + diffViewType(state) { + return this.glFeatures.unifiedDiffLines ? INLINE_DIFF_VIEW_TYPE : state.diffs.diffViewType; + }, diffFiles: state => state.diffs.diffFiles, }), canExpandUp() { @@ -226,32 +235,27 @@ export default { </script> <template> - <td :colspan="colspan" class="text-center"> + <td :colspan="colspan" class="text-center gl-font-regular"> <div class="content js-line-expansion-content"> <a - v-if="canExpandUp" - v-tooltip - class="cursor-pointer js-unfold unfold-icon d-inline-block pt-2 pb-2" - data-placement="top" - data-container="body" - :title="__('Expand up')" - @click="handleExpandLines(EXPAND_UP)" + v-if="canExpandDown" + class="gl-mx-2 gl-cursor-pointer js-unfold-down gl-display-inline-block gl-py-4" + @click="handleExpandLines(EXPAND_DOWN)" > - <icon :size="12" name="expand-up" aria-hidden="true" /> + <gl-icon :size="12" name="expand-down" aria-hidden="true" /> + <span>{{ $options.i18n.showMore }}</span> </a> - <a class="mx-2 cursor-pointer js-unfold-all" @click="handleExpandLines()"> - <span>{{ s__('Diffs|Show unchanged lines') }}</span> + <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="canExpandDown" - v-tooltip - class="cursor-pointer js-unfold-down has-tooltip unfold-icon d-inline-block pt-2 pb-2" - data-placement="top" - data-container="body" - :title="__('Expand down')" - @click="handleExpandLines(EXPAND_DOWN)" + v-if="canExpandUp" + class="gl-mx-2 gl-cursor-pointer js-unfold gl-display-inline-block gl-py-4" + @click="handleExpandLines(EXPAND_UP)" > - <icon :size="12" name="expand-down" aria-hidden="true" /> + <gl-icon :size="12" name="expand-up" aria-hidden="true" /> + <span>{{ $options.i18n.showMore }}</span> </a> </div> </td> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index eace673c2d7..9a7ed76bad3 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -1,23 +1,32 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; import { escape } from 'lodash'; -import { GlLoadingIcon } 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 DiffFileHeader from './diff_file_header.vue'; import DiffContent from './diff_content.vue'; import { diffViewerErrors } from '~/ide/constants'; +import { GENERIC_ERROR, DIFF_FILE } from '../i18n'; export default { components: { DiffFileHeader, DiffContent, + GlButton, GlLoadingIcon, }, + directives: { + SafeHtml, + }, mixins: [glFeatureFlagsMixin()], + i18n: { + genericError: GENERIC_ERROR, + ...DIFF_FILE, + }, props: { file: { type: Object, @@ -50,7 +59,7 @@ export default { ...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>', @@ -72,9 +81,7 @@ export default { }, 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>', @@ -93,11 +100,7 @@ export default { }, 'file.file_hash': { handler: function watchFileHash() { - if ( - this.glFeatures.autoExpandCollapsedDiffs && - this.viewDiffsFileByFile && - this.file.viewer.collapsed - ) { + if (this.viewDiffsFileByFile && this.file.viewer.collapsed) { this.isCollapsed = false; this.handleLoadCollapsedDiff(); } else { @@ -107,7 +110,7 @@ export default { immediate: true, }, 'file.viewer.collapsed': function setIsCollapsed(newVal) { - if (!this.viewDiffsFileByFile && !this.glFeatures.autoExpandCollapsedDiffs) { + if (!this.viewDiffsFileByFile) { this.isCollapsed = newVal; } }, @@ -149,7 +152,7 @@ export default { }) .catch(() => { this.isLoadingCollapsedDiff = false; - createFlash(__('Something went wrong on our end. Please try again!')); + createFlash(this.$options.i18n.genericError); }); }, showForkMessage() { @@ -185,32 +188,38 @@ export default { /> <div v-if="forkMessageVisible" class="js-file-fork-suggestion-section file-fork-suggestion"> - <span class="file-fork-suggestion-note" v-html="forkMessage"></span> + <span v-safe-html="forkMessage" class="file-fork-suggestion-note"></span> <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 class="nothing-here-block" v-html="errorMessage"></div> + <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="isCollapsed" class="gl-p-7 gl-text-center collapsed-file-warning"> + <p class="gl-mb-8 gl-mt-5"> + {{ $options.i18n.collapsed }} + </p> + <gl-button + class="gl-alert-action gl-mb-5" + data-testid="expandButton" + @click="handleToggle" + > + {{ $options.i18n.expand }} + </gl-button> </div> <diff-content v-show="!isCollapsed && !isFileTooLarge" diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 5727fbaaf68..fded391cc84 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -1,9 +1,16 @@ <script> +/* eslint-disable vue/no-v-html */ import { escape } from 'lodash'; import { mapActions, mapGetters } from 'vuex'; -import { GlDeprecatedButton, GlTooltipDirective, GlLoadingIcon } from '@gitlab/ui'; +import { + GlDeprecatedButton, + GlTooltipDirective, + GlSafeHtmlDirective, + GlLoadingIcon, + GlIcon, + GlButton, +} from '@gitlab/ui'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; -import Icon from '~/vue_shared/components/icon.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue'; import { truncateSha } from '~/lib/utils/text_utility'; import { __, s__, sprintf } from '~/locale'; @@ -18,12 +25,14 @@ export default { GlDeprecatedButton, ClipboardButton, EditButton, - Icon, + GlIcon, FileIcon, DiffStats, + GlButton, }, directives: { GlTooltip: GlTooltipDirective, + SafeHtml: GlSafeHtmlDirective, }, props: { discussionPath: { @@ -77,6 +86,21 @@ export default { return this.discussionPath; }, + submoduleDiffCompareLinkText() { + if (this.diffFile.submodule_compare) { + const truncatedOldSha = escape(truncateSha(this.diffFile.submodule_compare.old_sha)); + const truncatedNewSha = escape(truncateSha(this.diffFile.submodule_compare.new_sha)); + return sprintf( + s__('Compare %{oldCommitId}...%{newCommitId}'), + { + oldCommitId: `<span class="commit-sha">${truncatedOldSha}</span>`, + newCommitId: `<span class="commit-sha">${truncatedNewSha}</span>`, + }, + false, + ); + } + return null; + }, filePath() { if (this.diffFile.submodule) { return `${this.diffFile.file_path} @ ${truncateSha(this.diffFile.blob.id)}`; @@ -133,6 +157,7 @@ export default { 'toggleFileDiscussions', 'toggleFileDiscussionWrappers', 'toggleFullDiff', + 'toggleActiveFileByHash', ]), handleToggleFile() { this.$emit('toggleFile'); @@ -149,6 +174,9 @@ export default { const selector = this.diffContentIDSelector; scrollToElement(document.querySelector(selector)); window.location.hash = selector; + if (!this.viewDiffsFileByFile) { + this.toggleActiveFileByHash(this.diffFile.file_hash); + } } }, }, @@ -162,7 +190,7 @@ export default { @click.self="handleToggleFile" > <div class="file-header-content"> - <icon + <gl-icon v-if="collapsible" ref="collapseIcon" :name="collapseIcon" @@ -237,7 +265,7 @@ export default { type="button" @click="toggleFileDiscussionWrappers(diffFile)" > - <icon name="comment" /> + <gl-icon name="comment" /> </gl-deprecated-button> </span> @@ -273,8 +301,8 @@ export default { @click="toggleFullDiff(diffFile.file_path)" > <gl-loading-icon v-if="diffFile.isLoadingFullFile" color="dark" inline /> - <icon v-else-if="diffFile.isShowingFullFile" name="doc-changes" /> - <icon v-else name="doc-expand" /> + <gl-icon v-else-if="diffFile.isShowingFullFile" name="doc-changes" /> + <gl-icon v-else name="doc-expand" /> </gl-deprecated-button> <gl-deprecated-button ref="viewButton" @@ -287,7 +315,7 @@ export default { data-track-property="diff_toggle_view_sha" :title="viewFileButtonText" > - <icon name="doc-text" /> + <gl-icon name="doc-text" /> </gl-deprecated-button> <a @@ -303,9 +331,22 @@ export default { data-track-property="diff_toggle_external" class="btn btn-file-option" > - <icon name="external-link" /> + <gl-icon name="external-link" /> </a> </div> </div> + + <div + v-if="diffFile.submodule_compare" + class="file-actions d-none d-sm-flex align-items-center flex-wrap" + > + <gl-button + v-gl-tooltip.hover + v-safe-html="submoduleDiffCompareLinkText" + class="submodule-compare" + :title="s__('Compare submodule commit revisions')" + :href="diffFile.submodule_compare.url" + /> + </div> </div> </template> diff --git a/app/assets/javascripts/diffs/components/diff_file_row.vue b/app/assets/javascripts/diffs/components/diff_file_row.vue index 43b669625f4..2856e6ae8eb 100644 --- a/app/assets/javascripts/diffs/components/diff_file_row.vue +++ b/app/assets/javascripts/diffs/components/diff_file_row.vue @@ -3,9 +3,10 @@ * This component is an iterative step towards refactoring and simplifying `vue_shared/components/file_row.vue` * https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23720 */ +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import FileRow from '~/vue_shared/components/file_row.vue'; -import FileRowStats from './file_row_stats.vue'; import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue'; +import FileRowStats from './file_row_stats.vue'; export default { name: 'DiffFileRow', @@ -14,6 +15,7 @@ export default { FileRowStats, ChangedFileIcon, }, + mixins: [glFeatureFlagsMixin()], props: { file: { type: Object, @@ -28,11 +30,28 @@ export default { required: false, default: null, }, + viewedFiles: { + type: Object, + required: false, + default: () => ({}), + }, }, computed: { showFileRowStats() { return !this.hideFileStats && this.file.type === 'blob'; }, + fileClasses() { + if (!this.glFeatures.highlightCurrentDiffRow) { + return ''; + } + + return this.file.type === 'blob' && !this.viewedFiles[this.file.fileHash] + ? 'gl-font-weight-bold' + : ''; + }, + isActive() { + return this.currentDiffFileId === this.file.fileHash; + }, }, }; </script> @@ -41,8 +60,9 @@ export default { <file-row :file="file" v-bind="$attrs" - :class="{ 'is-active': currentDiffFileId === file.fileHash }" + :class="{ 'is-active': isActive }" class="diff-file-row" + :file-classes="fileClasses" v-on="$listeners" > <file-row-stats v-if="showFileRowStats" :file="file" class="mr-1" /> diff --git a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue index be19d8520b5..439319f487c 100644 --- a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue +++ b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue @@ -1,14 +1,13 @@ <script> -import { GlTooltipDirective } from '@gitlab/ui'; +import { GlTooltipDirective, GlIcon } from '@gitlab/ui'; import { n__ } from '~/locale'; -import Icon from '~/vue_shared/components/icon.vue'; import { 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 { components: { - Icon, + GlIcon, UserAvatarImage, }, directives: { @@ -68,7 +67,7 @@ export default { class="diff-notes-collapse js-diff-comment-avatar js-diff-comment-button" @click="$emit('toggleLineDiscussions')" > - <icon :size="12" name="collapse" /> + <gl-icon :size="12" name="collapse" /> </button> <template v-else> <user-avatar-image diff --git a/app/assets/javascripts/diffs/components/diff_stats.vue b/app/assets/javascripts/diffs/components/diff_stats.vue index 439d8097e56..05fbbd39fae 100644 --- a/app/assets/javascripts/diffs/components/diff_stats.vue +++ b/app/assets/javascripts/diffs/components/diff_stats.vue @@ -1,10 +1,10 @@ <script> import { isNumber } from 'lodash'; -import Icon from '~/vue_shared/components/icon.vue'; +import { GlIcon } from '@gitlab/ui'; import { n__ } from '~/locale'; export default { - components: { Icon }, + components: { GlIcon }, props: { addedLines: { type: Number, @@ -46,7 +46,7 @@ export default { }" > <div v-if="hasDiffFiles" class="diff-stats-group"> - <icon name="doc-code" class="diff-stats-icon text-secondary" /> + <gl-icon name="doc-code" class="diff-stats-icon text-secondary" /> <span class="text-secondary bold">{{ diffFilesCountText }} {{ filesText }}</span> </div> <div diff --git a/app/assets/javascripts/diffs/components/edit_button.vue b/app/assets/javascripts/diffs/components/edit_button.vue index 21fdb19287d..ff1af5569dc 100644 --- a/app/assets/javascripts/diffs/components/edit_button.vue +++ b/app/assets/javascripts/diffs/components/edit_button.vue @@ -1,12 +1,11 @@ <script> -import { GlTooltipDirective, GlDeprecatedButton } from '@gitlab/ui'; +import { GlTooltipDirective, GlDeprecatedButton, GlIcon } from '@gitlab/ui'; import { __ } from '~/locale'; -import Icon from '~/vue_shared/components/icon.vue'; export default { components: { GlDeprecatedButton, - Icon, + GlIcon, }, directives: { GlTooltip: GlTooltipDirective, @@ -59,7 +58,7 @@ export default { class="rounded-0 js-edit-blob" @click.native="handleEditClick" > - <icon name="pencil" /> + <gl-icon name="pencil" /> </gl-deprecated-button> </span> </template> diff --git a/app/assets/javascripts/diffs/components/image_diff_overlay.vue b/app/assets/javascripts/diffs/components/image_diff_overlay.vue index be7e6789216..3956c2fab49 100644 --- a/app/assets/javascripts/diffs/components/image_diff_overlay.vue +++ b/app/assets/javascripts/diffs/components/image_diff_overlay.vue @@ -2,12 +2,12 @@ import { mapActions, mapGetters } from 'vuex'; import { isArray } from 'lodash'; import imageDiffMixin from 'ee_else_ce/diffs/mixins/image_diff'; -import Icon from '~/vue_shared/components/icon.vue'; +import { GlIcon } from '@gitlab/ui'; export default { name: 'ImageDiffOverlay', components: { - Icon, + GlIcon, }, mixins: [imageDiffMixin], props: { @@ -112,7 +112,7 @@ export default { type="button" @click="clickedToggle(discussion)" > - <icon v-if="showCommentIcon" name="image-comment-dark" /> + <gl-icon v-if="showCommentIcon" name="image-comment-dark" /> <template v-else> {{ toggleText(discussion, index) }} </template> @@ -127,7 +127,7 @@ export default { class="btn-transparent comment-indicator" type="button" > - <icon name="image-comment-dark" /> + <gl-icon name="image-comment-dark" /> </button> </div> </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 168e8c6c14e..7fab750089e 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -1,7 +1,6 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; -import { GlTooltipDirective } from '@gitlab/ui'; -import DiffTableCell from './diff_table_cell.vue'; +import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import { MATCH_LINE_TYPE, NEW_LINE_TYPE, @@ -10,14 +9,23 @@ import { CONTEXT_LINE_CLASS_NAME, LINE_POSITION_LEFT, LINE_POSITION_RIGHT, + LINE_HOVER_CLASS_NAME, + OLD_NO_NEW_LINE_TYPE, + NEW_NO_NEW_LINE_TYPE, + EMPTY_CELL_TYPE, } from '../constants'; +import { __ } from '~/locale'; +import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; +import DiffGutterAvatars from './diff_gutter_avatars.vue'; export default { components: { - DiffTableCell, + DiffGutterAvatars, + GlIcon, }, directives: { GlTooltip: GlTooltipDirective, + SafeHtml, }, props: { fileHash: { @@ -49,6 +57,7 @@ export default { }; }, computed: { + ...mapGetters(['isLoggedIn']), ...mapGetters('diffs', ['fileLineCoverage']), ...mapState({ isHighlighted(state) { @@ -78,6 +87,70 @@ export default { coverageState() { return this.fileLineCoverage(this.filePath, this.line.new_line); }, + isMetaLine() { + const { type } = this.line; + + return ( + type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE || type === EMPTY_CELL_TYPE + ); + }, + classNameMapCell() { + const { type } = this.line; + + return [ + type, + { + hll: this.isHighlighted, + [LINE_HOVER_CLASS_NAME]: + this.isLoggedIn && this.isHover && !this.isContextLine && !this.isMetaLine, + }, + ]; + }, + addCommentTooltip() { + const brokenSymlinks = this.line.commentsDisabled; + let tooltip = __('Add a comment to this line'); + + if (brokenSymlinks) { + if (brokenSymlinks.wasSymbolic || brokenSymlinks.isSymbolic) { + tooltip = __( + 'Commenting on symbolic links that replace or are replaced by files is currently not supported.', + ); + } else if (brokenSymlinks.wasReal || brokenSymlinks.isReal) { + tooltip = __( + 'Commenting on files that replace or are replaced by symbolic links is currently not supported.', + ); + } + } + + return tooltip; + }, + shouldRenderCommentButton() { + if (this.isLoggedIn) { + const isDiffHead = parseBoolean(getParameterByName('diff_head')); + return !isDiffHead || gon.features?.mergeRefHeadComments; + } + + return false; + }, + shouldShowCommentButton() { + return this.isHover && !this.isContextLine && !this.isMetaLine && !this.hasDiscussions; + }, + hasDiscussions() { + return this.line.discussions && this.line.discussions.length > 0; + }, + lineHref() { + return `#${this.line.line_code || ''}`; + }, + lineCode() { + return ( + this.line.line_code || + (this.line.left && this.line.left.line_code) || + (this.line.right && this.line.right.line_code) + ); + }, + shouldShowAvatarsOnGutter() { + return this.hasDiscussions; + }, }, created() { this.newLineType = NEW_LINE_TYPE; @@ -89,12 +162,20 @@ export default { this.scrollToLineIfNeededInline(this.line); }, methods: { - ...mapActions('diffs', ['scrollToLineIfNeededInline']), + ...mapActions('diffs', [ + 'scrollToLineIfNeededInline', + 'showCommentForm', + 'setHighlightedRow', + 'toggleLineDiscussions', + ]), handleMouseMove(e) { // To show the comment icon on the gutter we need to know if we hover the line. // Current table structure doesn't allow us to do this with CSS in both of the diff view types this.isHover = e.type === 'mouseover'; }, + handleCommentButton() { + this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash }); + }, }, }; </script> @@ -108,25 +189,52 @@ export default { @mouseover="handleMouseMove" @mouseout="handleMouseMove" > - <diff-table-cell - :file-hash="fileHash" - :line="line" - :line-type="oldLineType" - :is-bottom="isBottom" - :is-hover="isHover" - :show-comment-button="true" - :is-highlighted="isHighlighted" - class="diff-line-num old_line" - /> - <diff-table-cell - :file-hash="fileHash" - :line="line" - :line-type="newLineType" - :is-bottom="isBottom" - :is-hover="isHover" - :is-highlighted="isHighlighted" - class="diff-line-num new_line qa-new-diff-line" - /> + <td ref="oldTd" class="diff-line-num old_line" :class="classNameMapCell"> + <span + v-if="shouldRenderCommentButton" + ref="addNoteTooltip" + v-gl-tooltip + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltip" + > + <button + v-show="shouldShowCommentButton" + ref="addDiffNoteButton" + type="button" + class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + :disabled="line.commentsDisabled" + @click="handleCommentButton" + > + <gl-icon :size="12" name="comment" /> + </button> + </span> + <a + v-if="line.old_line" + ref="lineNumberRefOld" + :data-linenumber="line.old_line" + :href="lineHref" + @click="setHighlightedRow(lineCode)" + > + </a> + <diff-gutter-avatars + v-if="shouldShowAvatarsOnGutter" + :discussions="line.discussions" + :discussions-expanded="line.discussionsExpanded" + @toggleLineDiscussions=" + toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded }) + " + /> + </td> + <td ref="newTd" class="diff-line-num new_line qa-new-diff-line" :class="classNameMapCell"> + <a + v-if="line.new_line" + ref="lineNumberRefNew" + :data-linenumber="line.new_line" + :href="lineHref" + @click="setHighlightedRow(lineCode)" + > + </a> + </td> <td v-gl-tooltip.hover :title="coverageState.text" @@ -134,6 +242,7 @@ export default { class="line-coverage" ></td> <td + v-safe-html="line.rich_text" :class="[ line.type, { @@ -141,7 +250,6 @@ export default { }, ]" class="line_content with-coverage" - v-html="line.rich_text" ></td> </tr> </template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index e82d06ee385..13805910648 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -1,5 +1,6 @@ <script> 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 inlineDiffTableRow from './inline_diff_table_row.vue'; @@ -14,7 +15,7 @@ export default { InlineDraftCommentRow, inlineDiffExpansionRow, }, - mixins: [draftCommentsMixin], + mixins: [draftCommentsMixin, glFeatureFlagsMixin()], props: { diffFile: { type: Object, diff --git a/app/assets/javascripts/diffs/components/merge_conflict_warning.vue b/app/assets/javascripts/diffs/components/merge_conflict_warning.vue new file mode 100644 index 00000000000..e47bea8e589 --- /dev/null +++ b/app/assets/javascripts/diffs/components/merge_conflict_warning.vue @@ -0,0 +1,72 @@ +<script> +import { GlButton, GlAlert } from '@gitlab/ui'; +import { CENTERED_LIMITED_CONTAINER_CLASSES } from '../constants'; + +export default { + components: { + GlAlert, + GlButton, + }, + props: { + limited: { + type: Boolean, + required: true, + }, + mergeable: { + type: Boolean, + required: true, + }, + resolutionPath: { + type: String, + required: true, + }, + }, + computed: { + containerClasses() { + return { + [CENTERED_LIMITED_CONTAINER_CLASSES]: this.limited, + }; + }, + }, +}; +</script> + +<template> + <div :class="containerClasses"> + <gl-alert + :dismissible="false" + :title="__('There are merge conflicts')" + variant="warning" + class="gl-mb-5" + > + <p class="gl-mb-2"> + {{ __('The comparison view may be inaccurate due to merge conflicts.') }} + </p> + <p class="gl-mb-0"> + {{ + __( + 'Resolve these conflicts or ask someone with write access to this repository to merge it locally.', + ) + }} + </p> + <template #actions> + <gl-button + v-if="resolutionPath" + :href="resolutionPath" + variant="info" + class="gl-mr-5 gl-alert-action" + > + {{ __('Resolve conflicts') }} + </gl-button> + <gl-button + v-if="mergeable" + class="gl-alert-action" + data-toggle="modal" + data-target="#modal_merge_info" + > + {{ __('Merge locally') }} + </gl-button> + </template> + </gl-alert> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/no_changes.vue b/app/assets/javascripts/diffs/components/no_changes.vue index 93afa978862..a640dcb0a90 100644 --- a/app/assets/javascripts/diffs/components/no_changes.vue +++ b/app/assets/javascripts/diffs/components/no_changes.vue @@ -1,12 +1,11 @@ <script> import { mapGetters } from 'vuex'; -import { escape } from 'lodash'; -import { GlButton } from '@gitlab/ui'; -import { __, sprintf } from '~/locale'; +import { GlButton, GlSprintf } from '@gitlab/ui'; export default { components: { GlButton, + GlSprintf, }, props: { changesEmptyStateIllustration: { @@ -16,20 +15,6 @@ export default { }, computed: { ...mapGetters(['getNoteableData']), - emptyStateText() { - return sprintf( - __( - 'No changes between %{ref_start}%{source_branch}%{ref_end} and %{ref_start}%{target_branch}%{ref_end}', - ), - { - ref_start: '<span class="ref-name">', - ref_end: '</span>', - source_branch: escape(this.getNoteableData.source_branch), - target_branch: escape(this.getNoteableData.target_branch), - }, - false, - ); - }, }, }; </script> @@ -41,7 +26,14 @@ export default { </div> <div class="col-12"> <div class="text-content text-center"> - <span v-html="emptyStateText"></span> + <gl-sprintf :message="__('No changes between %{sourceBranch} and %{targetBranch}')"> + <template #sourceBranch> + <span class="ref-name">{{ getNoteableData.source_branch }}</span> + </template> + <template #targetBranch> + <span class="ref-name">{{ getNoteableData.target_branch }}</span> + </template> + </gl-sprintf> <div class="text-center"> <gl-button :href="getNoteableData.new_blob_path" variant="success" category="primary">{{ __('Create commit') 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 ccb32a2a745..0bf47dc77a6 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -1,8 +1,7 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; import $ from 'jquery'; -import { GlTooltipDirective } from '@gitlab/ui'; -import DiffTableCell from './diff_table_cell.vue'; +import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import { MATCH_LINE_TYPE, NEW_LINE_TYPE, @@ -13,14 +12,20 @@ import { PARALLEL_DIFF_VIEW_TYPE, NEW_NO_NEW_LINE_TYPE, EMPTY_CELL_TYPE, + LINE_HOVER_CLASS_NAME, } from '../constants'; +import { __ } from '~/locale'; +import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; +import DiffGutterAvatars from './diff_gutter_avatars.vue'; export default { components: { - DiffTableCell, + GlIcon, + DiffGutterAvatars, }, directives: { GlTooltip: GlTooltipDirective, + SafeHtml, }, props: { fileHash: { @@ -50,10 +55,12 @@ export default { return { isLeftHover: false, isRightHover: false, + isCommentButtonRendered: false, }; }, computed: { ...mapGetters('diffs', ['fileLineCoverage']), + ...mapGetters(['isLoggedIn']), ...mapState({ isHighlighted(state) { if (this.isCommented) return true; @@ -65,12 +72,15 @@ export default { return lineCode ? lineCode === state.diffs.highlightedRow : false; }, }), - isContextLine() { + isContextLineLeft() { return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE; }, + isContextLineRight() { + return this.line.right && this.line.right.type === CONTEXT_LINE_TYPE; + }, classNameMap() { return { - [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, + [CONTEXT_LINE_CLASS_NAME]: this.isContextLineLeft, [PARALLEL_DIFF_VIEW_TYPE]: true, }; }, @@ -97,6 +107,129 @@ export default { coverageState() { return this.fileLineCoverage(this.filePath, this.line.right.new_line); }, + classNameMapCellLeft() { + const { type } = this.line.left; + + return [ + type, + { + hll: this.isHighlighted, + [LINE_HOVER_CLASS_NAME]: + this.isLoggedIn && this.isLeftHover && !this.isContextLineLeft && !this.isMetaLineLeft, + }, + ]; + }, + classNameMapCellRight() { + const { type } = this.line.right; + + return [ + type, + { + hll: this.isHighlighted, + [LINE_HOVER_CLASS_NAME]: + this.isLoggedIn && + this.isRightHover && + !this.isContextLineRight && + !this.isMetaLineRight, + }, + ]; + }, + addCommentTooltipLeft() { + const brokenSymlinks = this.line.left.commentsDisabled; + let tooltip = __('Add a comment to this line'); + + if (brokenSymlinks) { + if (brokenSymlinks.wasSymbolic || brokenSymlinks.isSymbolic) { + tooltip = __( + 'Commenting on symbolic links that replace or are replaced by files is currently not supported.', + ); + } else if (brokenSymlinks.wasReal || brokenSymlinks.isReal) { + tooltip = __( + 'Commenting on files that replace or are replaced by symbolic links is currently not supported.', + ); + } + } + + return tooltip; + }, + addCommentTooltipRight() { + const brokenSymlinks = this.line.right.commentsDisabled; + let tooltip = __('Add a comment to this line'); + + if (brokenSymlinks) { + if (brokenSymlinks.wasSymbolic || brokenSymlinks.isSymbolic) { + tooltip = __( + 'Commenting on symbolic links that replace or are replaced by files is currently not supported.', + ); + } else if (brokenSymlinks.wasReal || brokenSymlinks.isReal) { + tooltip = __( + 'Commenting on files that replace or are replaced by symbolic links is currently not supported.', + ); + } + } + + return tooltip; + }, + shouldRenderCommentButton() { + if (!this.isCommentButtonRendered) { + return false; + } + + if (this.isLoggedIn) { + const isDiffHead = parseBoolean(getParameterByName('diff_head')); + return !isDiffHead || gon.features?.mergeRefHeadComments; + } + + return false; + }, + shouldShowCommentButtonLeft() { + return ( + this.isLeftHover && + !this.isContextLineLeft && + !this.isMetaLineLeft && + !this.hasDiscussionsLeft + ); + }, + shouldShowCommentButtonRight() { + return ( + this.isRightHover && + !this.isContextLineRight && + !this.isMetaLineRight && + !this.hasDiscussionsRight + ); + }, + hasDiscussionsLeft() { + return this.line.left?.discussions?.length > 0; + }, + hasDiscussionsRight() { + return this.line.right?.discussions?.length > 0; + }, + lineHrefOld() { + return `#${this.line.left.line_code || ''}`; + }, + lineHrefNew() { + return `#${this.line.right.line_code || ''}`; + }, + lineCode() { + return ( + (this.line.left && this.line.left.line_code) || + (this.line.right && this.line.right.line_code) + ); + }, + isMetaLineLeft() { + const type = this.line.left?.type; + + return ( + type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE || type === EMPTY_CELL_TYPE + ); + }, + isMetaLineRight() { + const type = this.line.right?.type; + + return ( + type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE || type === EMPTY_CELL_TYPE + ); + }, }, created() { this.newLineType = NEW_LINE_TYPE; @@ -105,9 +238,26 @@ export default { }, mounted() { this.scrollToLineIfNeededParallel(this.line); + this.unwatchShouldShowCommentButton = this.$watch( + vm => [vm.shouldShowCommentButtonLeft, vm.shouldShowCommentButtonRight].join(), + newVal => { + if (newVal) { + this.isCommentButtonRendered = true; + this.unwatchShouldShowCommentButton(); + } + }, + ); + }, + beforeDestroy() { + this.unwatchShouldShowCommentButton(); }, methods: { - ...mapActions('diffs', ['scrollToLineIfNeededParallel']), + ...mapActions('diffs', [ + 'scrollToLineIfNeededParallel', + 'showCommentForm', + 'setHighlightedRow', + 'toggleLineDiscussions', + ]), handleMouseMove(e) { const isHover = e.type === 'mouseover'; const hoveringCell = e.target.closest('td'); @@ -133,6 +283,9 @@ export default { table.addClass(`${lineClass}-selected`); } }, + handleCommentButton(line) { + this.showCommentForm({ lineCode: line.line_code, fileHash: this.fileHash }); + }, }, }; </script> @@ -145,25 +298,53 @@ export default { @mouseout="handleMouseMove" > <template v-if="line.left && !isMatchLineLeft"> - <diff-table-cell - :file-hash="fileHash" - :line="line.left" - :line-type="oldLineType" - :is-bottom="isBottom" - :is-hover="isLeftHover" - :is-highlighted="isHighlighted" - :show-comment-button="true" - :diff-view-type="parallelDiffViewType" - line-position="left" - class="diff-line-num old_line" - /> + <td ref="oldTd" :class="classNameMapCellLeft" class="diff-line-num old_line"> + <span + v-if="shouldRenderCommentButton" + ref="addNoteTooltipLeft" + v-gl-tooltip + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltipLeft" + > + <button + v-show="shouldShowCommentButtonLeft" + ref="addDiffNoteButtonLeft" + 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" + ref="lineNumberRefOld" + :data-linenumber="line.left.old_line" + :href="lineHrefOld" + @click="setHighlightedRow(lineCode)" + > + </a> + <diff-gutter-avatars + v-if="hasDiscussionsLeft" + :discussions="line.left.discussions" + :discussions-expanded="line.left.discussionsExpanded" + @toggleLineDiscussions=" + toggleLineDiscussions({ + lineCode: line.left.line_code, + fileHash, + expanded: !line.left.discussionsExpanded, + }) + " + /> + </td> <td :class="parallelViewLeftLineType" class="line-coverage left-side"></td> <td :id="line.left.line_code" + v-safe-html="line.left.rich_text" :class="parallelViewLeftLineType" class="line_content with-coverage parallel left-side" @mousedown="handleParallelLineMouseDown" - v-html="line.left.rich_text" ></td> </template> <template v-else> @@ -172,18 +353,46 @@ export default { <td class="line_content with-coverage parallel left-side empty-cell"></td> </template> <template v-if="line.right && !isMatchLineRight"> - <diff-table-cell - :file-hash="fileHash" - :line="line.right" - :line-type="newLineType" - :is-bottom="isBottom" - :is-hover="isRightHover" - :is-highlighted="isHighlighted" - :show-comment-button="true" - :diff-view-type="parallelDiffViewType" - line-position="right" - class="diff-line-num new_line" - /> + <td ref="newTd" :class="classNameMapCellRight" class="diff-line-num new_line"> + <span + v-if="shouldRenderCommentButton" + ref="addNoteTooltipRight" + v-gl-tooltip + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltipRight" + > + <button + v-show="shouldShowCommentButtonRight" + ref="addDiffNoteButtonRight" + 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" + ref="lineNumberRefNew" + :data-linenumber="line.right.new_line" + :href="lineHrefNew" + @click="setHighlightedRow(lineCode)" + > + </a> + <diff-gutter-avatars + v-if="hasDiscussionsRight" + :discussions="line.right.discussions" + :discussions-expanded="line.right.discussionsExpanded" + @toggleLineDiscussions=" + toggleLineDiscussions({ + lineCode: line.right.line_code, + fileHash, + expanded: !line.right.discussionsExpanded, + }) + " + /> + </td> <td v-gl-tooltip.hover :title="coverageState.text" @@ -192,6 +401,7 @@ export default { ></td> <td :id="line.right.line_code" + v-safe-html="line.right.rich_text" :class="[ line.right.type, { @@ -200,7 +410,6 @@ export default { ]" class="line_content with-coverage parallel right-side" @mousedown="handleParallelLineMouseDown" - v-html="line.right.rich_text" ></td> </template> <template v-else> diff --git a/app/assets/javascripts/diffs/components/settings_dropdown.vue b/app/assets/javascripts/diffs/components/settings_dropdown.vue index c45de481a17..78647065c8e 100644 --- a/app/assets/javascripts/diffs/components/settings_dropdown.vue +++ b/app/assets/javascripts/diffs/components/settings_dropdown.vue @@ -1,17 +1,24 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; -import { GlDeprecatedButton } from '@gitlab/ui'; -import Icon from '~/vue_shared/components/icon.vue'; +import { GlButtonGroup, GlButton, GlDropdown } from '@gitlab/ui'; +import { __ } from '~/locale'; export default { components: { - GlDeprecatedButton, - Icon, + GlButtonGroup, + GlButton, + GlDropdown, }, computed: { ...mapGetters('diffs', ['isInlineView', 'isParallelView']), ...mapState('diffs', ['renderTreeList', 'showWhitespace']), }, + mounted() { + this.patchAriaLabel(); + }, + updated() { + this.patchAriaLabel(); + }, methods: { ...mapActions('diffs', [ 'setInlineDiffViewType', @@ -19,74 +26,69 @@ export default { 'setRenderTreeList', 'setShowWhitespace', ]), + patchAriaLabel() { + this.$el + .querySelector('.js-show-diff-settings') + .setAttribute('aria-label', __('Diff view settings')); + }, }, }; </script> <template> - <div class="dropdown"> - <button - type="button" - class="btn btn-default js-show-diff-settings" - data-toggle="dropdown" - data-display="static" - > - <icon name="settings" /> <icon name="chevron-down" /> - </button> - <div class="dropdown-menu dropdown-menu-right p-2 pt-3 pb-3"> - <div> - <span class="bold d-block mb-1">{{ __('File browser') }}</span> - <div class="btn-group d-flex"> - <gl-deprecated-button - :class="{ active: !renderTreeList }" - class="w-100 js-list-view" - @click="setRenderTreeList(false)" - > - {{ __('List view') }} - </gl-deprecated-button> - <gl-deprecated-button - :class="{ active: renderTreeList }" - class="w-100 js-tree-view" - @click="setRenderTreeList(true)" - > - {{ __('Tree view') }} - </gl-deprecated-button> - </div> - </div> - <div class="mt-2"> - <span class="bold d-block mb-1">{{ __('Compare changes') }}</span> - <div class="btn-group d-flex js-diff-view-buttons"> - <gl-deprecated-button - id="inline-diff-btn" - :class="{ active: isInlineView }" - class="w-100 js-inline-diff-button" - data-view-type="inline" - @click="setInlineDiffViewType" - > - {{ __('Inline') }} - </gl-deprecated-button> - <gl-deprecated-button - id="parallel-diff-btn" - :class="{ active: isParallelView }" - class="w-100 js-parallel-diff-button" - data-view-type="parallel" - @click="setParallelDiffViewType" - > - {{ __('Side-by-side') }} - </gl-deprecated-button> - </div> - </div> - <div class="mt-2"> - <label class="mb-0"> - <input - id="show-whitespace" - type="checkbox" - :checked="showWhitespace" - @change="setShowWhitespace({ showWhitespace: $event.target.checked, pushState: true })" - /> - {{ __('Show whitespace changes') }} - </label> - </div> + <gl-dropdown icon="settings" toggle-class="js-show-diff-settings" right> + <div class="gl-px-3"> + <span class="gl-font-weight-bold gl-display-block gl-mb-2">{{ __('File browser') }}</span> + <gl-button-group class="gl-display-flex"> + <gl-button + :class="{ selected: !renderTreeList }" + class="gl-w-half js-list-view" + @click="setRenderTreeList(false)" + > + {{ __('List view') }} + </gl-button> + <gl-button + :class="{ selected: renderTreeList }" + class="gl-w-half js-tree-view" + @click="setRenderTreeList(true)" + > + {{ __('Tree view') }} + </gl-button> + </gl-button-group> + </div> + <div class="gl-mt-3 gl-px-3"> + <span class="gl-font-weight-bold gl-display-block gl-mb-2">{{ __('Compare changes') }}</span> + <gl-button-group class="gl-display-flex js-diff-view-buttons"> + <gl-button + id="inline-diff-btn" + :class="{ selected: isInlineView }" + class="gl-w-half js-inline-diff-button" + data-view-type="inline" + @click="setInlineDiffViewType" + > + {{ __('Inline') }} + </gl-button> + <gl-button + id="parallel-diff-btn" + :class="{ selected: isParallelView }" + class="gl-w-half js-parallel-diff-button" + data-view-type="parallel" + @click="setParallelDiffViewType" + > + {{ __('Side-by-side') }} + </gl-button> + </gl-button-group> + </div> + <div class="gl-mt-3 gl-px-3"> + <label class="gl-mb-0"> + <input + id="show-whitespace" + type="checkbox" + :checked="showWhitespace" + @change="setShowWhitespace({ showWhitespace: $event.target.checked, pushState: true })" + /> + {{ __('Show whitespace changes') }} + </label> </div> - </div> + </gl-dropdown> </template> diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index 38fbd8e61d4..d03d450b12d 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -1,8 +1,7 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; -import { GlTooltipDirective } from '@gitlab/ui'; +import { GlTooltipDirective, GlIcon } from '@gitlab/ui'; import { s__, sprintf } from '~/locale'; -import Icon from '~/vue_shared/components/icon.vue'; import FileTree from '~/vue_shared/components/file_tree.vue'; import DiffFileRow from './diff_file_row.vue'; @@ -11,7 +10,7 @@ export default { GlTooltip: GlTooltipDirective, }, components: { - Icon, + GlIcon, FileTree, }, props: { @@ -26,7 +25,7 @@ export default { }; }, computed: { - ...mapState('diffs', ['tree', 'renderTreeList', 'currentDiffFileId']), + ...mapState('diffs', ['tree', 'renderTreeList', 'currentDiffFileId', 'viewedDiffFileIds']), ...mapGetters('diffs', ['allBlobs']), filteredTreeList() { const search = this.search.toLowerCase().trim(); @@ -66,7 +65,7 @@ export default { <div class="tree-list-holder d-flex flex-column"> <div class="gl-mb-3 position-relative tree-list-search d-flex"> <div class="flex-fill d-flex"> - <icon name="search" class="position-absolute tree-list-icon" /> + <gl-icon name="search" class="position-absolute tree-list-icon" /> <label for="diff-tree-search" class="sr-only">{{ $options.searchPlaceholder }}</label> <input id="diff-tree-search" @@ -83,7 +82,7 @@ export default { class="position-absolute bg-transparent tree-list-icon tree-list-clear-icon border-0 p-0" @click="clearSearch" > - <icon name="close" /> + <gl-icon name="close" /> </button> </div> </div> @@ -94,6 +93,7 @@ export default { :key="file.key" :file="file" :level="0" + :viewed-files="viewedDiffFileIds" :hide-file-stats="hideFileStats" :file-row-component="$options.DiffFileRow" :current-diff-file-id="currentDiffFileId" diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 447136036ee..dc97d9993da 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -34,7 +34,6 @@ export const COUNT_OF_AVATARS_IN_GUTTER = 3; export const LENGTH_OF_AVATAR_TOOLTIP = 17; export const LINES_TO_BE_RENDERED_DIRECTLY = 100; -export const MAX_LINES_TO_BE_RENDERED = 2000; export const DIFF_FILE_SYMLINK_MODE = '120000'; export const DIFF_FILE_DELETED_MODE = '0'; @@ -69,6 +68,11 @@ export const DIFFS_PER_PAGE = 20; export const DIFF_COMPARE_BASE_VERSION_INDEX = -1; export const DIFF_COMPARE_HEAD_VERSION_INDEX = -2; +// Diff View Alerts +export const ALERT_OVERFLOW_HIDDEN = 'overflow'; +export const ALERT_MERGE_CONFLICT = 'merge-conflict'; +export const ALERT_COLLAPSED_FILES = 'collapsed'; + // State machine states export const STATE_IDLING = 'idle'; export const STATE_LOADING = 'loading'; diff --git a/app/assets/javascripts/diffs/diff_file.js b/app/assets/javascripts/diffs/diff_file.js index 717c4a79ef9..610b71235d9 100644 --- a/app/assets/javascripts/diffs/diff_file.js +++ b/app/assets/javascripts/diffs/diff_file.js @@ -18,7 +18,6 @@ function fileSymlinkInformation(file, fileList) { ); } -/* eslint-disable-next-line import/prefer-default-export */ export function prepareRawDiffFile({ file, allFiles }) { Object.assign(file, { brokenSymlink: fileSymlinkInformation(file, allFiles), diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js new file mode 100644 index 00000000000..8b91543587c --- /dev/null +++ b/app/assets/javascripts/diffs/i18n.js @@ -0,0 +1,14 @@ +import { __ } from '~/locale'; + +export const GENERIC_ERROR = __('Something went wrong on our end. Please try again!'); + +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'), + collapsed: __('This file is collapsed.'), + expand: __('Expand file'), +}; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index d5581474c9b..0f275f1cb3e 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -52,7 +52,6 @@ export const setBaseConfig = ({ commit }, options) => { projectPath, dismissEndpoint, showSuggestPopover, - useSingleDiffStyle, } = options; commit(types.SET_BASE_CONFIG, { endpoint, @@ -62,61 +61,18 @@ export const setBaseConfig = ({ commit }, options) => { projectPath, dismissEndpoint, showSuggestPopover, - useSingleDiffStyle, }); }; -export const fetchDiffFiles = ({ state, commit }) => { - const worker = new TreeWorker(); - const urlParams = { - w: state.showWhitespace ? '0' : '1', - }; - let returnData; - - if (state.useSingleDiffStyle) { - urlParams.view = state.diffViewType; - } - - commit(types.SET_LOADING, true); - - worker.addEventListener('message', ({ data }) => { - commit(types.SET_TREE_DATA, data); - - worker.terminate(); - }); - - return axios - .get(mergeUrlParams(urlParams, state.endpoint)) - .then(res => { - commit(types.SET_LOADING, false); - - commit(types.SET_MERGE_REQUEST_DIFFS, res.data.merge_request_diffs || []); - commit(types.SET_DIFF_DATA, res.data); - - worker.postMessage(state.diffFiles); - - returnData = res.data; - return Vue.nextTick(); - }) - .then(() => { - handleLocationHash(); - return returnData; - }) - .catch(() => worker.terminate()); -}; - export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { const id = window?.location?.hash; const isNoteLink = id.indexOf('#note') === 0; const urlParams = { per_page: DIFFS_PER_PAGE, w: state.showWhitespace ? '0' : '1', + view: window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType, }; - if (state.useSingleDiffStyle) { - urlParams.view = state.diffViewType; - } - commit(types.SET_BATCH_LOADING, true); commit(types.SET_RETRIEVING_BATCHES, true); @@ -128,7 +84,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { commit(types.SET_BATCH_LOADING, false); if (!isNoteLink && !state.currentDiffFileId) { - commit(types.UPDATE_CURRENT_DIFF_FILE_ID, diff_files[0].file_hash); + commit(types.VIEW_DIFF_FILE, diff_files[0].file_hash); } if (isNoteLink) { @@ -144,7 +100,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { !state.diffFiles.some(f => f.file_hash === state.currentDiffFileId) && !isNoteLink ) { - commit(types.UPDATE_CURRENT_DIFF_FILE_ID, state.diffFiles[0].file_hash); + commit(types.VIEW_DIFF_FILE, state.diffFiles[0].file_hash); } if (gon.features?.codeNavigation) { @@ -175,11 +131,9 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { export const fetchDiffFilesMeta = ({ commit, state }) => { const worker = new TreeWorker(); - const urlParams = {}; - - if (state.useSingleDiffStyle) { - urlParams.view = state.diffViewType; - } + const urlParams = { + view: window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType, + }; commit(types.SET_LOADING, true); @@ -229,7 +183,7 @@ export const fetchCoverageFiles = ({ commit, state }) => { export const setHighlightedRow = ({ commit }, lineCode) => { const fileHash = lineCode.split('_')[0]; commit(types.SET_HIGHLIGHTED_ROW, lineCode); - commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash); + commit(types.VIEW_DIFF_FILE, fileHash); }; // This is adding line discussions to the actual lines in the diff tree @@ -240,10 +194,7 @@ export const assignDiscussionsToDiff = ( ) => { const id = window?.location?.hash; const isNoteLink = id.indexOf('#note') === 0; - const diffPositionByLineCode = getDiffPositionByLineCode( - state.diffFiles, - state.useSingleDiffStyle, - ); + const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles); const hash = getLocationHash(); discussions @@ -477,13 +428,17 @@ export const toggleTreeOpen = ({ commit }, path) => { commit(types.TOGGLE_FOLDER_OPEN, path); }; +export const toggleActiveFileByHash = ({ commit }, hash) => { + commit(types.VIEW_DIFF_FILE, hash); +}; + export const scrollToFile = ({ state, commit }, path) => { if (!state.treeEntries[path]) return; const { fileHash } = state.treeEntries[path]; document.location.hash = fileHash; - commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash); + commit(types.VIEW_DIFF_FILE, fileHash); }; export const toggleShowTreeList = ({ commit, state }, saving = true) => { @@ -751,7 +706,7 @@ export const setCurrentDiffFileIdFromNote = ({ commit, state, rootGetters }, not const fileHash = rootGetters.getDiscussion(note.discussion_id).diff_file?.file_hash; if (fileHash && state.diffFiles.some(f => f.file_hash === fileHash)) { - commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash); + commit(types.VIEW_DIFF_FILE, fileHash); } }; @@ -759,5 +714,5 @@ export const navigateToDiffFileIndex = ({ commit, state }, index) => { const fileHash = state.diffFiles[index].file_hash; document.location.hash = fileHash; - commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash); + commit(types.VIEW_DIFF_FILE, fileHash); }; diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index a24894b8d6b..42df5873a41 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -1,4 +1,5 @@ import { __, n__ } from '~/locale'; +import { parallelizeDiffLines } from './utils'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants'; export * from './getters_versions_dropdowns'; @@ -129,3 +130,11 @@ 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) { + return null; + } + + return parallelizeDiffLines(file.highlighted_diff_lines || []); +}; diff --git a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js index 1e8e736c028..135b1c61ef5 100644 --- a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js +++ b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js @@ -11,17 +11,26 @@ export const diffCompareDropdownTargetVersions = (state, getters) => { // startVersion only exists if the user has selected a version other // than "base" so if startVersion is null then base must be selected - const diffHead = parseBoolean(getParameterByName('diff_head')); + const defaultMergeRefForDiffs = window.gon?.features?.defaultMergeRefForDiffs || false; + const diffHeadParam = getParameterByName('diff_head'); + const diffHead = parseBoolean(diffHeadParam) || (!diffHeadParam && defaultMergeRefForDiffs); const isBaseSelected = !state.startVersion && !diffHead; const isHeadSelected = !state.startVersion && diffHead; + let baseVersion = null; - const baseVersion = { - versionName: state.targetBranchName, - version_index: DIFF_COMPARE_BASE_VERSION_INDEX, - href: state.mergeRequestDiff.base_version_path, - isBase: true, - selected: isBaseSelected, - }; + if ( + !defaultMergeRefForDiffs || + (defaultMergeRefForDiffs && !state.mergeRequestDiff.head_version_path) + ) { + baseVersion = { + versionName: state.targetBranchName, + version_index: DIFF_COMPARE_BASE_VERSION_INDEX, + href: state.mergeRequestDiff.base_version_path, + isBase: true, + selected: + isBaseSelected || (defaultMergeRefForDiffs && !state.mergeRequestDiff.head_version_path), + }; + } const headVersion = { versionName: state.targetBranchName, @@ -40,7 +49,11 @@ export const diffCompareDropdownTargetVersions = (state, getters) => { }; }; - return [...state.mergeRequestDiffs.slice(1).map(formatVersion), baseVersion, headVersion]; + return [ + ...state.mergeRequestDiffs.slice(1).map(formatVersion), + baseVersion, + state.mergeRequestDiff.head_version_path && headVersion, + ].filter(a => a); }; export const diffCompareDropdownSourceVersions = (state, getters) => { diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index d31a600e354..001d9d9f594 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -34,6 +34,7 @@ export default () => ({ showTreeList: true, currentDiffFileId: '', projectPath: '', + viewedDiffFileIds: {}, commentForms: [], highlightedRow: null, renderTreeList: true, @@ -41,5 +42,4 @@ export default () => ({ fileFinderVisible: false, dismissEndpoint: '', showSuggestPopover: true, - useSingleDiffStyle: false, }); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 4b1dbc34902..5dba2e9d10d 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -19,7 +19,7 @@ export const SET_LINE_DISCUSSIONS_FOR_FILE = 'SET_LINE_DISCUSSIONS_FOR_FILE'; export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FILE'; export const TOGGLE_FOLDER_OPEN = 'TOGGLE_FOLDER_OPEN'; export const TOGGLE_SHOW_TREE_LIST = 'TOGGLE_SHOW_TREE_LIST'; -export const UPDATE_CURRENT_DIFF_FILE_ID = 'UPDATE_CURRENT_DIFF_FILE_ID'; +export const VIEW_DIFF_FILE = 'VIEW_DIFF_FILE'; export const OPEN_DIFF_FILE_COMMENT_FORM = 'OPEN_DIFF_FILE_COMMENT_FORM'; export const UPDATE_DIFF_FILE_COMMENT_FORM = 'UPDATE_DIFF_FILE_COMMENT_FORM'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 0d41f1c2178..7925c620c4e 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -1,4 +1,6 @@ +import Vue from 'vue'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; +import { INLINE_DIFF_VIEW_TYPE } from '../constants'; import { findDiffFile, addLineReferences, @@ -24,7 +26,6 @@ export default { projectPath, dismissEndpoint, showSuggestPopover, - useSingleDiffStyle, } = options; Object.assign(state, { endpoint, @@ -34,7 +35,6 @@ export default { projectPath, dismissEndpoint, showSuggestPopover, - useSingleDiffStyle, }); }, @@ -57,10 +57,7 @@ export default { [types.SET_DIFF_DATA](state, data) { let files = state.diffFiles; - if ( - !(gon?.features?.diffsBatchLoad && window.location.search.indexOf('diff_id') === -1) && - data.diff_files - ) { + if (window.location.search.indexOf('diff_id') !== -1 && data.diff_files) { files = prepareDiffData(data, files); } @@ -154,7 +151,9 @@ export default { addContextLines({ inlineLines: diffFile.highlighted_diff_lines, parallelLines: diffFile.parallel_diff_lines, - diffViewType: state.diffViewType, + diffViewType: window.gon?.features?.unifiedDiffLines + ? INLINE_DIFF_VIEW_TYPE + : state.diffViewType, contextLines: lines, bottom, lineNumbers, @@ -249,7 +248,7 @@ export default { }); } - if (!file.parallel_diff_lines || !file.highlighted_diff_lines) { + if (!file.parallel_diff_lines.length || !file.highlighted_diff_lines.length) { const newDiscussions = (file.discussions || []) .filter(d => d.id !== discussion.id) .concat(discussion); @@ -293,8 +292,9 @@ export default { [types.TOGGLE_SHOW_TREE_LIST](state) { state.showTreeList = !state.showTreeList; }, - [types.UPDATE_CURRENT_DIFF_FILE_ID](state, fileId) { + [types.VIEW_DIFF_FILE](state, fileId) { state.currentDiffFileId = fileId; + Vue.set(state.viewedDiffFileIds, fileId, true); }, [types.OPEN_DIFF_FILE_COMMENT_FORM](state, formData) { state.commentForms.push({ diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index f014cddda32..69330ffae2f 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -11,7 +11,6 @@ import { OLD_LINE_TYPE, MATCH_LINE_TYPE, LINES_TO_BE_RENDERED_DIRECTLY, - MAX_LINES_TO_BE_RENDERED, TREE_TYPE, INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE, @@ -20,6 +19,77 @@ import { } from '../constants'; import { prepareRawDiffFile } from '../diff_file'; +export const isAdded = line => ['new', 'new-nonewline'].includes(line.type); +export const isRemoved = line => ['old', 'old-nonewline'].includes(line.type); +export const isUnchanged = line => !line.type; +export const isMeta = line => ['match', 'new-nonewline', 'old-nonewline'].includes(line.type); + +/** + * Pass in the inline diff lines array which gets converted + * to the parallel diff lines. + * This allows for us to convert inline diff lines to parallel + * on the frontend without needing to send any requests + * to the API. + * + * This method has been taken from the already existing backend + * implementation at lib/gitlab/diff/parallel_diff.rb + * + * @param {Object[]} diffLines - inline diff lines + * + * @returns {Object[]} parallel lines + */ +export const parallelizeDiffLines = (diffLines = []) => { + let freeRightIndex = null; + const lines = []; + + for (let i = 0, diffLinesLength = diffLines.length, index = 0; i < diffLinesLength; i += 1) { + const line = diffLines[i]; + + if (isRemoved(line)) { + lines.push({ + [LINE_POSITION_LEFT]: line, + [LINE_POSITION_RIGHT]: null, + }); + + if (freeRightIndex === null) { + // Once we come upon a new line it can be put on the right of this old line + freeRightIndex = index; + } + index += 1; + } else if (isAdded(line)) { + if (freeRightIndex !== null) { + // 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; + + // If there are any other old lines on the left that don't yet have + // a new counterpart on the right, update the free_right_index + const nextFreeRightIndex = freeRightIndex + 1; + freeRightIndex = nextFreeRightIndex < index ? nextFreeRightIndex : null; + } else { + lines.push({ + [LINE_POSITION_LEFT]: null, + [LINE_POSITION_RIGHT]: line, + }); + + freeRightIndex = null; + index += 1; + } + } else if (isMeta(line) || isUnchanged(line)) { + // line in the right panel is the same as in the left one + lines.push({ + [LINE_POSITION_LEFT]: line, + [LINE_POSITION_RIGHT]: line, + }); + + freeRightIndex = null; + index += 1; + } + } + + return lines; +}; + export function findDiffFile(files, match, matchKey = 'file_hash') { return files.find(file => file[matchKey] === match); } @@ -281,7 +351,7 @@ function mergeTwoFiles(target, source) { function ensureBasicDiffFileLines(file) { const missingInline = !file.highlighted_diff_lines; - const missingParallel = !file.parallel_diff_lines; + const missingParallel = !file.parallel_diff_lines || window.gon?.features?.unifiedDiffLines; Object.assign(file, { highlighted_diff_lines: missingInline ? [] : file.highlighted_diff_lines, @@ -379,12 +449,10 @@ function getVisibleDiffLines(file) { } function finalizeDiffFile(file) { - const name = (file.viewer && file.viewer.name) || diffViewerModes.text; const lines = getVisibleDiffLines(file); Object.assign(file, { renderIt: lines < LINES_TO_BE_RENDERED_DIRECTLY, - collapsed: name === diffViewerModes.text && lines > MAX_LINES_TO_BE_RENDERED, isShowingFullFile: false, isLoadingFullFile: false, discussions: [], @@ -417,11 +485,11 @@ export function prepareDiffData(diff, priorFiles = []) { return deduplicateFilesList([...priorFiles, ...cleanedFiles]); } -export function getDiffPositionByLineCode(diffFiles, useSingleDiffStyle) { +export function getDiffPositionByLineCode(diffFiles) { let lines = []; const hasInlineDiffs = diffFiles.some(file => file.highlighted_diff_lines.length > 0); - if (!useSingleDiffStyle || hasInlineDiffs) { + if (hasInlineDiffs) { // In either of these cases, we can use `highlighted_diff_lines` because // that will include all of the parallel diff lines, too diff --git a/app/assets/javascripts/diffs/utils/uuids.js b/app/assets/javascripts/diffs/utils/uuids.js index 1a529c07ccc..12448350e62 100644 --- a/app/assets/javascripts/diffs/utils/uuids.js +++ b/app/assets/javascripts/diffs/utils/uuids.js @@ -11,9 +11,6 @@ * @typedef {String} UUIDv4 */ -// https://gitlab.com/gitlab-org/frontend/rfcs/-/issues/20 -/* eslint-disable import/prefer-default-export */ - import MersenneTwister from 'mersenne-twister'; import stringHash from 'string-hash'; import { isString } from 'lodash'; |