diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
19 files changed, 332 insertions, 175 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 253e1e3b70e..7c610968209 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -3,6 +3,13 @@ import { GlLoadingIcon, GlPagination, GlSprintf } from '@gitlab/ui'; import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import Mousetrap from 'mousetrap'; import { mapState, mapGetters, mapActions } from 'vuex'; +import { + keysFor, + MR_PREVIOUS_FILE_IN_DIFF, + MR_NEXT_FILE_IN_DIFF, + MR_COMMITS_NEXT_COMMIT, + MR_COMMITS_PREVIOUS_COMMIT, +} from '~/behaviors/shortcuts/keybindings'; import { deprecatedCreateFlash as createFlash } from '~/flash'; import { isSingleViewStyle } from '~/helpers/diffs_helper'; import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; @@ -77,6 +84,16 @@ export default { required: false, default: '', }, + endpointCodequality: { + type: String, + required: false, + default: '', + }, + endpointUpdateUser: { + type: String, + required: false, + default: '', + }, projectPath: { type: String, required: true, @@ -153,6 +170,7 @@ export default { plainDiffPath: (state) => state.diffs.plainDiffPath, emailPatchPath: (state) => state.diffs.emailPatchPath, retrievingBatches: (state) => state.diffs.retrievingBatches, + codequalityDiff: (state) => state.diffs.codequalityDiff, }), ...mapState('diffs', [ 'showTreeList', @@ -167,6 +185,7 @@ export default { 'mrReviews', ]), ...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']), + ...mapGetters('batchComments', ['draftsCount']), ...mapGetters(['isNotesFetched', 'getNoteableData']), diffs() { if (!this.viewDiffsFileByFile) { @@ -264,6 +283,7 @@ export default { endpointMetadata: this.endpointMetadata, endpointBatch: this.endpointBatch, endpointCoverage: this.endpointCoverage, + endpointUpdateUser: this.endpointUpdateUser, projectPath: this.projectPath, dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, @@ -272,6 +292,10 @@ export default { mrReviews: this.rehydratedMrReviews, }); + if (this.endpointCodequality) { + this.setCodequalityEndpoint(this.endpointCodequality); + } + if (this.shouldShow) { this.fetchData(); } @@ -316,9 +340,11 @@ export default { ...mapActions('diffs', [ 'moveToNeighboringCommit', 'setBaseConfig', + 'setCodequalityEndpoint', 'fetchDiffFilesMeta', 'fetchDiffFilesBatch', 'fetchCoverageFiles', + 'fetchCodequality', 'startRenderDiffsQueue', 'assignDiscussionsToDiff', 'setHighlightedRow', @@ -342,14 +368,6 @@ export default { refetchDiffData() { this.fetchData(false); }, - startDiffRendering() { - requestIdleCallback( - () => { - this.startRenderDiffsQueue(); - }, - { timeout: 1000 }, - ); - }, needsReload() { return this.diffFiles.length && isSingleViewStyle(this.diffFiles[0]); }, @@ -361,8 +379,6 @@ export default { .then(({ real_size }) => { this.diffFilesLength = parseInt(real_size, 10); if (toggleTree) this.setTreeDisplay(); - - this.startDiffRendering(); }) .catch(() => { createFlash(__('Something went wrong on our end. Please try again!')); @@ -377,7 +393,6 @@ export default { // 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!')); }); @@ -386,6 +401,10 @@ export default { this.fetchCoverageFiles(); } + if (this.endpointCodequality) { + this.fetchCodequality(); + } + if (!this.isNotesFetched) { notesEventHub.$emit('fetchNotesData'); } @@ -406,30 +425,23 @@ export default { } }, setEventListeners() { - Mousetrap.bind(['[', 'k', ']', 'j'], (e, combo) => { - switch (combo) { - case '[': - case 'k': - this.jumpToFile(-1); - break; - case ']': - case 'j': - this.jumpToFile(+1); - break; - default: - break; - } - }); + Mousetrap.bind(keysFor(MR_PREVIOUS_FILE_IN_DIFF), () => this.jumpToFile(-1)); + Mousetrap.bind(keysFor(MR_NEXT_FILE_IN_DIFF), () => this.jumpToFile(+1)); if (this.commit) { - Mousetrap.bind('c', () => this.moveToNeighboringCommit({ direction: 'next' })); - Mousetrap.bind('x', () => this.moveToNeighboringCommit({ direction: 'previous' })); + Mousetrap.bind(keysFor(MR_COMMITS_NEXT_COMMIT), () => + this.moveToNeighboringCommit({ direction: 'next' }), + ); + Mousetrap.bind(keysFor(MR_COMMITS_PREVIOUS_COMMIT), () => + this.moveToNeighboringCommit({ direction: 'previous' }), + ); } }, removeEventListeners() { - Mousetrap.unbind(['[', 'k', ']', 'j']); - Mousetrap.unbind('c'); - Mousetrap.unbind('x'); + Mousetrap.unbind(keysFor(MR_PREVIOUS_FILE_IN_DIFF)); + Mousetrap.unbind(keysFor(MR_NEXT_FILE_IN_DIFF)); + Mousetrap.unbind(keysFor(MR_COMMITS_NEXT_COMMIT)); + Mousetrap.unbind(keysFor(MR_COMMITS_PREVIOUS_COMMIT)); }, jumpToFile(step) { const targetIndex = this.currentDiffIndex + step; @@ -489,6 +501,7 @@ export default { <div v-if="renderFileTree" :style="{ width: `${treeWidth}px` }" + :class="{ 'review-bar-visible': draftsCount > 0 }" class="diff-tree-list js-diff-tree-list px-3 pr-md-0" > <panel-resizer diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index 92b317eb3f0..bc0f2fb0b69 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -1,7 +1,6 @@ <script> /* eslint-disable vue/no-v-html */ -import { GlButtonGroup, GlButton, GlTooltipDirective, GlIcon } from '@gitlab/ui'; -import { mapActions } from 'vuex'; +import { GlButtonGroup, GlButton, GlTooltipDirective } from '@gitlab/ui'; import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; @@ -9,7 +8,6 @@ import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import { setUrlParams } from '../../lib/utils/url_utility'; import initUserPopovers from '../../user_popovers'; /** @@ -24,14 +22,6 @@ import initUserPopovers from '../../user_popovers'; * coexist, but there is an issue to remove the duplication. * https://gitlab.com/gitlab-org/gitlab-foss/issues/51613 * - * EXCEPTION WARNING - * 1. The commit navigation buttons (next neighbor, previous neighbor) - * are not duplicated because: - * - We don't have the same data available on the Rails side (yet, - * without backend work) - * - This Vue component should always be what's used when in the - * context of an MR diff, so the HAML should never have any idea - * about navigating among commits. */ export default { @@ -42,7 +32,6 @@ export default { CommitPipelineStatus, GlButtonGroup, GlButton, - GlIcon, }, directives: { GlTooltip: GlTooltipDirective, @@ -94,28 +83,12 @@ export default { // Strip the newline at the beginning return this.commit.description_html.replace(/^
/, ''); }, - nextCommitUrl() { - return this.commit.next_commit_id - ? setUrlParams({ commit_id: this.commit.next_commit_id }) - : ''; - }, - previousCommitUrl() { - return this.commit.prev_commit_id - ? setUrlParams({ commit_id: this.commit.prev_commit_id }) - : ''; - }, - hasNeighborCommits() { - return this.commit.next_commit_id || this.commit.prev_commit_id; - }, }, created() { this.$nextTick(() => { initUserPopovers(this.$el.querySelectorAll('.js-user-link')); }); }, - methods: { - ...mapActions('diffs', ['moveToNeighboringCommit']), - }, }; </script> @@ -146,38 +119,6 @@ export default { class="input-group-text" /> </gl-button-group> - <div v-if="hasNeighborCommits" class="commit-nav-buttons ml-3"> - <gl-button-group> - <gl-button - :href="previousCommitUrl" - :disabled="!commit.prev_commit_id" - @click.prevent="moveToNeighboringCommit({ direction: 'previous' })" - > - <span - v-if="!commit.prev_commit_id" - v-gl-tooltip - class="h-100 w-100 position-absolute" - :title="__('You\'re at the first commit')" - ></span> - <gl-icon name="chevron-left" /> - {{ __('Prev') }} - </gl-button> - <gl-button - :href="nextCommitUrl" - :disabled="!commit.next_commit_id" - @click.prevent="moveToNeighboringCommit({ direction: 'next' })" - > - <span - v-if="!commit.next_commit_id" - v-gl-tooltip - class="h-100 w-100 position-absolute" - :title="__('You\'re at the last commit')" - ></span> - {{ __('Next') }} - <gl-icon name="chevron-right" /> - </gl-button> - </gl-button-group> - </div> </div> <div> <div class="d-flex float-left align-items-center align-self-start"> diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index 6b1e2bfb34e..7526c5347f7 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -1,7 +1,8 @@ <script> -import { GlTooltipDirective, GlLink, GlButton, GlSprintf } from '@gitlab/ui'; +import { GlTooltipDirective, GlIcon, GlLink, GlButtonGroup, GlButton, GlSprintf } from '@gitlab/ui'; import { mapActions, mapGetters, mapState } from 'vuex'; import { __ } from '~/locale'; +import { setUrlParams } from '../../lib/utils/url_utility'; import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '../constants'; import eventHub from '../event_hub'; import CompareDropdownLayout from './compare_dropdown_layout.vue'; @@ -11,7 +12,9 @@ import SettingsDropdown from './settings_dropdown.vue'; export default { components: { CompareDropdownLayout, + GlIcon, GlLink, + GlButtonGroup, GlButton, GlSprintf, SettingsDropdown, @@ -56,6 +59,19 @@ export default { hasSourceVersions() { return this.diffCompareDropdownSourceVersions.length > 0; }, + nextCommitUrl() { + return this.commit.next_commit_id + ? setUrlParams({ commit_id: this.commit.next_commit_id }) + : ''; + }, + previousCommitUrl() { + return this.commit.prev_commit_id + ? setUrlParams({ commit_id: this.commit.prev_commit_id }) + : ''; + }, + hasNeighborCommits() { + return this.commit && (this.commit.next_commit_id || this.commit.prev_commit_id); + }, }, created() { this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES; @@ -65,6 +81,7 @@ export default { expandAllFiles() { eventHub.$emit(EVT_EXPAND_ALL_FILES); }, + ...mapActions('diffs', ['moveToNeighboringCommit']), }, }; </script> @@ -84,6 +101,7 @@ export default { icon="file-tree" class="gl-mr-3 js-toggle-tree-list" :title="toggleFileBrowserTitle" + :aria-label="toggleFileBrowserTitle" :selected="showTreeList" @click="setShowTreeList({ showTreeList: !showTreeList })" /> @@ -91,6 +109,38 @@ export default { {{ __('Viewing commit') }} <gl-link :href="commit.commit_url" class="monospace">{{ commit.short_id }}</gl-link> </div> + <div v-if="hasNeighborCommits" class="commit-nav-buttons ml-3"> + <gl-button-group> + <gl-button + :href="previousCommitUrl" + :disabled="!commit.prev_commit_id" + @click.prevent="moveToNeighboringCommit({ direction: 'previous' })" + > + <span + v-if="!commit.prev_commit_id" + v-gl-tooltip + class="h-100 w-100 position-absolute position-top-0 position-left-0" + :title="__('You\'re at the first commit')" + ></span> + <gl-icon name="chevron-left" /> + {{ __('Prev') }} + </gl-button> + <gl-button + :href="nextCommitUrl" + :disabled="!commit.next_commit_id" + @click.prevent="moveToNeighboringCommit({ direction: 'next' })" + > + <span + v-if="!commit.next_commit_id" + v-gl-tooltip + class="h-100 w-100 position-absolute position-top-0 position-left-0" + :title="__('You\'re at the last commit')" + ></span> + {{ __('Next') }} + <gl-icon name="chevron-right" /> + </gl-button> + </gl-button-group> + </div> <gl-sprintf v-else-if="hasSourceVersions" class="d-flex align-items-center compare-versions-container" diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index d0d457d8582..5e05ec87f84 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -68,6 +68,7 @@ export default { }" type="button" class="js-diff-notes-toggle" + :aria-label="__('Show comments')" @click="toggleDiscussion({ discussionId: discussion.id })" > <gl-icon v-if="discussion.expanded" name="collapse" class="collapse-icon" /> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index ca4543f7002..bdbc13a38c4 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -80,7 +80,7 @@ export default { genericError: GENERIC_ERROR, }, computed: { - ...mapState('diffs', ['currentDiffFileId']), + ...mapState('diffs', ['currentDiffFileId', 'codequalityDiff']), ...mapGetters(['isNotesFetched']), ...mapGetters('diffs', ['getDiffFileDiscussions']), viewBlobHref() { @@ -148,6 +148,11 @@ export default { return loggedIn && featureOn; }, + hasCodequalityChanges() { + return ( + this.codequalityDiff?.files && this.codequalityDiff?.files[this.file.file_path]?.length > 0 + ); + }, }, watch: { 'file.id': { @@ -294,6 +299,7 @@ export default { :add-merge-request-buttons="true" :view-diffs-file-by-file="viewDiffsFileByFile" :show-local-file-reviews="showLocalFileReviews" + :has-codequality-changes="hasCodequalityChanges" class="js-file-title file-title gl-border-1 gl-border-solid gl-border-gray-100" :class="hasBodyClasses.header" @toggleFile="handleToggle" diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 1f50b3a38a6..3b4e21ab61b 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -41,6 +41,7 @@ export default { GlDropdownDivider, GlFormCheckbox, GlLoadingIcon, + CodeQualityBadge: () => import('ee_component/diffs/components/code_quality_badge.vue'), }, directives: { GlTooltip: GlTooltipDirective, @@ -49,6 +50,7 @@ export default { mixins: [glFeatureFlagsMixin()], i18n: { ...DIFF_FILE_HEADER, + compareButtonLabel: s__('Compare submodule commit revisions'), }, props: { discussionPath: { @@ -94,6 +96,11 @@ export default { required: false, default: false, }, + hasCodequalityChanges: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { @@ -192,6 +199,9 @@ export default { isReviewable() { return reviewable(this.diffFile); }, + externalUrlLabel() { + return sprintf(__('View on %{url}'), { url: this.diffFile.formatted_external_url }); + }, }, methods: { ...mapActions('diffs', [ @@ -323,6 +333,8 @@ export default { data-track-property="diff_copy_file" /> + <code-quality-badge v-if="hasCodequalityChanges" class="gl-mr-2" /> + <small v-if="isModeChanged" ref="fileMode" class="mr-1"> {{ diffFile.a_mode }} → {{ diffFile.b_mode }} </small> @@ -352,7 +364,8 @@ export default { ref="externalLink" v-gl-tooltip.hover :href="diffFile.external_url" - :title="`View on ${diffFile.formatted_external_url}`" + :title="externalUrlLabel" + :aria-label="externalUrlLabel" target="_blank" data-track-event="click_toggle_external_button" data-track-label="diff_toggle_external_button" @@ -444,7 +457,8 @@ export default { v-gl-tooltip.hover v-safe-html="submoduleDiffCompareLinkText" class="submodule-compare" - :title="s__('Compare submodule commit revisions')" + :title="$options.i18n.compareButtonLabel" + :aria-label="$options.i18n.compareButtonLabel" :href="diffFile.submodule_compare.url" /> </div> diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue index 2f09f2e24b2..51da1966630 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -10,7 +10,12 @@ import { } from '../../notes/components/multiline_comment_utils'; import noteForm from '../../notes/components/note_form.vue'; import autosave from '../../notes/mixins/autosave'; -import { DIFF_NOTE_TYPE, INLINE_DIFF_LINES_KEY, PARALLEL_DIFF_VIEW_TYPE } from '../constants'; +import { + DIFF_NOTE_TYPE, + INLINE_DIFF_LINES_KEY, + PARALLEL_DIFF_VIEW_TYPE, + OLD_LINE_TYPE, +} from '../constants'; export default { components: { @@ -113,6 +118,34 @@ export default { const lines = getDiffLines(); return commentLineOptions(lines, this.line, this.line.line_code, side); }, + commentLines() { + if (!this.selectedCommentPosition) return []; + + const lines = []; + const { start, end } = this.selectedCommentPosition; + const diffLines = this.diffFile[INLINE_DIFF_LINES_KEY]; + let isAdding = false; + + for (let i = 0, diffLinesLength = diffLines.length - 1; i <= diffLinesLength; i += 1) { + const line = diffLines[i]; + + if (start.line_code === line.line_code) { + isAdding = true; + } + + if (isAdding) { + if (line.type !== OLD_LINE_TYPE) { + lines.push(line); + } + + if (end.line_code === line.line_code) { + break; + } + } + } + + return lines; + }, }, mounted() { if (this.isLoggedIn) { @@ -177,6 +210,7 @@ export default { :is-editing="true" :line-code="line.line_code" :line="line" + :lines="commentLines" :help-page-path="helpPagePath" :diff-file="diffFile" :show-suggest-popover="showSuggestPopover" diff --git a/app/assets/javascripts/diffs/components/diff_row.vue b/app/assets/javascripts/diffs/components/diff_row.vue index ab6890d66b5..8d398a2ded4 100644 --- a/app/assets/javascripts/diffs/components/diff_row.vue +++ b/app/assets/javascripts/diffs/components/diff_row.vue @@ -1,5 +1,6 @@ <script> -import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; +/* eslint-disable vue/no-v-html */ +import { GlTooltipDirective } from '@gitlab/ui'; import { mapActions, mapGetters, mapState } from 'vuex'; import { BV_HIDE_TOOLTIP } from '~/lib/utils/constants'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; @@ -12,17 +13,20 @@ import { CONFLICT_THEIR, CONFLICT_MARKER, } from '../constants'; +import { + getInteropInlineAttributes, + getInteropOldSideAttributes, + getInteropNewSideAttributes, +} from '../utils/interoperability'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; import * as utils from './diff_row_utils'; export default { components: { - GlIcon, DiffGutterAvatars, }, directives: { GlTooltip: GlTooltipDirective, - SafeHtml, }, mixins: [glFeatureFlagsMixin()], props: { @@ -117,6 +121,16 @@ export default { isLeftConflictMarker() { return [CONFLICT_MARKER_OUR, CONFLICT_MARKER_THEIR].includes(this.line.left?.type); }, + interopLeftAttributes() { + if (this.inline) { + return getInteropInlineAttributes(this.line.left); + } + + return getInteropOldSideAttributes(this.line.left); + }, + interopRightAttributes() { + return getInteropNewSideAttributes(this.line.right); + }, }, mounted() { this.scrollToLineIfNeededParallel(this.line); @@ -182,6 +196,7 @@ export default { <div data-testid="left-side" class="diff-grid-left left-side" + v-bind="interopLeftAttributes" @dragover.prevent @dragenter="onDragEnter(line.left, index)" @dragend="onDragEnd" @@ -203,14 +218,13 @@ export default { <button :draggable="glFeatures.dragCommentSelection" type="button" - class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + class="add-diff-note unified-diff-components-diff-note-button note-button js-add-diff-note-button qa-diff-comment" + data-qa-selector="diff_comment_button" :class="{ 'gl-cursor-grab': dragging }" :disabled="line.left.commentsDisabled" @click="handleCommentButton(line.left)" @dragstart="onDragStart({ ...line.left, index })" - > - <gl-icon :size="12" name="comment" /> - </button> + ></button> </span> </template> <a @@ -258,7 +272,7 @@ export default { @mousedown="handleParallelLineMouseDown" > <strong v-if="isLeftConflictMarker">{{ conflictText(line.left) }}</strong> - <span v-else v-safe-html="line.left.rich_text"></span> + <span v-else v-html="line.left.rich_text"></span> </div> </template> <template v-else-if="!inline || (line.left && line.left.type === $options.CONFLICT_MARKER)"> @@ -288,6 +302,7 @@ export default { v-if="!inline" data-testid="right-side" class="diff-grid-right right-side" + v-bind="interopRightAttributes" @dragover.prevent @dragenter="onDragEnter(line.right, index)" @dragend="onDragEnd" @@ -305,14 +320,12 @@ export default { <button :draggable="glFeatures.dragCommentSelection" type="button" - class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + class="add-diff-note unified-diff-components-diff-note-button note-button js-add-diff-note-button qa-diff-comment" :class="{ 'gl-cursor-grab': dragging }" :disabled="line.right.commentsDisabled" @click="handleCommentButton(line.right)" @dragstart="onDragStart({ ...line.right, index })" - > - <gl-icon :size="12" name="comment" /> - </button> + ></button> </span> </template> <a @@ -349,7 +362,6 @@ export default { <div :id="line.right.line_code" :key="line.right.rich_text" - v-safe-html="line.right.rich_text" :class="[ line.right.type, { @@ -364,7 +376,7 @@ export default { <strong v-if="line.right.type === $options.CONFLICT_MARKER_THEIR">{{ conflictText(line.right) }}</strong> - <span v-else v-safe-html="line.right.rich_text"></span> + <span v-else v-html="line.right.rich_text"></span> </div> </template> <template v-else> diff --git a/app/assets/javascripts/diffs/components/image_diff_overlay.vue b/app/assets/javascripts/diffs/components/image_diff_overlay.vue index 3d05202fb2d..5572338908f 100644 --- a/app/assets/javascripts/diffs/components/image_diff_overlay.vue +++ b/app/assets/javascripts/diffs/components/image_diff_overlay.vue @@ -122,6 +122,7 @@ export default { :disabled="!shouldToggleDiscussion" class="js-image-badge" type="button" + :aria-label="__('Show comments')" @click="clickedToggle(discussion)" > <gl-icon v-if="showCommentIcon" name="image-comment-dark" :size="24" /> 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 fb9202c5aab..25403b1547e 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -2,6 +2,7 @@ import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import { mapActions, mapGetters, mapState } from 'vuex'; import { CONTEXT_LINE_CLASS_NAME } from '../constants'; +import { getInteropInlineAttributes } from '../utils/interoperability'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; import { isHighlighted, @@ -96,6 +97,9 @@ export default { shouldShowAvatarsOnGutter() { return this.line.hasDiscussions; }, + interopAttrs() { + return getInteropInlineAttributes(this.line); + }, }, mounted() { this.scrollToLineIfNeededInline(this.line); @@ -124,6 +128,7 @@ export default { :id="inlineRowId" :class="classNameMap" class="line_holder" + v-bind="interopAttrs" @mouseover="handleMouseMove" @mouseout="handleMouseMove" > @@ -140,8 +145,8 @@ export default { ref="addDiffNoteButton" type="button" class="add-diff-note note-button js-add-diff-note-button" - data-qa-selector="diff_comment_button" :disabled="line.commentsDisabled" + :aria-label="addCommentTooltip" @click="handleCommentButton" > <gl-icon :size="12" name="comment" /> 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 3d20dfd0c9b..96946d0fd88 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -3,6 +3,10 @@ import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gi import $ from 'jquery'; import { mapActions, mapGetters, mapState } from 'vuex'; import { CONTEXT_LINE_CLASS_NAME, PARALLEL_DIFF_VIEW_TYPE } from '../constants'; +import { + getInteropOldSideAttributes, + getInteropNewSideAttributes, +} from '../utils/interoperability'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; import * as utils from './diff_row_utils'; @@ -108,6 +112,12 @@ export default { this.line.hasDiscussionsRight, ); }, + interopLeftAttributes() { + return getInteropOldSideAttributes(this.line.left); + }, + interopRightAttributes() { + return getInteropNewSideAttributes(this.line.right); + }, }, mounted() { this.scrollToLineIfNeededParallel(this.line); @@ -185,6 +195,7 @@ export default { type="button" class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" :disabled="line.left.commentsDisabled" + :aria-label="addCommentTooltipLeft" @click="handleCommentButton(line.left)" > <gl-icon :size="12" name="comment" /> @@ -217,6 +228,7 @@ export default { :key="line.left.line_code" v-safe-html="line.left.rich_text" :class="parallelViewLeftLineType" + v-bind="interopLeftAttributes" class="line_content with-coverage parallel left-side" @mousedown="handleParallelLineMouseDown" ></td> @@ -241,6 +253,7 @@ export default { type="button" class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" :disabled="line.right.commentsDisabled" + :aria-label="addCommentTooltipRight" @click="handleCommentButton(line.right)" > <gl-icon :size="12" name="comment" /> @@ -283,6 +296,7 @@ export default { hll: isHighlighted, }, ]" + v-bind="interopRightAttributes" class="line_content with-coverage parallel right-side" @mousedown="handleParallelLineMouseDown" ></td> diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 87e9af174e5..5a8862c2b70 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -73,6 +73,8 @@ export default function initDiffsApp(store) { endpointMetadata: dataset.endpointMetadata || '', endpointBatch: dataset.endpointBatch || '', endpointCoverage: dataset.endpointCoverage || '', + endpointCodequality: dataset.endpointCodequality || '', + endpointUpdateUser: dataset.updateCurrentUserPath, projectPath: dataset.projectPath, helpPagePath: dataset.helpPagePath, currentUser: JSON.parse(dataset.currentUserData) || {}, @@ -114,6 +116,8 @@ export default function initDiffsApp(store) { endpointMetadata: this.endpointMetadata, endpointBatch: this.endpointBatch, endpointCoverage: this.endpointCoverage, + endpointCodequality: this.endpointCodequality, + endpointUpdateUser: this.endpointUpdateUser, currentUser: this.currentUser, projectPath: this.projectPath, helpPagePath: this.helpPagePath, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 8796016def9..428faf693b0 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -49,7 +49,6 @@ import { convertExpandLines, idleCallback, allDiscussionWrappersExpanded, - prepareDiffData, prepareLineForRenamedFile, } from './utils'; @@ -59,6 +58,7 @@ export const setBaseConfig = ({ commit }, options) => { endpointMetadata, endpointBatch, endpointCoverage, + endpointUpdateUser, projectPath, dismissEndpoint, showSuggestPopover, @@ -71,6 +71,7 @@ export const setBaseConfig = ({ commit }, options) => { endpointMetadata, endpointBatch, endpointCoverage, + endpointUpdateUser, projectPath, dismissEndpoint, showSuggestPopover, @@ -163,7 +164,15 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { return pagination.next_page; }) - .then((nextPage) => nextPage && getBatch(nextPage)) + .then((nextPage) => { + dispatch('startRenderDiffsQueue'); + + if (nextPage) { + return getBatch(nextPage); + } + + return null; + }) .catch(() => commit(types.SET_RETRIEVING_BATCHES, false)); return getBatch() @@ -197,13 +206,7 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { commit(types.SET_MERGE_REQUEST_DIFFS, data.merge_request_diffs || []); commit(types.SET_DIFF_METADATA, strippedData); - worker.postMessage( - prepareDiffData({ - diff: data, - priorFiles: state.diffFiles, - meta: true, - }), - ); + worker.postMessage(data.diff_files); return data; }) @@ -304,33 +307,41 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi }; export const startRenderDiffsQueue = ({ state, commit }) => { - const checkItem = () => - new Promise((resolve) => { - const nextFile = state.diffFiles.find( - (file) => - !file.renderIt && - file.viewer && - (!isCollapsed(file) || file.viewer.name !== diffViewerModes.text), - ); - - if (nextFile) { - requestAnimationFrame(() => { - commit(types.RENDER_FILE, nextFile); + const diffFilesToRender = state.diffFiles.filter( + (file) => + !file.renderIt && + file.viewer && + (!isCollapsed(file) || file.viewer.name !== diffViewerModes.text), + ); + let currentDiffFileIndex = 0; + + const checkItem = () => { + const nextFile = diffFilesToRender[currentDiffFileIndex]; + + if (nextFile) { + let retryCount = 0; + currentDiffFileIndex += 1; + commit(types.RENDER_FILE, nextFile); + + const requestIdle = () => + requestIdleCallback((idleDeadline) => { + // Wait for at least 5ms before trying to render + // or for 5 tries and then force render the file + if (idleDeadline.timeRemaining() >= 5 || retryCount > 4) { + checkItem(); + } else { + requestIdle(); + retryCount += 1; + } }); - requestIdleCallback( - () => { - checkItem() - .then(resolve) - .catch(() => {}); - }, - { timeout: 1000 }, - ); - } else { - resolve(); - } - }); - return checkItem(); + requestIdle(); + } + }; + + if (diffFilesToRender.length) { + checkItem(); + } }; export const setRenderIt = ({ commit }, file) => commit(types.RENDER_FILE, file); @@ -738,10 +749,22 @@ export const navigateToDiffFileIndex = ({ commit, state }, index) => { commit(types.VIEW_DIFF_FILE, fileHash); }; -export const setFileByFile = ({ commit }, { fileByFile }) => { +export const setFileByFile = ({ state, commit }, { fileByFile }) => { const fileViewMode = fileByFile ? DIFF_VIEW_FILE_BY_FILE : DIFF_VIEW_ALL_FILES; commit(types.SET_FILE_BY_FILE, fileByFile); Cookies.set(DIFF_FILE_BY_FILE_COOKIE_NAME, fileViewMode); + + return axios + .put(state.endpointUpdateUser, { + view_diffs_file_by_file: fileByFile, + }) + .then(() => { + // https://gitlab.com/gitlab-org/gitlab/-/issues/326961 + // We can't even do a simple console warning here because + // the pipeline will fail. However, the issue above will + // eventually handle errors appropriately. + // console.warn('Saving the file-by-fil user preference failed.'); + }); }; export function reviewFile({ commit, state }, { file, reviewed = true }) { diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 1fc2a684e95..dec3f87b03e 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -156,16 +156,16 @@ export const diffLines = (state) => (file, unifiedDiffComponents) => { ); }; -export function suggestionCommitMessage(state) { +export function suggestionCommitMessage(state, _, rootState) { return (values = {}) => computeSuggestionCommitMessage({ message: state.defaultSuggestionCommitMessage, values: { - branch_name: state.branchName, - project_path: state.projectPath, - project_name: state.projectName, - username: state.username, - user_full_name: state.userFullName, + branch_name: rootState.page.mrMetadata.branch_name, + project_path: rootState.page.mrMetadata.project_path, + project_name: rootState.page.mrMetadata.project_name, + username: rootState.page.mrMetadata.username, + user_full_name: rootState.page.mrMetadata.user_full_name, ...values, }, }); diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index f93435363ec..1674d3d3b5a 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -9,7 +9,8 @@ import { import { fileByFile } from '../../utils/preferences'; import { getDefaultWhitespace } from '../utils'; -const viewTypeFromQueryString = getParameterValues('view')[0]; +const getViewTypeFromQueryString = () => getParameterValues('view')[0]; + const viewTypeFromCookie = Cookies.get(DIFF_VIEW_COOKIE_NAME); const defaultViewType = INLINE_DIFF_VIEW_TYPE; const whiteSpaceFromQueryString = getParameterValues('w')[0]; @@ -23,6 +24,7 @@ export default () => ({ addedLines: null, removedLines: null, endpoint: '', + endpointUpdateUser: '', basePath: '', commit: null, startVersion: null, // Null unless a target diff is selected for comparison that is not the "base" diff @@ -30,7 +32,7 @@ export default () => ({ coverageFiles: {}, mergeRequestDiffs: [], mergeRequestDiff: null, - diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType, + diffViewType: getViewTypeFromQueryString() || viewTypeFromCookie || defaultViewType, tree: [], treeEntries: {}, showTreeList: true, diff --git a/app/assets/javascripts/diffs/store/modules/index.js b/app/assets/javascripts/diffs/store/modules/index.js index 6860e24db6b..03d11e60745 100644 --- a/app/assets/javascripts/diffs/store/modules/index.js +++ b/app/assets/javascripts/diffs/store/modules/index.js @@ -1,7 +1,7 @@ -import * as actions from '../actions'; +import * as actions from 'ee_else_ce/diffs/store/actions'; +import createState from 'ee_else_ce/diffs/store/modules/diff_state'; +import mutations from 'ee_else_ce/diffs/store/mutations'; import * as getters from '../getters'; -import mutations from '../mutations'; -import createState from './diff_state'; export default () => ({ namespaced: true, diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index d06793c05af..9ff9a02d444 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -33,6 +33,7 @@ export default { endpointMetadata, endpointBatch, endpointCoverage, + endpointUpdateUser, projectPath, dismissEndpoint, showSuggestPopover, @@ -45,6 +46,7 @@ export default { endpointMetadata, endpointBatch, endpointCoverage, + endpointUpdateUser, projectPath, dismissEndpoint, showSuggestPopover, @@ -77,15 +79,10 @@ export default { }, [types.SET_DIFF_DATA_BATCH](state, data) { - const files = prepareDiffData({ + state.diffFiles = prepareDiffData({ diff: data, priorFiles: state.diffFiles, }); - - Object.assign(state, { - ...convertObjectPropsToCamelCase(data), - }); - updateDiffFilesInState(state, files); }, [types.SET_COVERAGE_DATA](state, coverageFiles) { diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index b37a75eb2a3..7fa51b9ddea 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -381,22 +381,13 @@ function prepareDiffFileLines(file) { inlineLines.forEach((line) => prepareLine(line, file)); // WARNING: In-Place Mutations! - Object.assign(file, { - inlineLinesCount: inlineLines.length, - }); - return file; } -function getVisibleDiffLines(file) { - return file.inlineLinesCount; -} - -function finalizeDiffFile(file) { - const lines = getVisibleDiffLines(file); - +function finalizeDiffFile(file, index) { Object.assign(file, { - renderIt: lines < LINES_TO_BE_RENDERED_DIRECTLY, + renderIt: + index < 3 ? file[INLINE_DIFF_LINES_KEY].length < LINES_TO_BE_RENDERED_DIRECTLY : false, isShowingFullFile: false, isLoadingFullFile: false, discussions: [], @@ -424,7 +415,7 @@ export function prepareDiffData({ diff, priorFiles = [], meta = false }) { .map((file, index, allFiles) => prepareRawDiffFile({ file, allFiles, meta })) .map(ensureBasicDiffFileLines) .map(prepareDiffFileLines) - .map(finalizeDiffFile); + .map((file, index) => finalizeDiffFile(file, priorFiles.length + index)); return deduplicateFilesList([...priorFiles, ...cleanedFiles]); } diff --git a/app/assets/javascripts/diffs/utils/interoperability.js b/app/assets/javascripts/diffs/utils/interoperability.js new file mode 100644 index 00000000000..a52e8fd25f5 --- /dev/null +++ b/app/assets/javascripts/diffs/utils/interoperability.js @@ -0,0 +1,49 @@ +const OLD = 'old'; +const NEW = 'new'; +const ATTR_PREFIX = 'data-interop-'; + +export const ATTR_TYPE = `${ATTR_PREFIX}type`; +export const ATTR_LINE = `${ATTR_PREFIX}line`; +export const ATTR_NEW_LINE = `${ATTR_PREFIX}new-line`; +export const ATTR_OLD_LINE = `${ATTR_PREFIX}old-line`; + +export const getInteropInlineAttributes = (line) => { + if (!line) { + return null; + } + + const interopType = line.type?.startsWith(OLD) ? OLD : NEW; + + const interopLine = interopType === OLD ? line.old_line : line.new_line; + + return { + [ATTR_TYPE]: interopType, + [ATTR_LINE]: interopLine, + [ATTR_NEW_LINE]: line.new_line, + [ATTR_OLD_LINE]: line.old_line, + }; +}; + +export const getInteropOldSideAttributes = (line) => { + if (!line) { + return null; + } + + return { + [ATTR_TYPE]: OLD, + [ATTR_LINE]: line.old_line, + [ATTR_OLD_LINE]: line.old_line, + }; +}; + +export const getInteropNewSideAttributes = (line) => { + if (!line) { + return null; + } + + return { + [ATTR_TYPE]: NEW, + [ATTR_LINE]: line.new_line, + [ATTR_NEW_LINE]: line.new_line, + }; +}; |