diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
14 files changed, 180 insertions, 45 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 465f9836140..f405b82b05b 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -44,6 +44,7 @@ import { TRACKING_MULTIPLE_FILES_MODE, } from '../constants'; +import { discussionIntersectionObserverHandlerFactory } from '../utils/discussions'; import diffsEventHub from '../event_hub'; import { reviewStatuses } from '../utils/file_reviews'; import { diffsApp } from '../utils/performance'; @@ -86,6 +87,9 @@ export default { ALERT_MERGE_CONFLICT, ALERT_COLLAPSED_FILES, }, + provide: { + discussionObserverHandler: discussionIntersectionObserverHandlerFactory(), + }, props: { endpoint: { type: String, @@ -392,8 +396,6 @@ export default { diffsApp.instrument(); }, created() { - this.mergeRequestContainers = document.querySelectorAll('.merge-request-container'); - this.adjustView(); this.subscribeToEvents(); @@ -521,13 +523,6 @@ export default { } else { this.removeEventListeners(); } - - if (!this.isFluidLayout && this.glFeatures.mrChangesFluidLayout) { - this.mergeRequestContainers.forEach((el) => { - el.classList.toggle('limit-container-width', !this.shouldShow); - el.classList.toggle('container-limited', !this.shouldShow); - }); - } }, setEventListeners() { Mousetrap.bind(keysFor(MR_PREVIOUS_FILE_IN_DIFF), () => this.jumpToFile(-1)); @@ -579,7 +574,7 @@ export default { jumpToFile(step) { const targetIndex = this.currentDiffIndex + step; if (targetIndex >= 0 && targetIndex < this.diffFiles.length) { - this.scrollToFile(this.diffFiles[targetIndex].file_path); + this.scrollToFile({ path: this.diffFiles[targetIndex].file_path }); } }, setTreeDisplay() { diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index 4435a533591..e54fde72847 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -1,5 +1,5 @@ <script> -import { GlButtonGroup, GlButton, GlTooltipDirective } from '@gitlab/ui'; +import { GlButtonGroup, GlButton, GlTooltipDirective, GlSafeHtmlDirective } from '@gitlab/ui'; import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; @@ -34,6 +34,7 @@ export default { }, directives: { GlTooltip: GlTooltipDirective, + SafeHtml: GlSafeHtmlDirective, }, mixins: [glFeatureFlagsMixin()], props: { @@ -88,6 +89,9 @@ export default { initUserPopovers(this.$el.querySelectorAll('.js-user-link')); }); }, + safeHtmlConfig: { + ADD_TAGS: ['gl-emoji'], + }, }; </script> @@ -101,7 +105,7 @@ export default { > <div v-if="commit.signature_html" - v-html="commit.signature_html /* eslint-disable-line vue/no-v-html */" + v-safe-html:[$options.safeHtmlConfig]="commit.signature_html" ></div> <commit-pipeline-status v-if="commit.pipeline_status_path" @@ -142,9 +146,9 @@ export default { <div class="commit-detail flex-list"> <div class="commit-content" data-qa-selector="commit_content"> <a + v-safe-html:[$options.safeHtmlConfig]="commit.title_html" :href="commit.commit_url" class="commit-row-message item-title" - v-html="commit.title_html /* eslint-disable-line vue/no-v-html */" ></a> <span class="commit-row-message d-block d-sm-none">· {{ commit.short_id }}</span> @@ -174,9 +178,9 @@ export default { <div> <pre v-if="commit.description_html" + v-safe-html:[$options.safeHtmlConfig]="commitDescription" :class="{ 'js-toggle-content': collapsible, 'd-block': !collapsible }" class="commit-row-description gl-mb-3 gl-text-body" - v-html="commitDescription /* eslint-disable-line vue/no-v-html */" ></pre> </div> </li> diff --git a/app/assets/javascripts/diffs/components/diff_comment_cell.vue b/app/assets/javascripts/diffs/components/diff_comment_cell.vue index 4af4b46f94c..a4fae652d02 100644 --- a/app/assets/javascripts/diffs/components/diff_comment_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_comment_cell.vue @@ -29,6 +29,11 @@ export default { required: false, default: false, }, + lineRange: { + type: Object, + required: false, + default: null, + }, linePosition: { type: String, required: false, @@ -59,6 +64,7 @@ export default { <diff-line-note-form :diff-file-hash="diffFileHash" :line="line" + :range="lineRange" :note-target-line="line" :help-page-path="helpPagePath" :line-position="linePosition" diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 4bcb99424db..238f07ac22c 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -14,7 +14,6 @@ import { import { escape } from 'lodash'; import { mapActions, mapGetters, mapState } from 'vuex'; import { IdState } from 'vendor/vue-virtual-scroller'; -import { diffViewerModes } from '~/ide/constants'; import { scrollToElement } from '~/lib/utils/common_utils'; import { truncateSha } from '~/lib/utils/text_utility'; import { __, s__, sprintf } from '~/locale'; @@ -50,7 +49,7 @@ export default { mixins: [glFeatureFlagsMixin(), IdState({ idProp: (vm) => vm.diffFile.file_hash })], i18n: { ...DIFF_FILE_HEADER, - compareButtonLabel: s__('Compare submodule commit revisions'), + compareButtonLabel: __('Compare submodule commit revisions'), }, props: { discussionPath: { @@ -130,7 +129,7 @@ export default { 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}'), + __('Compare %{oldCommitId}...%{newCommitId}'), { oldCommitId: `<span class="commit-sha">${truncatedOldSha}</span>`, newCommitId: `<span class="commit-sha">${truncatedNewSha}</span>`, @@ -181,7 +180,7 @@ export default { return this.diffFile.renamed_file; }, isModeChanged() { - return this.diffFile.viewer.name === diffViewerModes.mode_changed; + return this.diffFile.mode_changed; }, expandDiffToFullFileTitle() { if (this.diffFile.isShowingFullFile) { @@ -221,7 +220,7 @@ export default { 'toggleFileDiscussions', 'toggleFileDiscussionWrappers', 'toggleFullDiff', - 'toggleActiveFileByHash', + 'setCurrentFileHash', 'reviewFile', 'setFileCollapsedByUser', ]), @@ -244,7 +243,7 @@ export default { scrollToElement(document.querySelector(selector)); window.location.hash = selector; if (!this.viewDiffsFileByFile) { - this.toggleActiveFileByHash(this.diffFile.file_hash); + this.setCurrentFileHash(this.diffFile.file_hash); } } }, 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 c445989f143..9d355c96af1 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -2,6 +2,7 @@ import { mapState, mapGetters, mapActions } from 'vuex'; 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'; import MultilineCommentForm from '../../notes/components/multiline_comment_form.vue'; import { @@ -32,6 +33,11 @@ export default { type: Object, required: true, }, + range: { + type: Object, + required: false, + default: null, + }, linePosition: { type: String, required: false, @@ -49,6 +55,7 @@ export default { }, data() { return { + lines: null, commentLineStart: { line_code: this.line.line_code, type: this.line.type, @@ -116,10 +123,8 @@ export default { return commentLineOptions(lines, this.line, this.line.line_code, side); }, commentLines() { - if (!this.selectedCommentPosition) return []; - const lines = []; - const { start, end } = this.selectedCommentPosition; + const { start, end } = this.lines; const diffLines = this.diffFile[INLINE_DIFF_LINES_KEY]; let isAdding = false; @@ -144,6 +149,13 @@ export default { return lines; }, }, + created() { + if (this.range) { + this.lines = { ...this.range }; + } else if (this.line) { + this.lines = { start: this.line, end: this.line }; + } + }, mounted() { if (this.isLoggedIn) { const keys = [ @@ -166,16 +178,16 @@ export default { 'saveDiffDiscussion', 'setSuggestPopoverDismissed', ]), - handleCancelCommentForm(shouldConfirm, isDirty) { + async handleCancelCommentForm(shouldConfirm, isDirty) { if (shouldConfirm && isDirty) { const msg = s__('Notes|Are you sure you want to cancel creating this comment?'); - // eslint-disable-next-line no-alert - if (!window.confirm(msg)) { + const confirmed = await confirmAction(msg); + + if (!confirmed) { return; } } - this.cancelCommentForm({ lineCode: this.line.line_code, fileHash: this.diffFileHash, @@ -189,6 +201,9 @@ export default { this.handleCancelCommentForm(), ); }, + updateStartLine(line) { + this.lines.start = line; + }, }, }; </script> @@ -199,7 +214,9 @@ export default { <multiline-comment-form v-model="commentLineStart" :line="line" + :line-range="lines" :comment-line-options="commentLineOptions" + @input="updateStartLine" /> </div> <note-form diff --git a/app/assets/javascripts/diffs/components/diff_view.vue b/app/assets/javascripts/diffs/components/diff_view.vue index 64ded1ca8ca..55c796182ee 100644 --- a/app/assets/javascripts/diffs/components/diff_view.vue +++ b/app/assets/javascripts/diffs/components/diff_view.vue @@ -6,6 +6,7 @@ import draftCommentsMixin from '~/diffs/mixins/draft_comments'; import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; import { hide } from '~/tooltips'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import { pickDirection } from '../utils/diff_line'; import DiffCommentCell from './diff_comment_cell.vue'; import DiffExpansionCell from './diff_expansion_cell.vue'; import DiffRow from './diff_row.vue'; @@ -106,6 +107,16 @@ export default { }); this.idState.dragStart = null; }, + singleLineComment(code, line) { + const lineDir = pickDirection({ line, code }); + + this.idState.updatedLineRange = { + start: lineDir, + end: lineDir, + }; + + this.showCommentForm({ lineCode: lineDir.line_code, fileHash: this.diffFile.file_hash }); + }, isHighlighted(line) { return isHighlighted( this.highlightedRow, @@ -169,7 +180,7 @@ export default { :index="index" :is-highlighted="isHighlighted(line)" :file-line-coverage="fileLineCoverage" - @showCommentForm="(lineCode) => showCommentForm({ lineCode, fileHash: diffFile.file_hash })" + @showCommentForm="(code) => singleLineComment(code, line)" @setHighlightedRow="setHighlightedRow" @toggleLineDiscussions=" ({ lineCode, expanded }) => @@ -193,6 +204,7 @@ export default { <diff-comment-cell v-if="line.left && (line.left.renderDiscussion || line.left.hasCommentForm)" :line="line.left" + :line-range="idState.updatedLineRange" :diff-file-hash="diffFile.file_hash" :help-page-path="helpPagePath" line-position="left" @@ -206,6 +218,7 @@ export default { <diff-comment-cell v-if="line.right && (line.right.renderDiscussion || line.right.hasCommentForm)" :line="line.right" + :line-range="idState.updatedLineRange" :diff-file-hash="diffFile.file_hash" :line-index="index" :help-page-path="helpPagePath" diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index 41d885d3dc1..85e4199d1c1 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -98,7 +98,7 @@ export default { :file-row-component="$options.DiffFileRow" :current-diff-file-id="currentDiffFileId" @toggleTreeOpen="toggleTreeOpen" - @clickFile="scrollToFile" + @clickFile="(path) => scrollToFile({ path })" /> </template> <p v-else class="prepend-top-20 append-bottom-20 text-center"> diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 1b1ab59b2b4..260ebdf2141 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -138,7 +138,7 @@ export default function initDiffsApp(store) { ...mapActions('diffs', ['toggleFileFinder', 'scrollToFile']), openFile(file) { window.mrTabs.tabShown('diffs'); - this.scrollToFile(file.path); + this.scrollToFile({ path: file.path }); }, }, render(createElement) { diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 5c94c6b803b..692cb913a57 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -85,6 +85,12 @@ export const setBaseConfig = ({ commit }, options) => { viewDiffsFileByFile, mrReviews, }); + + Array.from(new Set(Object.values(mrReviews).flat())).forEach((id) => { + const viewedId = id.replace(/^hash:/, ''); + + commit(types.SET_DIFF_FILE_VIEWED, { id: viewedId, seen: true }); + }); }; export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { @@ -127,7 +133,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { } if (!isNoteLink && !state.currentDiffFileId) { - commit(types.VIEW_DIFF_FILE, diff_files[0]?.file_hash); + commit(types.SET_CURRENT_DIFF_FILE, diff_files[0]?.file_hash); } if (isNoteLink) { @@ -143,7 +149,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { !state.diffFiles.some((f) => f.file_hash === state.currentDiffFileId) && !isNoteLink ) { - commit(types.VIEW_DIFF_FILE, state.diffFiles[0].file_hash); + commit(types.SET_CURRENT_DIFF_FILE, state.diffFiles[0].file_hash); } if (state.diffFiles?.length) { @@ -248,7 +254,7 @@ export const fetchCoverageFiles = ({ commit, state }) => { export const setHighlightedRow = ({ commit }, lineCode) => { const fileHash = lineCode.split('_')[0]; commit(types.SET_HIGHLIGHTED_ROW, lineCode); - commit(types.VIEW_DIFF_FILE, fileHash); + commit(types.SET_CURRENT_DIFF_FILE, fileHash); handleLocationHash(); }; @@ -514,23 +520,25 @@ export const toggleTreeOpen = ({ commit }, path) => { commit(types.TOGGLE_FOLDER_OPEN, path); }; -export const toggleActiveFileByHash = ({ commit }, hash) => { - commit(types.VIEW_DIFF_FILE, hash); +export const setCurrentFileHash = ({ commit }, hash) => { + commit(types.SET_CURRENT_DIFF_FILE, hash); }; -export const scrollToFile = ({ state, commit, getters }, path) => { +export const scrollToFile = ({ state, commit, getters }, { path, setHash = true }) => { if (!state.treeEntries[path]) return; const { fileHash } = state.treeEntries[path]; - commit(types.VIEW_DIFF_FILE, fileHash); + commit(types.SET_CURRENT_DIFF_FILE, fileHash); if (getters.isVirtualScrollingEnabled) { eventHub.$emit('scrollToFileHash', fileHash); - setTimeout(() => { - window.history.replaceState(null, null, `#${fileHash}`); - }); + if (setHash) { + setTimeout(() => { + window.history.replaceState(null, null, `#${fileHash}`); + }); + } } else { document.location.hash = fileHash; @@ -804,7 +812,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.VIEW_DIFF_FILE, fileHash); + commit(types.SET_CURRENT_DIFF_FILE, fileHash); } }; @@ -812,7 +820,7 @@ export const navigateToDiffFileIndex = ({ commit, state }, index) => { const fileHash = state.diffFiles[index].file_hash; document.location.hash = fileHash; - commit(types.VIEW_DIFF_FILE, fileHash); + commit(types.SET_CURRENT_DIFF_FILE, fileHash); }; export const setFileByFile = ({ state, commit }, { fileByFile }) => { @@ -848,6 +856,8 @@ export function reviewFile({ commit, state }, { file, reviewed = true }) { const reviews = markFileReview(state.mrReviews, file, reviewed); setReviewsForMergeRequest(mrPath, reviews); + + commit(types.SET_DIFF_FILE_VIEWED, { id: file.file_hash, seen: reviewed }); commit(types.SET_MR_FILE_REVIEWS, reviews); } diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 60836f747f5..51c21c1bfc4 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -20,7 +20,8 @@ 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 SET_SHOW_TREE_LIST = 'SET_SHOW_TREE_LIST'; -export const VIEW_DIFF_FILE = 'VIEW_DIFF_FILE'; +export const SET_CURRENT_DIFF_FILE = 'SET_CURRENT_DIFF_FILE'; +export const SET_DIFF_FILE_VIEWED = 'SET_DIFF_FILE_VIEWED'; 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 6bc927b9d1f..4a9df0eafcc 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -254,9 +254,11 @@ export default { [types.SET_SHOW_TREE_LIST](state, showTreeList) { state.showTreeList = showTreeList; }, - [types.VIEW_DIFF_FILE](state, fileId) { + [types.SET_CURRENT_DIFF_FILE](state, fileId) { state.currentDiffFileId = fileId; - Vue.set(state.viewedDiffFileIds, fileId, true); + }, + [types.SET_DIFF_FILE_VIEWED](state, { id, seen }) { + Vue.set(state.viewedDiffFileIds, id, seen); }, [types.OPEN_DIFF_FILE_COMMENT_FORM](state, formData) { state.commentForms.push({ diff --git a/app/assets/javascripts/diffs/utils/diff_line.js b/app/assets/javascripts/diffs/utils/diff_line.js new file mode 100644 index 00000000000..a248cc6318b --- /dev/null +++ b/app/assets/javascripts/diffs/utils/diff_line.js @@ -0,0 +1,10 @@ +export function pickDirection({ line, code } = {}) { + const { left, right } = line; + let direction = left || right; + + if (right?.line_code === code) { + direction = right; + } + + return direction; +} diff --git a/app/assets/javascripts/diffs/utils/discussions.js b/app/assets/javascripts/diffs/utils/discussions.js new file mode 100644 index 00000000000..c404705d209 --- /dev/null +++ b/app/assets/javascripts/diffs/utils/discussions.js @@ -0,0 +1,76 @@ +function normalize(processable) { + const { entry } = processable; + const offset = entry.rootBounds.bottom - entry.boundingClientRect.top; + const direction = + offset < 0 ? 'Up' : 'Down'; /* eslint-disable-line @gitlab/require-i18n-strings */ + + return { + ...processable, + entry: { + time: entry.time, + type: entry.isIntersecting ? 'intersection' : `scroll${direction}`, + }, + }; +} + +function sort({ entry: alpha }, { entry: beta }) { + const diff = alpha.time - beta.time; + let order = 0; + + if (diff < 0) { + order = -1; + } else if (diff > 0) { + order = 1; + } else if (alpha.type === 'intersection' && beta.type === 'scrollUp') { + order = 2; + } else if (alpha.type === 'scrollUp' && beta.type === 'intersection') { + order = -2; + } + + return order; +} + +function filter(entry) { + return entry.type !== 'scrollDown'; +} + +export function discussionIntersectionObserverHandlerFactory() { + let unprocessed = []; + let timer = null; + + return (processable) => { + unprocessed.push(processable); + + if (timer) { + clearTimeout(timer); + } + + timer = setTimeout(() => { + unprocessed + .map(normalize) + .filter(filter) + .sort(sort) + .forEach((discussionObservationContainer) => { + const { + entry: { type }, + currentDiscussion, + isFirstUnresolved, + isDiffsPage, + functions: { setCurrentDiscussionId, getPreviousUnresolvedDiscussionId }, + } = discussionObservationContainer; + + if (type === 'intersection') { + setCurrentDiscussionId(currentDiscussion.id); + } else if (type === 'scrollUp') { + setCurrentDiscussionId( + isFirstUnresolved + ? null + : getPreviousUnresolvedDiscussionId(currentDiscussion.id, isDiffsPage), + ); + } + }); + + unprocessed = []; + }, 0); + }; +} diff --git a/app/assets/javascripts/diffs/utils/file_reviews.js b/app/assets/javascripts/diffs/utils/file_reviews.js index 7a4b1aa6b17..227be4e4a6c 100644 --- a/app/assets/javascripts/diffs/utils/file_reviews.js +++ b/app/assets/javascripts/diffs/utils/file_reviews.js @@ -52,8 +52,10 @@ export function markFileReview(reviews, file, reviewed = true) { if (reviewed) { fileReviews.add(file.id); + fileReviews.add(`hash:${file.file_hash}`); } else { fileReviews.delete(file.id); + fileReviews.delete(`hash:${file.file_hash}`); } updatedReviews[file.file_identifier_hash] = Array.from(fileReviews); |