diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-05-19 10:33:21 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-05-19 10:33:21 +0300 |
commit | 36a59d088eca61b834191dacea009677a96c052f (patch) | |
tree | e4f33972dab5d8ef79e3944a9f403035fceea43f /app/assets/javascripts/diffs | |
parent | a1761f15ec2cae7c7f7bbda39a75494add0dfd6f (diff) |
Add latest changes from gitlab-org/gitlab@15-0-stable-eev15.0.0-rc42
Diffstat (limited to 'app/assets/javascripts/diffs')
18 files changed, 304 insertions, 169 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index c86f2c8451c..c3436159cea 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -17,7 +17,6 @@ import { helpPagePath } from '~/helpers/help_page_helper'; import { parseBoolean } from '~/lib/utils/common_utils'; import { updateHistory } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; -import MrWidgetHowToMergeModal from '~/vue_merge_request_widget/components/mr_widget_how_to_merge_modal.vue'; import PanelResizer from '~/vue_shared/components/panel_resizer.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; @@ -75,7 +74,6 @@ export default { PanelResizer, GlPagination, GlSprintf, - MrWidgetHowToMergeModal, GlAlert, }, mixins: [glFeatureFlagsMixin()], @@ -373,7 +371,7 @@ export default { events.push(TRACKING_MULTIPLE_FILES_MODE); } - queueRedisHllEvents(events); + queueRedisHllEvents(events, { verifyCap: true }); this.subscribeToVirtualScrollingEvents(); }, @@ -738,15 +736,6 @@ export default { /> </div> </div> - <mr-widget-how-to-merge-modal - :is-fork="isForked" - :can-merge="canMerge" - :source-branch="branchName" - :source-project-path="sourceProjectFullPath" - :target-branch="targetBranchName" - :source-project-default-url="sourceProjectDefaultUrl" - :reviewing-docs-path="$options.howToMergeDocsPath" - /> </div> </div> </template> diff --git a/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue b/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue index 6c5973b7c28..fd219a7d00f 100644 --- a/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue +++ b/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue @@ -24,7 +24,12 @@ export default { </script> <template> - <gl-dropdown :text="selectedVersionName" data-qa-selector="dropdown_content"> + <gl-dropdown + :text="selectedVersionName" + data-qa-selector="dropdown_content" + size="small" + category="tertiary" + > <template v-for="version in versions"> <gl-dropdown-divider v-if="version.addDivider" :key="version.id" /> <gl-dropdown-item diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index 4dfd672f99b..8a5325cf218 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -79,7 +79,7 @@ export default { </script> <template> - <div class="mr-version-controls border-top"> + <div class="mr-version-controls"> <div class="mr-version-menus-container content-block"> <gl-button v-if="hasChanges" diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index b4bffdcb07f..1eba12a3ae9 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -156,7 +156,7 @@ export default { :link-href="author.path" :img-src="author.avatar_url" :img-alt="author.name" - :img-size="40" + :img-size="48" class="d-none d-sm-block new-comment" /> <diff-discussions diff --git a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue index 4c7b8e8f667..4e7dc578193 100644 --- a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue @@ -1,42 +1,33 @@ <script> -import { GlIcon } from '@gitlab/ui'; -import { mapState, mapActions } from 'vuex'; +import { GlTooltipDirective, GlSafeHtmlDirective, GlIcon, GlLoadingIcon } from '@gitlab/ui'; +import { mapActions } from 'vuex'; import createFlash from '~/flash'; import { s__, sprintf } from '~/locale'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import { UNFOLD_COUNT, INLINE_DIFF_VIEW_TYPE, INLINE_DIFF_LINES_KEY } from '../constants'; +import { UNFOLD_COUNT, INLINE_DIFF_LINES_KEY } from '../constants'; import * as utils from '../store/utils'; const EXPAND_ALL = 0; const EXPAND_UP = 1; const EXPAND_DOWN = 2; -const lineNumberByViewType = (viewType, diffLine) => { - const numberGetters = { - [INLINE_DIFF_VIEW_TYPE]: (line) => line?.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, + i18n: { + showMore: sprintf(s__('Diffs|Show %{unfoldCount} lines'), { unfoldCount: UNFOLD_COUNT }), + showAll: s__('Diffs|Show all unchanged lines'), + }, components: { GlIcon, + GlLoadingIcon, + }, + directives: { + GlTooltip: GlTooltipDirective, + SafeHtml: GlSafeHtmlDirective, }, mixins: [glFeatureFlagsMixin()], props: { - fileHash: { - type: String, - required: true, - }, - contextLinesPath: { - type: String, + file: { + type: Object, required: true, }, line: { @@ -53,34 +44,45 @@ export default { required: false, default: false, }, + inline: { + type: Boolean, + required: true, + }, + lineCountBetween: { + type: Number, + required: false, + default: -1, + }, + }, + data() { + return { loading: { up: false, down: false, all: false } }; }, computed: { - ...mapState({ - diffFiles: (state) => state.diffs.diffFiles, - }), canExpandUp() { return !this.isBottom; }, canExpandDown() { return this.isBottom || !this.isTop; }, - }, - created() { - this.EXPAND_DOWN = EXPAND_DOWN; - this.EXPAND_UP = EXPAND_UP; + isLineCountSmall() { + return this.lineCountBetween >= 20 || this.lineCountBetween === -1; + }, + showExpandDown() { + return this.canExpandDown && this.isLineCountSmall; + }, + showExpandUp() { + return this.canExpandUp && this.isLineCountSmall; + }, }, methods: { ...mapActions('diffs', ['loadMoreLines']), getPrevLineNumber(oldLineNumber, newLineNumber) { - const diffFile = utils.findDiffFile(this.diffFiles, this.fileHash); - const index = utils.getPreviousLineIndex(INLINE_DIFF_VIEW_TYPE, diffFile, { + const index = utils.getPreviousLineIndex(this.file, { oldLineNumber, newLineNumber, }); - return ( - lineNumberByViewType(INLINE_DIFF_VIEW_TYPE, diffFile[INLINE_DIFF_LINES_KEY][index - 2]) || 0 - ); + return this.file[INLINE_DIFF_LINES_KEY][index - 2]?.new_line || 0; }, callLoadMoreLines( endpoint, @@ -99,6 +101,9 @@ export default { message: s__('Diffs|Something went wrong while fetching diff lines.'), }); this.isRequesting = false; + }) + .finally(() => { + this.loading = { up: false, down: false, all: false }; }); }, handleExpandLines(type = EXPAND_ALL) { @@ -107,25 +112,26 @@ export default { } this.isRequesting = true; - const endpoint = this.contextLinesPath; - const { fileHash } = this; - const view = INLINE_DIFF_VIEW_TYPE; + const endpoint = this.file.context_lines_path; const oldLineNumber = this.line.meta_data.old_pos || 0; const newLineNumber = this.line.meta_data.new_pos || 0; const offset = newLineNumber - oldLineNumber; - const expandOptions = { endpoint, fileHash, view, oldLineNumber, newLineNumber, offset }; + const expandOptions = { endpoint, oldLineNumber, newLineNumber, offset }; if (type === EXPAND_UP) { + this.loading.up = true; this.handleExpandUpLines(expandOptions); } else if (type === EXPAND_DOWN) { + this.loading.down = true; this.handleExpandDownLines(expandOptions); } else { + this.loading.all = true; this.handleExpandAllLines(expandOptions); } }, handleExpandUpLines(expandOptions) { - const { endpoint, fileHash, view, oldLineNumber, newLineNumber, offset } = expandOptions; + const { endpoint, oldLineNumber, newLineNumber, offset } = expandOptions; const bottom = this.isBottom; const lineNumber = newLineNumber - 1; @@ -139,15 +145,13 @@ export default { unfold = false; } - const params = { since, to, bottom, offset, unfold, view }; + const params = { since, to, bottom, offset, unfold }; const lineNumbers = { oldLineNumber, newLineNumber }; - this.callLoadMoreLines(endpoint, params, lineNumbers, fileHash); + this.callLoadMoreLines(endpoint, params, lineNumbers, this.file.file_hash); }, handleExpandDownLines(expandOptions) { const { endpoint, - fileHash, - view, oldLineNumber: metaOldPos, newLineNumber: metaNewPos, offset, @@ -183,19 +187,19 @@ export default { } } - const params = { since, to, bottom, offset, unfold, view }; + const params = { since, to, bottom, offset, unfold }; const lineNumbers = { oldLineNumber, newLineNumber }; this.callLoadMoreLines( endpoint, params, lineNumbers, - fileHash, + this.file.file_hash, isExpandDown, nextLineNumbers, ); }, handleExpandAllLines(expandOptions) { - const { endpoint, fileHash, view, oldLineNumber, newLineNumber, offset } = expandOptions; + const { endpoint, oldLineNumber, newLineNumber, offset } = expandOptions; const bottom = this.isBottom; const unfold = false; let since; @@ -213,21 +217,71 @@ export default { to = newLineNumber - 1; } - const params = { since, to, bottom, offset, unfold, view }; + const params = { since, to, bottom, offset, unfold }; const lineNumbers = { oldLineNumber, newLineNumber }; - this.callLoadMoreLines(endpoint, params, lineNumbers, fileHash); + this.callLoadMoreLines(endpoint, params, lineNumbers, this.file.file_hash); }, }, + EXPAND_DOWN, + EXPAND_UP, }; </script> <template> - <div class="content js-line-expansion-content"> + <div + v-if="glFeatures.updatedDiffExpansionButtons" + class="diff-grid-row diff-grid-row-full diff-tr line_holder match expansion" + > + <div :class="{ parallel: !inline }" class="diff-grid-left diff-grid-2-col left-side"> + <div + class="diff-td diff-line-num gl-text-center! gl-p-0! gl-w-full! gl-display-flex gl-flex-direction-column" + > + <button + v-if="showExpandDown" + v-gl-tooltip.left + :title="s__('Diffs|Next 20 lines')" + type="button" + class="js-unfold-down gl-rounded-0 gl-border-0 diff-line-expand-button" + @click="handleExpandLines($options.EXPAND_DOWN)" + > + <gl-loading-icon v-if="loading.down" size="sm" color="dark" inline /> + <gl-icon v-else name="expand-down" /> + </button> + <button + v-if="lineCountBetween !== -1 && lineCountBetween < 20" + v-gl-tooltip.left + :title="s__('Diffs|Expand all lines')" + type="button" + class="js-unfold-all gl-rounded-0 gl-border-0 diff-line-expand-button" + @click="handleExpandLines()" + > + <gl-loading-icon v-if="loading.all" size="sm" color="dark" inline /> + <gl-icon v-else name="expand" /> + </button> + <button + v-if="showExpandUp" + v-gl-tooltip.left + :title="s__('Diffs|Previous 20 lines')" + type="button" + class="js-unfold gl-rounded-0 gl-border-0 diff-line-expand-button" + @click="handleExpandLines($options.EXPAND_UP)" + > + <gl-loading-icon v-if="loading.up" size="sm" color="dark" inline /> + <gl-icon v-else name="expand-up" /> + </button> + </div> + <div + v-safe-html="line.rich_text" + class="gl-display-flex! gl-flex-direction-column gl-justify-content-center diff-td line_content left-side gl-white-space-normal!" + ></div> + </div> + </div> + <div v-else class="content js-line-expansion-content"> <button type="button" :disabled="!canExpandDown" class="js-unfold-down gl-mx-2 gl-py-4 gl-cursor-pointer" - @click="handleExpandLines(EXPAND_DOWN)" + @click="handleExpandLines($options.EXPAND_DOWN)" > <gl-icon :size="12" name="expand-down" /> <span>{{ $options.i18n.showMore }}</span> @@ -244,7 +298,7 @@ export default { type="button" :disabled="!canExpandUp" class="js-unfold gl-mx-2 gl-py-4 gl-cursor-pointer" - @click="handleExpandLines(EXPAND_UP)" + @click="handleExpandLines($options.EXPAND_UP)" > <gl-icon :size="12" name="expand-up" /> <span>{{ $options.i18n.showMore }}</span> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index d8f27a967df..0b82be7140c 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -5,7 +5,6 @@ import { GlSafeHtmlDirective as SafeHtml, GlSprintf, GlAlert, - GlModalDirective, } from '@gitlab/ui'; import { escape } from 'lodash'; import { mapActions, mapGetters, mapState } from 'vuex'; @@ -42,7 +41,6 @@ export default { }, directives: { SafeHtml, - GlModalDirective, }, mixins: [glFeatureFlagsMixin(), IdState({ idProp: (vm) => vm.file.file_hash })], props: { @@ -360,10 +358,12 @@ export default { class="js-file-fork-suggestion-section file-fork-suggestion" > <span v-safe-html="forkMessage" class="file-fork-suggestion-note"></span> - <a + <gl-button :href="file.fork_path" - class="js-fork-suggestion-button btn btn-grouped btn-inverted btn-success" - >{{ $options.i18n.fork }}</a + class="js-fork-suggestion-button" + category="secondary" + variant="confirm" + >{{ $options.i18n.fork }}</gl-button > <button class="js-cancel-fork-suggestion-button btn btn-grouped" @@ -379,6 +379,53 @@ export default { :class="hasBodyClasses.contentByHash" data-testid="content-area" > + <gl-alert + v-if="!showLoadingIcon && file.conflict_type" + variant="danger" + :dismissible="false" + data-testid="conflictsAlert" + > + {{ $options.CONFLICT_TEXT[file.conflict_type] }} + <template v-if="!canMerge"> + {{ __('Ask someone with write access to resolve it.') }} + </template> + <gl-sprintf + v-else-if="conflictResolutionPath" + :message=" + __( + 'You can %{gitlabLinkStart}resolve conflicts on GitLab%{gitlabLinkEnd} or %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}.', + ) + " + > + <template #gitlabLink="{ content }"> + <gl-button + :href="conflictResolutionPath" + variant="link" + class="gl-vertical-align-text-bottom" + >{{ content }}</gl-button + > + </template> + <template #resolveLocally="{ content }"> + <gl-button + variant="link" + class="gl-vertical-align-text-bottom js-check-out-modal-trigger" + >{{ content }}</gl-button + > + </template> + </gl-sprintf> + <gl-sprintf + v-else + :message="__('You can %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}.')" + > + <template #resolveLocally="{ content }"> + <gl-button + variant="link" + class="gl-vertical-align-text-bottom js-check-out-modal-trigger" + >{{ content }}</gl-button + > + </template> + </gl-sprintf> + </gl-alert> <gl-loading-icon v-if="showLoadingIcon" size="sm" @@ -402,55 +449,6 @@ export default { <div v-else v-safe-html="errorMessage" class="nothing-here-block"></div> </div> <template v-else> - <gl-alert - v-if="file.conflict_type" - variant="danger" - :dismissible="false" - data-testid="conflictsAlert" - > - {{ $options.CONFLICT_TEXT[file.conflict_type] }} - <template v-if="!canMerge"> - {{ __('Ask someone with write access to resolve it.') }} - </template> - <gl-sprintf - v-else-if="conflictResolutionPath" - :message=" - __( - 'You can %{gitlabLinkStart}resolve conflicts on GitLab%{gitlabLinkEnd} or %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}.', - ) - " - > - <template #gitlabLink="{ content }"> - <gl-button - :href="conflictResolutionPath" - variant="link" - class="gl-vertical-align-text-bottom" - >{{ content }}</gl-button - > - </template> - <template #resolveLocally="{ content }"> - <gl-button - v-gl-modal-directive="'modal-merge-info'" - variant="link" - class="gl-vertical-align-text-bottom" - >{{ content }}</gl-button - > - </template> - </gl-sprintf> - <gl-sprintf - v-else - :message="__('You can %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}.')" - > - <template #resolveLocally="{ content }"> - <gl-button - v-gl-modal-directive="'modal-merge-info'" - variant="link" - class="gl-vertical-align-text-bottom" - >{{ content }}</gl-button - > - </template> - </gl-sprintf> - </gl-alert> <div v-if="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" diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 8cdbd2b7dbc..a75262ee303 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -302,6 +302,7 @@ export default { @click="handleFileNameClick" > <file-icon + v-if="!glFeatures.removeDiffHeaderIcons" :file-name="filePath" :size="16" aria-hidden="true" @@ -394,6 +395,7 @@ export default { <gl-dropdown v-gl-tooltip.hover.focus="$options.i18n.optionsDropdownTitle" size="small" + category="tertiary" right toggle-class="btn-icon js-diff-more-actions" class="gl-pt-0!" @@ -402,7 +404,7 @@ export default { @hidden="setMoreActionsShown(false)" > <template #button-content> - <gl-icon name="ellipsis_v" class="mr-0" :size="12" /> + <gl-icon name="ellipsis_v" class="mr-0" /> <span class="sr-only">{{ $options.i18n.optionsDropdownTitle }}</span> </template> <gl-dropdown-item 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 7a30740e31b..a2f0e2c2653 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -1,6 +1,6 @@ <script> import { mapState, mapGetters, mapActions } from 'vuex'; -import { s__ } from '~/locale'; +import { s__, __ } from '~/locale'; import diffLineNoteFormMixin from '~/notes/mixins/diff_line_note_form'; import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; @@ -179,7 +179,10 @@ export default { if (shouldConfirm && isDirty) { const msg = s__('Notes|Are you sure you want to cancel creating this comment?'); - const confirmed = await confirmAction(msg); + const confirmed = await confirmAction(msg, { + primaryBtnText: __('Discard changes'), + cancelBtnText: __('Continue editing'), + }); if (!confirmed) { return; diff --git a/app/assets/javascripts/diffs/components/diff_row.vue b/app/assets/javascripts/diffs/components/diff_row.vue index 4893803a3b6..1b07b00d725 100644 --- a/app/assets/javascripts/diffs/components/diff_row.vue +++ b/app/assets/javascripts/diffs/components/diff_row.vue @@ -160,7 +160,13 @@ export default { <!-- eslint-disable-next-line vue/no-deprecated-functional-template --> <template functional> - <div :class="$options.classNameMap(props)" class="diff-grid-row diff-tr line_holder"> + <div + :class="[ + $options.classNameMap(props), + { expansion: props.line.left && props.line.left.type === 'expanded' }, + ]" + class="diff-grid-row diff-tr line_holder" + > <div :id="props.line.left && props.line.left.line_code" data-testid="left-side" diff --git a/app/assets/javascripts/diffs/components/diff_row_utils.js b/app/assets/javascripts/diffs/components/diff_row_utils.js index 99999445c43..f610ac979ca 100644 --- a/app/assets/javascripts/diffs/components/diff_row_utils.js +++ b/app/assets/javascripts/diffs/components/diff_row_utils.js @@ -10,6 +10,7 @@ import { CONFLICT_MARKER_THEIR, CONFLICT_THEIR, CONFLICT_OUR, + EXPANDED_LINE_TYPE, } from '../constants'; export const isHighlighted = (highlightedRow, line, isCommented) => { @@ -118,10 +119,12 @@ export const mapParallel = (content) => (line) => { if (right) { right = { ...right, - renderDiscussion: Boolean(hasExpandedDiscussionOnRight && right.type), + renderDiscussion: Boolean( + hasExpandedDiscussionOnRight && right.type && right.type !== EXPANDED_LINE_TYPE, + ), hasDraft: content.hasParallelDraftRight(content.diffFile.file_hash, line), lineDraft: content.draftForLine(content.diffFile.file_hash, line, 'right'), - hasCommentForm: Boolean(right.hasForm && right.type), + hasCommentForm: Boolean(right.hasForm && right.type && right.type !== EXPANDED_LINE_TYPE), emptyCellClassMap: { conflict_their: line.left?.type === CONFLICT_OUR }, addCommentTooltip: addCommentTooltip(line.right), }; diff --git a/app/assets/javascripts/diffs/components/diff_view.vue b/app/assets/javascripts/diffs/components/diff_view.vue index f46b0a538f1..529f8e0a2f9 100644 --- a/app/assets/javascripts/diffs/components/diff_view.vue +++ b/app/assets/javascripts/diffs/components/diff_view.vue @@ -141,6 +141,18 @@ export default { table.classList.add(`${lineClass}-selected`); } }, + getCountBetweenIndex(index) { + if (index === 0) { + return -1; + } else if (!this.diffLines[index + 1]) { + return -1; + } + + return ( + Number(this.diffLines[index + 1].left.new_line) - + Number(this.diffLines[index - 1].left.new_line) + ); + }, }, userColorScheme: window.gon.user_color_scheme, }; @@ -158,38 +170,51 @@ export default { > <template v-for="(line, index) in diffLines"> <template v-if="line.isMatchLineLeft || line.isMatchLineRight"> - <div :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" - /> + <diff-expansion-cell + v-if="glFeatures.updatedDiffExpansionButtons" + :key="`expand-${index}`" + :file="diffFile" + :line="line.left" + :is-top="index === 0" + :is-bottom="index + 1 === diffLinesLength" + :inline="inline" + :line-count-between="getCountBetweenIndex(index)" + /> + <template v-else> + <div :key="`expand-${index}`" class="diff-tr line_expansion old-line_expansion match"> + <div class="diff-td text-center gl-font-regular"> + <diff-expansion-cell + :file="diffFile" + :context-lines-path="diffFile.context_lines_path" + :line="line.left" + :is-top="index === 0" + :is-bottom="index + 1 === diffLinesLength" + :inline="inline" + /> + </div> </div> - </div> - <div - v-if="line.left.rich_text" - :key="`expand-definition-${index}`" - class="diff-grid-row diff-tr line_holder match" - > - <div class="diff-grid-left diff-grid-3-col left-side"> - <div class="diff-td diff-line-num"></div> - <div v-if="inline" class="diff-td diff-line-num"></div> - <div - v-safe-html="line.left.rich_text" - class="diff-td line_content left-side gl-white-space-normal!" - ></div> + <div + v-if="line.left.rich_text" + :key="`expand-definition-${index}`" + class="diff-grid-row diff-tr line_holder match" + > + <div class="diff-grid-left diff-grid-3-col left-side"> + <div class="diff-td diff-line-num"></div> + <div v-if="inline" class="diff-td diff-line-num"></div> + <div + v-safe-html="line.left.rich_text" + class="diff-td line_content left-side gl-white-space-normal!" + ></div> + </div> + <div v-if="!inline" class="diff-grid-right diff-grid-3-col right-side"> + <div class="diff-td diff-line-num"></div> + <div + v-safe-html="line.left.rich_text" + class="diff-td line_content right-side gl-white-space-normal!" + ></div> + </div> </div> - <div v-if="!inline" class="diff-grid-right diff-grid-3-col right-side"> - <div class="diff-td diff-line-num"></div> - <div - v-safe-html="line.left.rich_text" - class="diff-td line_content right-side gl-white-space-normal!" - ></div> - </div> - </div> + </template> </template> <diff-row v-if="!line.isMatchLineLeft && !line.isMatchLineRight" diff --git a/app/assets/javascripts/diffs/components/hidden_files_warning.vue b/app/assets/javascripts/diffs/components/hidden_files_warning.vue index b9962682848..f6a8c679f3b 100644 --- a/app/assets/javascripts/diffs/components/hidden_files_warning.vue +++ b/app/assets/javascripts/diffs/components/hidden_files_warning.vue @@ -51,7 +51,7 @@ export default { __( 'To preserve performance only %{strongStart}%{visible} of %{total}%{strongEnd} files are displayed.', ), - { visible, total }, + { visible, total } /* eslint-disable-line @gitlab/vue-no-new-non-primitive-in-template */, ) " > diff --git a/app/assets/javascripts/diffs/components/image_diff_overlay.vue b/app/assets/javascripts/diffs/components/image_diff_overlay.vue index 8871be1f9af..bd040cd1ba1 100644 --- a/app/assets/javascripts/diffs/components/image_diff_overlay.vue +++ b/app/assets/javascripts/diffs/components/image_diff_overlay.vue @@ -132,10 +132,10 @@ export default { <design-note-pin v-if="canComment && currentCommentForm" - :position="{ + :position="/* eslint-disable @gitlab/vue-no-new-non-primitive-in-template */ { left: `${currentCommentForm.xPercent}%`, top: `${currentCommentForm.yPercent}%`, - }" + } /* eslint-enable @gitlab/vue-no-new-non-primitive-in-template */" /> </div> </template> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index bbe27c0dbd6..6c0c9c4e1d0 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -1,6 +1,7 @@ export const INLINE_DIFF_VIEW_TYPE = 'inline'; export const PARALLEL_DIFF_VIEW_TYPE = 'parallel'; export const MATCH_LINE_TYPE = 'match'; +export const EXPANDED_LINE_TYPE = 'expanded'; export const OLD_NO_NEW_LINE_TYPE = 'old-nonewline'; export const NEW_NO_NEW_LINE_TYPE = 'new-nonewline'; export const CONTEXT_LINE_TYPE = 'context'; @@ -101,6 +102,8 @@ export const CONFLICT_MARKER_THEIR = 'conflict_marker_their'; // Tracking events export const DEFER_DURATION = 750; +export const TRACKING_CAP_KEY = 'code_review_events_dispatched'; +export const TRACKING_CAP_LENGTH = 86400000; // 24 hours export const TRACKING_CLICK_DIFF_VIEW_SETTING = 'i_code_review_click_diff_view_setting'; export const TRACKING_DIFF_VIEW_INLINE = 'i_code_review_diff_view_inline'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index fb35114c0a9..d2b798245fc 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -4,6 +4,7 @@ import { DIFF_FILE_MANUAL_COLLAPSE, DIFF_FILE_AUTOMATIC_COLLAPSE, INLINE_DIFF_LINES_KEY, + EXPANDED_LINE_TYPE, } from '../constants'; import * as types from './mutation_types'; import { @@ -131,6 +132,7 @@ export default { : line.line_code || `${fileHash}_${line.old_line}_${line.new_line}`; return { ...line, + type: line.type || EXPANDED_LINE_TYPE, line_code: lineCode, discussions: line.discussions || [], hasForm: false, diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index f2028892a5f..92f3cf83740 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -15,13 +15,15 @@ import { CONFLICT_MARKER, CONFLICT_MARKER_OUR, CONFLICT_MARKER_THEIR, + EXPANDED_LINE_TYPE, } from '../constants'; import { prepareRawDiffFile } from '../utils/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); +export const isMeta = (line) => + ['match', EXPANDED_LINE_TYPE, 'new-nonewline', 'old-nonewline'].includes(line.type); export const isConflictMarker = (line) => [CONFLICT_MARKER_OUR, CONFLICT_MARKER_THEIR].includes(line.type); export const isConflictSeperator = (line) => line.type === CONFLICT_MARKER; @@ -60,7 +62,7 @@ export const parallelizeDiffLines = (diffLines, inline) => { const line = diffLines[i]; line.chunk = chunk; - if (isMeta(line)) chunk += 1; + if (isMeta(line) && line.type !== EXPANDED_LINE_TYPE) chunk += 1; if (isRemoved(line) || isConflictOur(line) || inline) { lines.push({ @@ -205,7 +207,7 @@ export const findIndexInInlineLines = (lines, lineNumbers) => { ); }; -export const getPreviousLineIndex = (diffViewType, file, lineNumbers) => { +export const getPreviousLineIndex = (file, lineNumbers) => { return findIndexInInlineLines(file[INLINE_DIFF_LINES_KEY], lineNumbers); }; @@ -406,7 +408,7 @@ function deduplicateFilesList(files) { export function prepareDiffData({ diff, priorFiles = [], meta = false }) { const cleanedFiles = (diff.diff_files || []) - .map((file, index, allFiles) => prepareRawDiffFile({ file, allFiles, meta })) + .map((file, index, allFiles) => prepareRawDiffFile({ file, allFiles, meta, index })) .map(ensureBasicDiffFileLines) .map(prepareDiffFileLines) .map((file) => finalizeDiffFile(file)); diff --git a/app/assets/javascripts/diffs/utils/diff_file.js b/app/assets/javascripts/diffs/utils/diff_file.js index 54dcf70c491..bcd9fa01278 100644 --- a/app/assets/javascripts/diffs/utils/diff_file.js +++ b/app/assets/javascripts/diffs/utils/diff_file.js @@ -50,7 +50,7 @@ function identifier(file) { export const isNotDiffable = (file) => file?.viewer?.name === viewerModes.not_diffable; -export function prepareRawDiffFile({ file, allFiles, meta = false }) { +export function prepareRawDiffFile({ file, allFiles, meta = false, index = -1 }) { const additionalProperties = { brokenSymlink: fileSymlinkInformation(file, allFiles), viewer: { @@ -66,6 +66,10 @@ export function prepareRawDiffFile({ file, allFiles, meta = false }) { additionalProperties.id = identifier(file); } + if (index >= 0 && Number(index) === index) { + additionalProperties.order = index; + } + return Object.assign(file, additionalProperties); } @@ -89,6 +93,27 @@ export function getShortShaFromFile(file) { return file.content_sha ? truncateSha(String(file.content_sha)) : null; } +export function match({ fileA, fileB, mode = 'universal' } = {}) { + const matching = { + universal: (a, b) => (a?.id && b?.id ? a.id === b.id : false), + /* + * MR mode can be wildly incorrect if there is ever the possibility of files from multiple MRs + * (e.g. a browser-local merge request/file cache). + * That's why the default here is "universal" mode: UUIDs can't conflict, but you can opt into + * the dangerous one. + * + * For reference: + * file_identifier_hash === sha1( `${filePath}-${Boolean(isNew)}-${Boolean(isDeleted)}-${Boolean(isRenamed)}` ) + */ + mr: (a, b) => + a?.file_identifier_hash && b?.file_identifier_hash + ? a.file_identifier_hash === b.file_identifier_hash + : false, + }; + + return (matching[mode] || (() => false))(fileA, fileB); +} + export function stats(file) { let valid = false; let classes = ''; diff --git a/app/assets/javascripts/diffs/utils/queue_events.js b/app/assets/javascripts/diffs/utils/queue_events.js index 08fcc98d45f..cc7141df622 100644 --- a/app/assets/javascripts/diffs/utils/queue_events.js +++ b/app/assets/javascripts/diffs/utils/queue_events.js @@ -1,13 +1,31 @@ import { delay } from 'lodash'; import api from '~/api'; -import { DEFER_DURATION } from '../constants'; +import { DEFER_DURATION, TRACKING_CAP_KEY, TRACKING_CAP_LENGTH } from '../constants'; -function trackRedisHllUserEvent(event, deferDuration = 0) { +function shouldDispatchEvent() { + const timestamp = parseInt(localStorage.getItem(TRACKING_CAP_KEY), 10); + + if (Number.isNaN(timestamp)) { + return true; + } + + return timestamp + TRACKING_CAP_LENGTH < Date.now(); +} + +export function dispatchRedisHllUserEvent(event, deferDuration = 0) { delay(() => api.trackRedisHllUserEvent(event), deferDuration); } -export function queueRedisHllEvents(events) { +export function queueRedisHllEvents(events, { verifyCap = false } = {}) { + if (verifyCap) { + if (!shouldDispatchEvent()) { + return; + } + + localStorage.setItem(TRACKING_CAP_KEY, Date.now()); + } + events.forEach((event, index) => { - trackRedisHllUserEvent(event, DEFER_DURATION * (index + 1)); + dispatchRedisHllUserEvent(event, DEFER_DURATION * (index + 1)); }); } |