diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
-rw-r--r-- | app/assets/javascripts/diffs/components/app.vue | 20 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/compare_versions.vue | 12 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/diff_file.vue | 4 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/tree_list.vue | 116 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/store/actions.js | 24 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/store/mutations.js | 94 |
6 files changed, 178 insertions, 92 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index edca45f22f9..59680959bb1 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -41,6 +41,11 @@ export default { required: true, }, }, + data() { + return { + assignedDiscussions: false, + }; + }, computed: { ...mapState({ isLoading: state => state.diffs.isLoading, @@ -58,9 +63,9 @@ export default { plainDiffPath: state => state.diffs.plainDiffPath, emailPatchPath: state => state.diffs.emailPatchPath, }), - ...mapState('diffs', ['showTreeList']), + ...mapState('diffs', ['showTreeList', 'isLoading']), ...mapGetters('diffs', ['isParallelView']), - ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), + ...mapGetters(['isNotesFetched', 'getNoteableData']), targetBranch() { return { branchName: this.targetBranchName, @@ -147,13 +152,10 @@ export default { } }, setDiscussions() { - if (this.isNotesFetched) { - requestIdleCallback( - () => { - this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); - }, - { timeout: 1000 }, - ); + if (this.isNotesFetched && !this.assignedDiscussions && !this.isLoading) { + this.assignedDiscussions = true; + + requestIdleCallback(() => this.assignDiscussionsToDiff(), { timeout: 1000 }); } }, adjustView() { diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index 9bbf62c0eb6..29b5aff0fb1 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -40,17 +40,14 @@ export default { comparableDiffs() { return this.mergeRequestDiffs.slice(1); }, - isWhitespaceVisible() { - return !getParameterValues('w')[0]; - }, toggleWhitespaceText() { - if (this.isWhitespaceVisible) { + if (this.isWhitespaceVisible()) { return __('Hide whitespace changes'); } return __('Show whitespace changes'); }, toggleWhitespacePath() { - if (this.isWhitespaceVisible) { + if (this.isWhitespaceVisible()) { return mergeUrlParams({ w: 1 }, window.location.href); } @@ -67,6 +64,9 @@ export default { 'expandAllFiles', 'toggleShowTreeList', ]), + isWhitespaceVisible() { + return getParameterValues('w')[0] !== '1'; + }, }, }; </script> @@ -121,7 +121,7 @@ export default { </a> <a :href="toggleWhitespacePath" - class="btn btn-default" + class="btn btn-default qa-toggle-whitespace" > {{ toggleWhitespaceText }} </a> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index f72c7a84e5c..958e57c5652 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -29,7 +29,7 @@ export default { }, computed: { ...mapState('diffs', ['currentDiffFileId']), - ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), + ...mapGetters(['isNotesFetched']), isCollapsed() { return this.file.collapsed || false; }, @@ -79,7 +79,7 @@ export default { .then(() => { requestIdleCallback( () => { - this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); + this.assignDiscussionsToDiff(); }, { timeout: 1000 }, ); diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index cfe4273742f..91052b303a6 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -1,17 +1,30 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; +import { GlTooltipDirective } from '@gitlab-org/gitlab-ui'; +import { convertPermissionToBoolean } from '~/lib/utils/common_utils'; import Icon from '~/vue_shared/components/icon.vue'; import FileRow from '~/vue_shared/components/file_row.vue'; import FileRowStats from './file_row_stats.vue'; +const treeListStorageKey = 'mr_diff_tree_list'; + export default { + directives: { + GlTooltip: GlTooltipDirective, + }, components: { Icon, FileRow, }, data() { + const treeListStored = localStorage.getItem(treeListStorageKey); + const renderTreeList = + treeListStored !== null ? convertPermissionToBoolean(treeListStored) : true; + return { search: '', + renderTreeList, + focusSearch: false, }; }, computed: { @@ -20,15 +33,35 @@ export default { filteredTreeList() { const search = this.search.toLowerCase().trim(); - if (search === '') return this.tree; + if (search === '') return this.renderTreeList ? this.tree : this.allBlobs; return this.allBlobs.filter(f => f.name.toLowerCase().indexOf(search) >= 0); }, + rowDisplayTextKey() { + if (this.renderTreeList && this.search.trim() === '') { + return 'name'; + } + + return 'path'; + }, }, methods: { ...mapActions('diffs', ['toggleTreeOpen', 'scrollToFile']), clearSearch() { this.search = ''; + this.toggleFocusSearch(false); + }, + toggleRenderTreeList(toggle) { + this.renderTreeList = toggle; + localStorage.setItem(treeListStorageKey, this.renderTreeList); + }, + toggleFocusSearch(toggle) { + this.focusSearch = toggle; + }, + blurSearch() { + if (this.search.trim() === '') { + this.toggleFocusSearch(false); + } }, }, FileRowStats, @@ -37,28 +70,67 @@ export default { <template> <div class="tree-list-holder d-flex flex-column"> - <div class="append-bottom-8 position-relative tree-list-search"> - <icon - name="search" - class="position-absolute tree-list-icon" - /> - <input - v-model="search" - :placeholder="s__('MergeRequest|Filter files')" - type="search" - class="form-control" - /> - <button - v-show="search" - :aria-label="__('Clear search')" - type="button" - class="position-absolute tree-list-icon tree-list-clear-icon border-0 p-0" - @click="clearSearch" - > + <div class="append-bottom-8 position-relative tree-list-search d-flex"> + <div class="flex-fill d-flex"> <icon - name="close" + name="search" + class="position-absolute tree-list-icon" + /> + <input + v-model="search" + :placeholder="s__('MergeRequest|Filter files')" + type="search" + class="form-control" + @focus="toggleFocusSearch(true)" + @blur="blurSearch" /> - </button> + <button + v-show="search" + :aria-label="__('Clear search')" + type="button" + class="position-absolute bg-transparent tree-list-icon tree-list-clear-icon border-0 p-0" + @click="clearSearch" + > + <icon + name="close" + /> + </button> + </div> + <div + v-show="!focusSearch" + class="btn-group prepend-left-8 tree-list-view-toggle" + > + <button + v-gl-tooltip.hover + :aria-label="__('List view')" + :title="__('List view')" + :class="{ + active: !renderTreeList + }" + class="btn btn-default pt-0 pb-0 d-flex align-items-center" + type="button" + @click="toggleRenderTreeList(false)" + > + <icon + name="hamburger" + /> + </button> + <button + v-gl-tooltip.hover + :aria-label="__('Tree view')" + :title="__('Tree view')" + :class="{ + active: renderTreeList + }" + class="btn btn-default pt-0 pb-0 d-flex align-items-center" + type="button" + @click="toggleRenderTreeList(true)" + > + <icon + name="file-tree" + /> + </button> + </div> </div> <div class="tree-list-scroll" @@ -72,6 +144,8 @@ export default { :hide-extra-on-tree="true" :extra-component="$options.FileRowStats" :show-changed-icon="true" + :display-text-key="rowDisplayTextKey" + :should-truncate-start="true" @toggleTreeOpen="toggleTreeOpen" @clickFile="scrollToFile" /> diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 1e0b27b538d..ca8ae605cb4 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -5,7 +5,6 @@ import createFlash from '~/flash'; import { s__ } from '~/locale'; import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils'; import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility'; -import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils'; import { getDiffPositionByLineCode, getNoteFormData } from './utils'; import * as types from './mutation_types'; import { @@ -36,18 +35,17 @@ export const fetchDiffFiles = ({ state, commit }) => { // This is adding line discussions to the actual lines in the diff tree // once for parallel and once for inline mode -export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => { +export const assignDiscussionsToDiff = ( + { commit, state, rootState }, + discussions = rootState.notes.discussions, +) => { const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles); - Object.values(allLineDiscussions).forEach(discussions => { - if (discussions.length > 0) { - const { fileHash } = discussions[0]; - commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { - fileHash, - discussions, - diffPositionByLineCode, - }); - } + discussions.filter(discussion => discussion.diff_discussion).forEach(discussion => { + commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { + discussion, + diffPositionByLineCode, + }); }); }; @@ -190,9 +188,7 @@ export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { return dispatch('saveNote', postData, { root: true }) .then(result => dispatch('updateDiscussion', result.discussion, { root: true })) - .then(discussion => - dispatch('assignDiscussionsToDiff', reduceDiscussionsToLineCodes([discussion])), - ) + .then(discussion => dispatch('assignDiscussionsToDiff', [discussion])) .catch(() => createFlash(s__('MergeRequests|Saving the comment failed'))); }; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 0b4485ecdb5..38a65f111a2 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -90,53 +90,67 @@ export default { })); }, - [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) { - const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); - const firstDiscussion = discussions[0]; - const isDiffDiscussion = firstDiscussion.diff_discussion; - const hasLineCode = firstDiscussion.line_code; - const diffPosition = diffPositionByLineCode[firstDiscussion.line_code]; - - if ( - selectedFile && - isDiffDiscussion && - hasLineCode && - diffPosition && + [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode }) { + const { latestDiff } = state; + + const discussionLineCode = discussion.line_code; + const fileHash = discussion.diff_file.file_hash; + const lineCheck = ({ lineCode }) => + lineCode === discussionLineCode && isDiscussionApplicableToLine({ - discussion: firstDiscussion, - diffPosition, - latestDiff: state.latestDiff, - }) - ) { - const targetLine = selectedFile.parallelDiffLines.find( - line => - (line.left && line.left.lineCode === firstDiscussion.line_code) || - (line.right && line.right.lineCode === firstDiscussion.line_code), - ); - if (targetLine) { - if (targetLine.left && targetLine.left.lineCode === firstDiscussion.line_code) { - Object.assign(targetLine.left, { - discussions, - }); - } else { - Object.assign(targetLine.right, { - discussions, + discussion, + diffPosition: diffPositionByLineCode[lineCode], + latestDiff, + }); + + state.diffFiles = state.diffFiles.map(diffFile => { + if (diffFile.fileHash === fileHash) { + const file = { ...diffFile }; + + if (file.highlightedDiffLines) { + file.highlightedDiffLines = file.highlightedDiffLines.map(line => { + if (lineCheck(line)) { + return { + ...line, + discussions: line.discussions.concat(discussion), + }; + } + + return line; }); } - } - - if (selectedFile.highlightedDiffLines) { - const targetInlineLine = selectedFile.highlightedDiffLines.find( - line => line.lineCode === firstDiscussion.line_code, - ); - if (targetInlineLine) { - Object.assign(targetInlineLine, { - discussions, + if (file.parallelDiffLines) { + file.parallelDiffLines = file.parallelDiffLines.map(line => { + const left = line.left && lineCheck(line.left); + const right = line.right && lineCheck(line.right); + + if (left || right) { + return { + left: { + ...line.left, + discussions: left ? line.left.discussions.concat(discussion) : [], + }, + right: { + ...line.right, + discussions: right && !left ? line.right.discussions.concat(discussion) : [], + }, + }; + } + + return line; }); } + + if (!file.parallelDiffLines || !file.highlightedDiffLines) { + file.discussions = file.discussions.concat(discussion); + } + + return file; } - } + + return diffFile; + }); }, [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) { |