diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
17 files changed, 287 insertions, 133 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 941365d9d1d..1e524882d5f 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 } from '@gitlab/ui'; +import { GlLoadingIcon, GlButtonGroup, GlButton } from '@gitlab/ui'; import Mousetrap from 'mousetrap'; import { __ } from '~/locale'; import createFlash from '~/flash'; @@ -36,6 +36,8 @@ export default { TreeList, GlLoadingIcon, PanelResizer, + GlButtonGroup, + GlButton, }, mixins: [glFeatureFlagsMixin()], props: { @@ -94,6 +96,11 @@ export default { required: false, default: false, }, + viewDiffsFileByFile: { + type: Boolean, + required: false, + default: false, + }, }, data() { const treeWidth = @@ -120,9 +127,18 @@ export default { emailPatchPath: state => state.diffs.emailPatchPath, retrievingBatches: state => state.diffs.retrievingBatches, }), - ...mapState('diffs', ['showTreeList', 'isLoading', 'startVersion']), + ...mapState('diffs', ['showTreeList', 'isLoading', 'startVersion', 'currentDiffFileId']), ...mapGetters('diffs', ['isParallelView', 'currentDiffIndex']), ...mapGetters(['isNotesFetched', 'getNoteableData']), + diffs() { + if (!this.viewDiffsFileByFile) { + return this.diffFiles; + } + + return this.diffFiles.filter((file, i) => { + return file.file_hash === this.currentDiffFileId || (i === 0 && !this.currentDiffFileId); + }); + }, canCurrentUserFork() { return this.currentUser.can_fork === true && this.currentUser.can_create_merge_request; }, @@ -183,16 +199,22 @@ export default { dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, useSingleDiffStyle: this.glFeatures.singleMrDiffView, + viewDiffsFileByFile: this.viewDiffsFileByFile, }); if (this.shouldShow) { this.fetchData(); } - const id = window && window.location && window.location.hash; + const id = window?.location?.hash; - if (id) { - this.setHighlightedRow(id.slice(1)); + if (id && id.indexOf('#note') !== 0) { + this.setHighlightedRow( + id + .split('diff-content') + .pop() + .slice(1), + ); } }, created() { @@ -236,6 +258,7 @@ export default { 'cacheTreeListWidth', 'scrollToFile', 'toggleShowTreeList', + 'navigateToDiffFileIndex', ]), refetchDiffData() { this.fetchData(false); @@ -398,7 +421,7 @@ export default { class="files d-flex" > <div - v-show="showTreeList" + v-if="showTreeList" :style="{ width: `${treeWidth}px` }" class="diff-tree-list js-diff-tree-list mr-3" > @@ -422,12 +445,31 @@ export default { <div v-if="isBatchLoading" class="loading"><gl-loading-icon size="lg" /></div> <template v-else-if="renderDiffFiles"> <diff-file - v-for="file in diffFiles" + v-for="file in diffs" :key="file.newPath" :file="file" :help-page-path="helpPagePath" :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> </template> <no-changes v-else :changes-empty-state-illustration="changesEmptyStateIllustration" /> </div> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 54852b113ae..00d36c0b978 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -30,6 +30,10 @@ export default { required: false, default: '', }, + viewDiffsFileByFile: { + type: Boolean, + required: true, + }, }, data() { return { @@ -154,6 +158,7 @@ export default { :collapsible="true" :expanded="!isCollapsed" :add-merge-request-buttons="true" + :view-diffs-file-by-file="viewDiffsFileByFile" class="js-file-title file-title" @toggleFile="handleToggle" @showForkMessage="showForkMessage" diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 61bbf13aa53..5727fbaaf68 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -2,7 +2,6 @@ import { escape } from 'lodash'; import { mapActions, mapGetters } from 'vuex'; import { GlDeprecatedButton, GlTooltipDirective, GlLoadingIcon } from '@gitlab/ui'; -import { polyfillSticky } from '~/lib/utils/sticky'; 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'; @@ -55,6 +54,11 @@ export default { type: Boolean, required: true, }, + viewDiffsFileByFile: { + type: Boolean, + required: false, + default: false, + }, }, computed: { ...mapGetters('diffs', ['diffHasExpandedDiscussions', 'diffHasDiscussions']), @@ -124,9 +128,6 @@ export default { return s__('MRDiff|Show full file'); }, }, - mounted() { - polyfillSticky(this.$refs.header); - }, methods: { ...mapActions('diffs', [ 'toggleFileDiscussions', @@ -167,22 +168,17 @@ export default { :name="collapseIcon" :size="16" aria-hidden="true" - class="diff-toggle-caret append-right-5" + class="diff-toggle-caret gl-mr-2" @click.stop="handleToggleFile" /> <a - v-once ref="titleWrapper" - class="append-right-4" + :v-once="!viewDiffsFileByFile" + class="gl-mr-2" :href="titleLink" @click="handleFileNameClick" > - <file-icon - :file-name="filePath" - :size="18" - aria-hidden="true" - css-classes="append-right-5" - /> + <file-icon :file-name="filePath" :size="18" aria-hidden="true" css-classes="gl-mr-2" /> <span v-if="isFileRenamed"> <strong v-gl-tooltip @@ -218,7 +214,7 @@ export default { {{ diffFile.a_mode }} → {{ diffFile.b_mode }} </small> - <span v-if="isUsingLfs" class="label label-lfs append-right-5"> {{ __('LFS') }} </span> + <span v-if="isUsingLfs" class="label label-lfs gl-mr-2"> {{ __('LFS') }} </span> </div> <div diff --git a/app/assets/javascripts/diffs/components/diff_file_row.vue b/app/assets/javascripts/diffs/components/diff_file_row.vue index c8ba8d6040e..43b669625f4 100644 --- a/app/assets/javascripts/diffs/components/diff_file_row.vue +++ b/app/assets/javascripts/diffs/components/diff_file_row.vue @@ -23,6 +23,11 @@ export default { type: Boolean, required: true, }, + currentDiffFileId: { + type: String, + required: false, + default: null, + }, }, computed: { showFileRowStats() { @@ -33,7 +38,13 @@ export default { </script> <template> - <file-row :file="file" v-bind="$attrs" v-on="$listeners"> + <file-row + :file="file" + v-bind="$attrs" + :class="{ 'is-active': currentDiffFileId === file.fileHash }" + class="diff-file-row" + v-on="$listeners" + > <file-row-stats v-if="showFileRowStats" :file="file" class="mr-1" /> <changed-file-icon :file="file" :size="16" :show-tooltip="true" /> </file-row> 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 74305ee69bc..d2f49bd0020 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -8,7 +8,10 @@ import MultilineCommentForm from '../../notes/components/multiline_comment_form. import autosave from '../../notes/mixins/autosave'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import { DIFF_NOTE_TYPE } from '../constants'; -import { commentLineOptions } from '../../notes/components/multiline_comment_utils'; +import { + commentLineOptions, + formatLineRange, +} from '../../notes/components/multiline_comment_utils'; export default { components: { @@ -44,8 +47,10 @@ export default { data() { return { commentLineStart: { - lineCode: this.line.line_code, + line_code: this.line.line_code, type: this.line.type, + old_line: this.line.old_line, + new_line: this.line.new_line, }, }; }, @@ -74,19 +79,26 @@ export default { diffViewType: this.diffViewType, diffFile: this.diffFile, linePosition: this.linePosition, - lineRange: { - start_line_code: this.commentLineStart.lineCode, - start_line_type: this.commentLineStart.type, - end_line_code: this.line.line_code, - end_line_type: this.line.type, - }, + lineRange: formatLineRange(this.commentLineStart, this.line), }; }, diffFile() { return this.getDiffFileByHash(this.diffFileHash); }, commentLineOptions() { - return commentLineOptions(this.diffFile.highlighted_diff_lines, this.line.line_code); + const combineSides = (acc, { left, right }) => { + // ignore null values match lines + if (left && left.type !== 'match') acc.push(left); + // if the line_codes are identically, return to avoid duplicates + if (left?.line_code === right?.line_code) return acc; + if (right && right.type !== 'match') acc.push(right); + return acc; + }; + const side = this.line.type === 'new' ? 'right' : 'left'; + const lines = this.diffFile.highlighted_diff_lines.length + ? this.diffFile.highlighted_diff_lines + : this.diffFile.parallel_diff_lines.reduce(combineSides, []); + return commentLineOptions(lines, this.line, this.line.line_code, side); }, }, mounted() { @@ -136,10 +148,7 @@ export default { <template> <div class="content discussion-form discussion-form-container discussion-notes"> - <div - v-if="glFeatures.multilineComments" - class="gl-mb-3 gl-text-gray-700 gl-border-gray-200 gl-border-b-solid gl-border-b-1 gl-pb-3" - > + <div v-if="glFeatures.multilineComments" class="gl-mb-3 gl-text-gray-700 gl-pb-3"> <multiline-comment-form v-model="commentLineStart" :line="line" diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index 514d26862a3..198113e330a 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -4,15 +4,13 @@ import { GlIcon } from '@gitlab/ui'; import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; import { - MATCH_LINE_TYPE, CONTEXT_LINE_TYPE, LINE_POSITION_RIGHT, EMPTY_CELL_TYPE, - OLD_LINE_TYPE, OLD_NO_NEW_LINE_TYPE, + OLD_LINE_TYPE, NEW_NO_NEW_LINE_TYPE, LINE_HOVER_CLASS_NAME, - LINE_UNFOLD_CLASS_NAME, } from '../constants'; export default { @@ -29,10 +27,6 @@ export default { type: String, required: true, }, - contextLinesPath: { - type: String, - required: true, - }, isHighlighted: { type: Boolean, required: true, @@ -52,11 +46,6 @@ export default { required: false, default: '', }, - isContentLine: { - type: Boolean, - required: false, - default: false, - }, isBottom: { type: Boolean, required: false, @@ -68,6 +57,11 @@ export default { default: false, }, }, + data() { + return { + isCommentButtonRendered: false, + }; + }, computed: { ...mapGetters(['isLoggedIn']), lineCode() { @@ -81,13 +75,7 @@ export default { return `#${this.line.line_code || ''}`; }, shouldShowCommentButton() { - return ( - this.isHover && - !this.isMatchLine && - !this.isContextLine && - !this.isMetaLine && - !this.hasDiscussions - ); + return this.isHover && !this.isContextLine && !this.isMetaLine && !this.hasDiscussions; }, hasDiscussions() { return this.line.discussions && this.line.discussions.length > 0; @@ -99,6 +87,10 @@ export default { return this.showCommentButton && this.hasDiscussions; }, shouldRenderCommentButton() { + if (!this.isCommentButtonRendered) { + return false; + } + if (this.isLoggedIn && this.showCommentButton) { const isDiffHead = parseBoolean(getParameterByName('diff_head')); return !isDiffHead || gon.features?.mergeRefHeadComments; @@ -106,9 +98,6 @@ export default { return false; }, - isMatchLine() { - return this.line.type === MATCH_LINE_TYPE; - }, isContextLine() { return this.line.type === CONTEXT_LINE_TYPE; }, @@ -126,13 +115,8 @@ export default { type, { hll: this.isHighlighted, - [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine, [LINE_HOVER_CLASS_NAME]: - this.isLoggedIn && - this.isHover && - !this.isMatchLine && - !this.isContextLine && - !this.isMetaLine, + this.isLoggedIn && this.isHover && !this.isContextLine && !this.isMetaLine, }, ]; }, @@ -140,6 +124,17 @@ export default { return this.lineType === OLD_LINE_TYPE ? this.line.old_line : this.line.new_line; }, }, + mounted() { + this.unwatchShouldShowCommentButton = this.$watch('shouldShowCommentButton', newVal => { + if (newVal) { + this.isCommentButtonRendered = true; + this.unwatchShouldShowCommentButton(); + } + }); + }, + beforeDestroy() { + this.unwatchShouldShowCommentButton(); + }, methods: { ...mapActions('diffs', ['showCommentForm', 'setHighlightedRow', 'toggleLineDiscussions']), handleCommentButton() { @@ -151,34 +146,32 @@ export default { <template> <td ref="td" :class="classNameMap"> - <div> - <button - v-if="shouldRenderCommentButton" - v-show="shouldShowCommentButton" - ref="addDiffNoteButton" - type="button" - class="add-diff-note js-add-diff-note-button qa-diff-comment" - title="Add a comment to this line" - @click="handleCommentButton" - > - <gl-icon :size="12" name="comment" /> - </button> - <a - v-if="lineNumber" - ref="lineNumberRef" - :data-linenumber="lineNumber" - :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 }) - " - /> - </div> + <button + v-if="shouldRenderCommentButton" + v-show="shouldShowCommentButton" + ref="addDiffNoteButton" + type="button" + class="add-diff-note js-add-diff-note-button qa-diff-comment" + title="Add a comment to this line" + @click="handleCommentButton" + > + <gl-icon :size="12" name="comment" /> + </button> + <a + v-if="lineNumber" + ref="lineNumberRef" + :data-linenumber="lineNumber" + :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> </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 bd99fcb71b8..168e8c6c14e 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -28,10 +28,6 @@ export default { type: String, required: true, }, - contextLinesPath: { - type: String, - required: true, - }, line: { type: Object, required: true, @@ -41,6 +37,11 @@ export default { required: false, default: false, }, + isCommented: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { @@ -51,7 +52,10 @@ export default { ...mapGetters('diffs', ['fileLineCoverage']), ...mapState({ isHighlighted(state) { - return this.line.line_code !== null && this.line.line_code === state.diffs.highlightedRow; + if (this.isCommented) return true; + + const lineCode = this.line.line_code; + return lineCode ? lineCode === state.diffs.highlightedRow : false; }, }), isContextLine() { @@ -106,7 +110,6 @@ export default { > <diff-table-cell :file-hash="fileHash" - :context-lines-path="contextLinesPath" :line="line" :line-type="oldLineType" :is-bottom="isBottom" @@ -117,7 +120,6 @@ export default { /> <diff-table-cell :file-hash="fileHash" - :context-lines-path="contextLinesPath" :line="line" :line-type="newLineType" :is-bottom="isBottom" diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index ad72016f03b..e82d06ee385 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -1,10 +1,11 @@ <script> -import { mapGetters } from 'vuex'; +import { mapGetters, mapState } from 'vuex'; 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'; import inlineDiffCommentRow from './inline_diff_comment_row.vue'; import inlineDiffExpansionRow from './inline_diff_expansion_row.vue'; +import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; export default { components: { @@ -31,9 +32,19 @@ export default { }, computed: { ...mapGetters('diffs', ['commitId']), + ...mapState({ + selectedCommentPosition: ({ notes }) => notes.selectedCommentPosition, + selectedCommentPositionHover: ({ notes }) => notes.selectedCommentPositionHover, + }), diffLinesLength() { return this.diffLines.length; }, + commentedLines() { + return getCommentedLines( + this.selectedCommentPosition || this.selectedCommentPositionHover, + this.diffLines, + ); + }, }, userColorScheme: window.gon.user_color_scheme, }; @@ -65,9 +76,9 @@ export default { :key="`${line.line_code || index}`" :file-hash="diffFile.file_hash" :file-path="diffFile.file_path" - :context-lines-path="diffFile.context_lines_path" :line="line" :is-bottom="index + 1 === diffLinesLength" + :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" /> <inline-diff-comment-row :key="`icr-${line.line_code || index}`" diff --git a/app/assets/javascripts/diffs/components/no_changes.vue b/app/assets/javascripts/diffs/components/no_changes.vue index 94c2695a945..93afa978862 100644 --- a/app/assets/javascripts/diffs/components/no_changes.vue +++ b/app/assets/javascripts/diffs/components/no_changes.vue @@ -1,12 +1,12 @@ <script> import { mapGetters } from 'vuex'; import { escape } from 'lodash'; -import { GlDeprecatedButton } from '@gitlab/ui'; +import { GlButton } from '@gitlab/ui'; import { __, sprintf } from '~/locale'; export default { components: { - GlDeprecatedButton, + GlButton, }, props: { changesEmptyStateIllustration: { @@ -43,9 +43,9 @@ export default { <div class="text-content text-center"> <span v-html="emptyStateText"></span> <div class="text-center"> - <gl-deprecated-button :href="getNoteableData.new_blob_path" variant="success">{{ + <gl-button :href="getNoteableData.new_blob_path" variant="success" category="primary">{{ __('Create commit') - }}</gl-deprecated-button> + }}</gl-button> </div> </div> </div> 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 83d803f42b1..ccb32a2a745 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -31,10 +31,6 @@ export default { type: String, required: true, }, - contextLinesPath: { - type: String, - required: true, - }, line: { type: Object, required: true, @@ -44,6 +40,11 @@ export default { required: false, default: false, }, + isCommented: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { @@ -55,6 +56,8 @@ export default { ...mapGetters('diffs', ['fileLineCoverage']), ...mapState({ isHighlighted(state) { + if (this.isCommented) return true; + const lineCode = (this.line.left && this.line.left.line_code) || (this.line.right && this.line.right.line_code); @@ -144,7 +147,6 @@ export default { <template v-if="line.left && !isMatchLineLeft"> <diff-table-cell :file-hash="fileHash" - :context-lines-path="contextLinesPath" :line="line.left" :line-type="oldLineType" :is-bottom="isBottom" @@ -172,7 +174,6 @@ export default { <template v-if="line.right && !isMatchLineRight"> <diff-table-cell :file-hash="fileHash" - :context-lines-path="contextLinesPath" :line="line.right" :line-type="newLineType" :is-bottom="isBottom" diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index b5fcc50ce26..46a691ad22d 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -1,10 +1,11 @@ <script> -import { mapGetters } from 'vuex'; +import { mapGetters, mapState } from 'vuex'; import draftCommentsMixin from '~/diffs/mixins/draft_comments'; import ParallelDraftCommentRow from '~/batch_comments/components/parallel_draft_comment_row.vue'; import parallelDiffTableRow from './parallel_diff_table_row.vue'; import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; import parallelDiffExpansionRow from './parallel_diff_expansion_row.vue'; +import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; export default { components: { @@ -31,9 +32,19 @@ export default { }, computed: { ...mapGetters('diffs', ['commitId']), + ...mapState({ + selectedCommentPosition: ({ notes }) => notes.selectedCommentPosition, + selectedCommentPositionHover: ({ notes }) => notes.selectedCommentPositionHover, + }), diffLinesLength() { return this.diffLines.length; }, + commentedLines() { + return getCommentedLines( + this.selectedCommentPosition || this.selectedCommentPositionHover, + this.diffLines, + ); + }, }, userColorScheme: window.gon.user_color_scheme, }; @@ -67,9 +78,9 @@ export default { :key="line.line_code" :file-hash="diffFile.file_hash" :file-path="diffFile.file_path" - :context-lines-path="diffFile.context_lines_path" :line="line" :is-bottom="index + 1 === diffLinesLength" + :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" /> <parallel-diff-comment-row :key="`dcr-${line.line_code || index}`" diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index 52611f3c82a..38fbd8e61d4 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -26,7 +26,7 @@ export default { }; }, computed: { - ...mapState('diffs', ['tree', 'renderTreeList']), + ...mapState('diffs', ['tree', 'renderTreeList', 'currentDiffFileId']), ...mapGetters('diffs', ['allBlobs']), filteredTreeList() { const search = this.search.toLowerCase().trim(); @@ -96,6 +96,7 @@ export default { :level="0" :hide-file-stats="hideFileStats" :file-row-component="$options.DiffFileRow" + :current-diff-file-id="currentDiffFileId" @toggleTreeOpen="toggleTreeOpen" @clickFile="scrollToFile" /> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 9269dacd582..e3dd882f3dc 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -1,3 +1,7 @@ +// The backend actually uses "hide_whitespace" while the frontend +// uses "show whitspace" so these values are opposite what you might expect +export const NO_SHOW_WHITESPACE = '1'; +export const SHOW_WHITESPACE = '0'; export const INLINE_DIFF_VIEW_TYPE = 'inline'; export const PARALLEL_DIFF_VIEW_TYPE = 'parallel'; export const MATCH_LINE_TYPE = 'match'; @@ -20,6 +24,7 @@ export const LINE_SIDE_LEFT = 'left-side'; export const LINE_SIDE_RIGHT = 'right-side'; export const DIFF_VIEW_COOKIE_NAME = 'diff_view'; +export const DIFF_WHITESPACE_COOKIE_NAME = 'diff_whitespace'; export const LINE_HOVER_CLASS_NAME = 'is-over'; export const LINE_UNFOLD_CLASS_NAME = 'unfold js-unfold'; export const CONTEXT_LINE_CLASS_NAME = 'diff-expanded'; @@ -35,7 +40,6 @@ export const MR_TREE_SHOW_KEY = 'mr_tree_show'; export const TREE_TYPE = 'tree'; export const TREE_LIST_STORAGE_KEY = 'mr_diff_tree_list'; -export const WHITESPACE_STORAGE_KEY = 'mr_show_whitespace'; export const TREE_LIST_WIDTH_STORAGE_KEY = 'mr_tree_list_width'; export const INITIAL_TREE_WIDTH = 320; diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index ce48e36bfd7..76ff67ab861 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -1,11 +1,11 @@ import Vue from 'vue'; import { mapActions, mapState, mapGetters } from 'vuex'; import { parseBoolean } from '~/lib/utils/common_utils'; -import { getParameterValues } from '~/lib/utils/url_utility'; import FindFile from '~/vue_shared/components/file_finder/index.vue'; import eventHub from '../notes/event_hub'; import diffsApp from './components/app.vue'; -import { TREE_LIST_STORAGE_KEY } from './constants'; +import { TREE_LIST_STORAGE_KEY, DIFF_WHITESPACE_COOKIE_NAME } from './constants'; +import Cookies from 'js-cookie'; export default function initDiffsApp(store) { const fileFinderEl = document.getElementById('js-diff-file-finder'); @@ -78,6 +78,7 @@ export default function initDiffsApp(store) { dismissEndpoint: dataset.dismissEndpoint, showSuggestPopover: parseBoolean(dataset.showSuggestPopover), showWhitespaceDefault: parseBoolean(dataset.showWhitespaceDefault), + viewDiffsFileByFile: parseBoolean(dataset.fileByFileDefault), }; }, computed: { @@ -86,15 +87,16 @@ export default function initDiffsApp(store) { }), }, created() { - let hideWhitespace = getParameterValues('w')[0]; const treeListStored = localStorage.getItem(TREE_LIST_STORAGE_KEY); const renderTreeList = treeListStored !== null ? parseBoolean(treeListStored) : true; this.setRenderTreeList(renderTreeList); - if (!hideWhitespace) { - hideWhitespace = this.showWhitespaceDefault ? '0' : '1'; + + // Set whitespace default as per user preferences unless cookie is already set + if (!Cookies.get(DIFF_WHITESPACE_COOKIE_NAME)) { + const hideWhitespace = this.showWhitespaceDefault ? '0' : '1'; + this.setShowWhitespace({ showWhitespace: hideWhitespace !== '1' }); } - this.setShowWhitespace({ showWhitespace: hideWhitespace !== '1' }); }, methods: { ...mapActions('diffs', ['setRenderTreeList', 'setShowWhitespace']), @@ -114,7 +116,7 @@ export default function initDiffsApp(store) { isFluidLayout: this.isFluidLayout, dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, - showWhitespaceDefault: this.showWhitespaceDefault, + viewDiffsFileByFile: this.viewDiffsFileByFile, }, }); }, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index a8d348e1836..d469ed8ee82 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -25,7 +25,6 @@ import { DIFF_VIEW_COOKIE_NAME, MR_TREE_SHOW_KEY, TREE_LIST_STORAGE_KEY, - WHITESPACE_STORAGE_KEY, TREE_LIST_WIDTH_STORAGE_KEY, OLD_LINE_KEY, NEW_LINE_KEY, @@ -38,6 +37,9 @@ import { INLINE_DIFF_LINES_KEY, PARALLEL_DIFF_LINES_KEY, DIFFS_PER_PAGE, + DIFF_WHITESPACE_COOKIE_NAME, + SHOW_WHITESPACE, + NO_SHOW_WHITESPACE, } from '../constants'; import { diffViewerModes } from '~/ide/constants'; @@ -103,7 +105,9 @@ export const fetchDiffFiles = ({ state, commit }) => { .catch(() => worker.terminate()); }; -export const fetchDiffFilesBatch = ({ commit, state }) => { +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', @@ -123,16 +127,36 @@ export const fetchDiffFilesBatch = ({ commit, state }) => { commit(types.SET_DIFF_DATA_BATCH, { diff_files }); commit(types.SET_BATCH_LOADING, false); + if (!isNoteLink && !state.currentDiffFileId) { + commit(types.UPDATE_CURRENT_DIFF_FILE_ID, diff_files[0].file_hash); + } + + if (isNoteLink) { + dispatch('setCurrentDiffFileIdFromNote', id.split('_').pop()); + } + if (!pagination.next_page) { commit(types.SET_RETRIEVING_BATCHES, false); + + // We need to check that the currentDiffFileId points to a file that exists + if ( + state.currentDiffFileId && + !state.diffFiles.some(f => f.file_hash === state.currentDiffFileId) && + !isNoteLink + ) { + commit(types.UPDATE_CURRENT_DIFF_FILE_ID, state.diffFiles[0].file_hash); + } + if (gon.features?.codeNavigation) { // eslint-disable-next-line promise/catch-or-return,promise/no-nesting import('~/code_navigation').then(m => m.default({ - blobs: state.diffFiles.map(f => ({ - path: f.new_path, - codeNavigationPath: f.code_navigation_path, - })), + blobs: state.diffFiles + .filter(f => f.code_navigation_path) + .map(f => ({ + path: f.new_path, + codeNavigationPath: f.code_navigation_path, + })), definitionPathPrefix: state.definitionPathPrefix, }), ); @@ -211,9 +235,11 @@ export const setHighlightedRow = ({ commit }, lineCode) => { // This is adding line discussions to the actual lines in the diff tree // once for parallel and once for inline mode export const assignDiscussionsToDiff = ( - { commit, state, rootState }, + { commit, state, rootState, dispatch }, discussions = rootState.notes.discussions, ) => { + const id = window?.location?.hash; + const isNoteLink = id.indexOf('#note') === 0; const diffPositionByLineCode = getDiffPositionByLineCode( state.diffFiles, state.useSingleDiffStyle, @@ -230,6 +256,10 @@ export const assignDiscussionsToDiff = ( }); }); + if (isNoteLink) { + dispatch('setCurrentDiffFileIdFromNote', id.split('_').pop()); + } + Vue.nextTick(() => { eventHub.$emit('scrollToDiscussion'); }); @@ -448,6 +478,8 @@ export const toggleTreeOpen = ({ commit }, path) => { }; export const scrollToFile = ({ state, commit }, path) => { + if (!state.treeEntries[path]) return; + const { fileHash } = state.treeEntries[path]; document.location.hash = fileHash; @@ -484,11 +516,12 @@ export const setRenderTreeList = ({ commit }, renderTreeList) => { export const setShowWhitespace = ({ commit }, { showWhitespace, pushState = false }) => { commit(types.SET_SHOW_WHITESPACE, showWhitespace); + const w = showWhitespace ? SHOW_WHITESPACE : NO_SHOW_WHITESPACE; - localStorage.setItem(WHITESPACE_STORAGE_KEY, showWhitespace); + Cookies.set(DIFF_WHITESPACE_COOKIE_NAME, w); if (pushState) { - historyPushState(mergeUrlParams({ w: showWhitespace ? '0' : '1' }, window.location.href)); + historyPushState(mergeUrlParams({ w }, window.location.href)); } eventHub.$emit('refetchDiffData'); @@ -710,5 +743,22 @@ export function moveToNeighboringCommit({ dispatch, state }, { direction }) { } } +export const setCurrentDiffFileIdFromNote = ({ commit, rootGetters }, noteId) => { + const note = rootGetters.notesById[noteId]; + + if (!note) return; + + const fileHash = rootGetters.getDiscussion(note.discussion_id).diff_file.file_hash; + + commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash); +}; + +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); +}; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index 87938ababed..1f165dd4971 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -1,10 +1,17 @@ import Cookies from 'js-cookie'; import { getParameterValues } from '~/lib/utils/url_utility'; -import { INLINE_DIFF_VIEW_TYPE, DIFF_VIEW_COOKIE_NAME } from '../../constants'; +import { + INLINE_DIFF_VIEW_TYPE, + DIFF_VIEW_COOKIE_NAME, + DIFF_WHITESPACE_COOKIE_NAME, +} from '../../constants'; +import { getDefaultWhitespace } from '../utils'; const viewTypeFromQueryString = getParameterValues('view')[0]; const viewTypeFromCookie = Cookies.get(DIFF_VIEW_COOKIE_NAME); const defaultViewType = INLINE_DIFF_VIEW_TYPE; +const whiteSpaceFromQueryString = getParameterValues('w')[0]; +const whiteSpaceFromCookie = Cookies.get(DIFF_WHITESPACE_COOKIE_NAME); export default () => ({ isLoading: true, @@ -29,7 +36,7 @@ export default () => ({ commentForms: [], highlightedRow: null, renderTreeList: true, - showWhitespace: true, + showWhitespace: getDefaultWhitespace(whiteSpaceFromQueryString, whiteSpaceFromCookie), fileFinderVisible: false, dismissEndpoint: '', showSuggestPopover: true, diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index d261be1b550..bc85dd0a1d4 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -15,6 +15,8 @@ import { TREE_TYPE, INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE, + SHOW_WHITESPACE, + NO_SHOW_WHITESPACE, } from '../constants'; export function findDiffFile(files, match, matchKey = 'file_hash') { @@ -701,3 +703,10 @@ export const allDiscussionWrappersExpanded = diff => { return discussionsExpanded; }; + +export const getDefaultWhitespace = (queryString, cookie) => { + // Querystring should override stored cookie value + if (queryString) return queryString === SHOW_WHITESPACE; + if (cookie === NO_SHOW_WHITESPACE) return false; + return true; +}; |