diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
33 files changed, 1079 insertions, 761 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index b5b05df4d34..edca45f22f9 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -4,23 +4,23 @@ import Icon from '~/vue_shared/components/icon.vue'; import { __ } from '~/locale'; import createFlash from '~/flash'; import eventHub from '../../notes/event_hub'; -import LoadingIcon from '../../vue_shared/components/loading_icon.vue'; import CompareVersions from './compare_versions.vue'; -import ChangedFiles from './changed_files.vue'; import DiffFile from './diff_file.vue'; import NoChanges from './no_changes.vue'; import HiddenFilesWarning from './hidden_files_warning.vue'; +import CommitWidget from './commit_widget.vue'; +import TreeList from './tree_list.vue'; export default { name: 'DiffsApp', components: { Icon, - LoadingIcon, CompareVersions, - ChangedFiles, DiffFile, NoChanges, HiddenFilesWarning, + CommitWidget, + TreeList, }, props: { endpoint: { @@ -58,8 +58,9 @@ export default { plainDiffPath: state => state.diffs.plainDiffPath, emailPatchPath: state => state.diffs.emailPatchPath, }), + ...mapState('diffs', ['showTreeList']), ...mapGetters('diffs', ['isParallelView']), - ...mapGetters(['isNotesFetched']), + ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), targetBranch() { return { branchName: this.targetBranchName, @@ -88,6 +89,9 @@ export default { canCurrentUserFork() { return this.currentUser.canFork === true && this.currentUser.canCreateMergeRequest; }, + showCompareVersions() { + return this.mergeRequestDiffs && this.mergeRequestDiff; + }, }, watch: { diffViewType() { @@ -102,6 +106,8 @@ export default { this.adjustView(); }, + isLoading: 'adjustView', + showTreeList: 'adjustView', }, mounted() { this.setBaseConfig({ endpoint: this.endpoint, projectPath: this.projectPath }); @@ -112,13 +118,25 @@ export default { }, created() { this.adjustView(); + eventHub.$once('fetchedNotesData', this.setDiscussions); }, methods: { - ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles', 'startRenderDiffsQueue']), + ...mapActions('diffs', [ + 'setBaseConfig', + 'fetchDiffFiles', + 'startRenderDiffsQueue', + 'assignDiscussionsToDiff', + ]), fetchData() { this.fetchDiffFiles() .then(() => { - requestIdleCallback(this.startRenderDiffsQueue, { timeout: 1000 }); + requestIdleCallback( + () => { + this.setDiscussions(); + this.startRenderDiffsQueue(); + }, + { timeout: 1000 }, + ); }) .catch(() => { createFlash(__('Something went wrong on our end. Please try again!')); @@ -128,11 +146,22 @@ export default { eventHub.$emit('fetchNotesData'); } }, + setDiscussions() { + if (this.isNotesFetched) { + requestIdleCallback( + () => { + this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); + }, + { timeout: 1000 }, + ); + } + }, adjustView() { - if (this.shouldShow && this.isParallelView) { - window.mrTabs.expandViewContainer(); - } else { - window.mrTabs.resetViewContainer(); + if (this.shouldShow) { + this.$nextTick(() => { + window.mrTabs.resetViewContainer(); + window.mrTabs.expandViewContainer(this.showTreeList); + }); } }, }, @@ -145,7 +174,7 @@ export default { v-if="isLoading" class="loading" > - <loading-icon /> + <gl-loading-icon /> </div> <div v-else @@ -154,7 +183,7 @@ export default { class="diffs tab-pane" > <compare-versions - v-if="!commit && mergeRequestDiffs.length > 1" + v-if="showCompareVersions" :merge-request-diffs="mergeRequestDiffs" :merge-request-diff="mergeRequestDiff" :start-version="startVersion" @@ -187,22 +216,31 @@ export default { </div> </div> - <changed-files - :diff-files="diffFiles" + <commit-widget + v-if="commit" + :commit="commit" /> - <div - v-if="diffFiles.length > 0" - class="files" - > - <diff-file - v-for="file in diffFiles" - :key="file.newPath" - :file="file" - :can-current-user-fork="canCurrentUserFork" - /> + <div class="files d-flex prepend-top-default"> + <div + v-show="showTreeList" + class="diff-tree-list" + > + <tree-list /> + </div> + <div + v-if="diffFiles.length > 0" + class="diff-files-holder" + > + <diff-file + v-for="file in diffFiles" + :key="file.newPath" + :file="file" + :can-current-user-fork="canCurrentUserFork" + /> + </div> + <no-changes v-else /> </div> - <no-changes v-else /> </div> </div> </template> diff --git a/app/assets/javascripts/diffs/components/changed_files.vue b/app/assets/javascripts/diffs/components/changed_files.vue deleted file mode 100644 index 97751db1254..00000000000 --- a/app/assets/javascripts/diffs/components/changed_files.vue +++ /dev/null @@ -1,171 +0,0 @@ -<script> -import { mapGetters, mapActions } from 'vuex'; -import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; -import Icon from '~/vue_shared/components/icon.vue'; -import { pluralize } from '~/lib/utils/text_utility'; -import { getParameterValues, mergeUrlParams } from '~/lib/utils/url_utility'; -import { contentTop } from '~/lib/utils/common_utils'; -import { __ } from '~/locale'; -import ChangedFilesDropdown from './changed_files_dropdown.vue'; -import changedFilesMixin from '../mixins/changed_files'; - -export default { - components: { - Icon, - ChangedFilesDropdown, - ClipboardButton, - }, - mixins: [changedFilesMixin], - data() { - return { - isStuck: false, - maxWidth: 'auto', - offsetTop: 0, - }; - }, - computed: { - ...mapGetters('diffs', ['isInlineView', 'isParallelView', 'areAllFilesCollapsed']), - sumAddedLines() { - return this.sumValues('addedLines'); - }, - sumRemovedLines() { - return this.sumValues('removedLines'); - }, - whitespaceVisible() { - return !getParameterValues('w')[0]; - }, - toggleWhitespaceText() { - if (this.whitespaceVisible) { - return __('Hide whitespace changes'); - } - return __('Show whitespace changes'); - }, - toggleWhitespacePath() { - if (this.whitespaceVisible) { - return mergeUrlParams({ w: 1 }, window.location.href); - } - - return mergeUrlParams({ w: 0 }, window.location.href); - }, - top() { - return `${this.offsetTop}px`; - }, - }, - created() { - document.addEventListener('scroll', this.handleScroll); - this.offsetTop = contentTop(); - }, - beforeDestroy() { - document.removeEventListener('scroll', this.handleScroll); - }, - methods: { - ...mapActions('diffs', ['setInlineDiffViewType', 'setParallelDiffViewType', 'expandAllFiles']), - pluralize, - handleScroll() { - if (!this.updating) { - this.$nextTick(this.updateIsStuck); - this.updating = true; - } - }, - updateIsStuck() { - if (!this.$refs.wrapper) { - return; - } - - const scrollPosition = window.scrollY; - - this.isStuck = scrollPosition + this.offsetTop >= this.$refs.placeholder.offsetTop; - this.updating = false; - }, - sumValues(key) { - return this.diffFiles.reduce((total, file) => total + file[key], 0); - }, - }, -}; -</script> - -<template> - <span> - <div ref="placeholder"></div> - <div - ref="wrapper" - :style="{ top }" - :class="{'is-stuck': isStuck}" - class="content-block oneline-block diff-files-changed diff-files-changed-merge-request - files-changed js-diff-files-changed" - > - <div class="files-changed-inner"> - <div - class="inline-parallel-buttons d-none d-md-block" - > - <a - v-if="areAllFilesCollapsed" - class="btn btn-default" - @click="expandAllFiles" - > - {{ __('Expand all') }} - </a> - <a - :href="toggleWhitespacePath" - class="btn btn-default" - > - {{ toggleWhitespaceText }} - </a> - <div class="btn-group"> - <button - id="inline-diff-btn" - :class="{ active: isInlineView }" - type="button" - class="btn js-inline-diff-button" - data-view-type="inline" - @click="setInlineDiffViewType" - > - {{ __('Inline') }} - </button> - <button - id="parallel-diff-btn" - :class="{ active: isParallelView }" - type="button" - class="btn js-parallel-diff-button" - data-view-type="parallel" - @click="setParallelDiffViewType" - > - {{ __('Side-by-side') }} - </button> - </div> - </div> - - <div class="commit-stat-summary dropdown"> - <changed-files-dropdown - :diff-files="diffFiles" - /> - - <span - class="js-diff-stats-additions-deletions-expanded - diff-stats-additions-deletions-expanded" - > - with - <strong class="cgreen"> - {{ pluralize(`${sumAddedLines} addition`, sumAddedLines) }} - </strong> - and - <strong class="cred"> - {{ pluralize(`${sumRemovedLines} deletion`, sumRemovedLines) }} - </strong> - </span> - <div - class="js-diff-stats-additions-deletions-collapsed - diff-stats-additions-deletions-collapsed float-right d-sm-none" - > - <strong class="cgreen"> - +{{ sumAddedLines }} - </strong> - <strong class="cred"> - -{{ sumRemovedLines }} - </strong> - </div> - </div> - </div> - </div> - </span> -</template> diff --git a/app/assets/javascripts/diffs/components/changed_files_dropdown.vue b/app/assets/javascripts/diffs/components/changed_files_dropdown.vue deleted file mode 100644 index 045688a32bf..00000000000 --- a/app/assets/javascripts/diffs/components/changed_files_dropdown.vue +++ /dev/null @@ -1,126 +0,0 @@ -<script> -import Icon from '~/vue_shared/components/icon.vue'; -import changedFilesMixin from '../mixins/changed_files'; - -export default { - components: { - Icon, - }, - mixins: [changedFilesMixin], - data() { - return { - searchText: '', - }; - }, - computed: { - filteredDiffFiles() { - return this.diffFiles.filter(file => - file.filePath.toLowerCase().includes(this.searchText.toLowerCase()), - ); - }, - }, - methods: { - clearSearch() { - this.searchText = ''; - }, - }, -}; -</script> - -<template> - <span> - Showing - <button - class="diff-stats-summary-toggler" - data-toggle="dropdown" - type="button" - aria-expanded="false" - > - <span> - {{ n__('%d changed file', '%d changed files', diffFiles.length) }} - </span> - <icon - class="caret-icon" - name="chevron-down" - /> - </button> - <div class="dropdown-menu diff-file-changes"> - <div class="dropdown-input"> - <input - v-model="searchText" - type="search" - class="dropdown-input-field" - placeholder="Search files" - autocomplete="off" - /> - <i - v-if="searchText.length === 0" - aria-hidden="true" - data-hidden="true" - class="fa fa-search dropdown-input-search"> - </i> - <i - v-else - role="button" - class="fa fa-times dropdown-input-search" - @click="clearSearch" - ></i> - </div> - <div class="dropdown-content"> - <ul> - <li - v-for="diffFile in filteredDiffFiles" - :key="diffFile.name" - > - <a - :href="`#${diffFile.fileHash}`" - :title="diffFile.newPath" - class="diff-changed-file" - > - <icon - :name="fileChangedIcon(diffFile)" - :size="16" - :class="fileChangedClass(diffFile)" - class="diff-file-changed-icon append-right-8" - /> - <span class="diff-changed-file-content append-right-8"> - <strong - v-if="diffFile.blob && diffFile.blob.name" - class="diff-changed-file-name" - > - {{ diffFile.blob.name }} - </strong> - <strong - v-else - class="diff-changed-blank-file-name" - > - {{ s__('Diffs|No file name available') }} - </strong> - <span class="diff-changed-file-path prepend-top-5"> - {{ truncatedDiffPath(diffFile.blob.path) }} - </span> - </span> - <span class="diff-changed-stats"> - <span class="cgreen"> - +{{ diffFile.addedLines }} - </span> - <span class="cred"> - -{{ diffFile.removedLines }} - </span> - </span> - </a> - </li> - - <li - v-show="filteredDiffFiles.length === 0" - class="dropdown-menu-empty-item" - > - <a> - {{ __('No files found') }} - </a> - </li> - </ul> - </div> - </div> - </span> -</template> diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue new file mode 100644 index 00000000000..993206b2e73 --- /dev/null +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -0,0 +1,129 @@ +<script> +import tooltip from '~/vue_shared/directives/tooltip'; +import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; +import Icon from '~/vue_shared/components/icon.vue'; +import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; +import CIIcon from '~/vue_shared/components/ci_icon.vue'; +import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; +import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; + +/** + * CommitItem + * + * ----------------------------------------------------------------- + * WARNING: Please keep changes up-to-date with the following files: + * - `views/projects/commits/_commit.html.haml` + * ----------------------------------------------------------------- + * + * This Component was cloned from a HAML view. For the time being they + * coexist, but there is an issue to remove the duplication. + * https://gitlab.com/gitlab-org/gitlab-ce/issues/51613 + * + */ +export default { + directives: { + tooltip, + }, + components: { + UserAvatarLink, + Icon, + ClipboardButton, + CIIcon, + TimeAgoTooltip, + CommitPipelineStatus, + }, + props: { + commit: { + type: Object, + required: true, + }, + }, + computed: { + authorName() { + return (this.commit.author && this.commit.author.name) || this.commit.authorName; + }, + authorUrl() { + return (this.commit.author && this.commit.author.webUrl) || `mailto:${this.commit.authorEmail}`; + }, + authorAvatar() { + return (this.commit.author && this.commit.author.avatarUrl) || this.commit.authorGravatarUrl; + }, + }, +}; +</script> + +<template> + <li class="commit flex-row js-toggle-container"> + <user-avatar-link + :link-href="authorUrl" + :img-src="authorAvatar" + :img-alt="authorName" + :img-size="36" + class="avatar-cell d-none d-sm-block" + /> + <div class="commit-detail flex-list"> + <div class="commit-content qa-commit-content"> + <a + :href="commit.commitUrl" + class="commit-row-message item-title" + v-html="commit.titleHtml" + ></a> + + <span class="commit-row-message d-block d-sm-none"> + · + {{ commit.shortId }} + </span> + + <button + v-if="commit.descriptionHtml" + class="text-expander js-toggle-button" + type="button" + :aria-label="__('Toggle commit description')" + > + <icon + :size="12" + name="ellipsis_h" + /> + </button> + + <div class="commiter"> + <a + :href="authorUrl" + v-text="authorName" + ></a> + {{ s__('CommitWidget|authored') }} + <time-ago-tooltip + :time="commit.authoredDate" + /> + </div> + + <pre + v-if="commit.descriptionHtml" + class="commit-row-description js-toggle-content append-bottom-8" + v-html="commit.descriptionHtml" + ></pre> + </div> + <div class="commit-actions flex-row d-none d-sm-flex"> + <div + v-if="commit.signatureHtml" + v-html="commit.signatureHtml" + ></div> + <commit-pipeline-status + v-if="commit.pipelineStatusPath" + :endpoint="commit.pipelineStatusPath" + /> + <div class="commit-sha-group"> + <div + class="label label-monospace" + v-text="commit.shortId" + ></div> + <clipboard-button + :text="commit.id" + :title="__('Copy commit SHA to clipboard')" + class="btn btn-default" + /> + </div> + </div> + </div> + </li> +</template> diff --git a/app/assets/javascripts/diffs/components/commit_widget.vue b/app/assets/javascripts/diffs/components/commit_widget.vue new file mode 100644 index 00000000000..cc8e72eb1c8 --- /dev/null +++ b/app/assets/javascripts/diffs/components/commit_widget.vue @@ -0,0 +1,40 @@ +<script> +import CommitItem from './commit_item.vue'; + +/** + * CommitWidget + * + * ----------------------------------------------------------------- + * WARNING: Please keep changes up-to-date with the following files: + * - `views/projects/merge_requests/diffs/_commit_widget.html.haml` + * ----------------------------------------------------------------- + * + * This Component was cloned from a HAML view. For the time being, + * they coexist, but there is an issue to remove the duplication. + * https://gitlab.com/gitlab-org/gitlab-ce/issues/51613 + * + */ +export default { + components: { + CommitItem, + }, + props: { + commit: { + type: Object, + required: true, + }, + }, +}; +</script> + +<template> + <div class="info-well prepend-top-default"> + <div class="well-segment"> + <ul class="blob-commit-info"> + <commit-item + :commit="commit" + /> + </ul> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index 1c9ad8e77f1..9bbf62c0eb6 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -1,9 +1,18 @@ <script> +import { mapActions, mapGetters, mapState } from 'vuex'; +import Tooltip from '@gitlab-org/gitlab-ui/dist/directives/tooltip'; +import { __ } from '~/locale'; +import { getParameterValues, mergeUrlParams } from '~/lib/utils/url_utility'; +import Icon from '~/vue_shared/components/icon.vue'; import CompareVersionsDropdown from './compare_versions_dropdown.vue'; export default { components: { CompareVersionsDropdown, + Icon, + }, + directives: { + Tooltip, }, props: { mergeRequestDiffs: { @@ -26,30 +35,119 @@ export default { }, }, computed: { + ...mapState('diffs', ['commit', 'showTreeList']), + ...mapGetters('diffs', ['isInlineView', 'isParallelView', 'areAllFilesCollapsed']), comparableDiffs() { return this.mergeRequestDiffs.slice(1); }, + isWhitespaceVisible() { + return !getParameterValues('w')[0]; + }, + toggleWhitespaceText() { + if (this.isWhitespaceVisible) { + return __('Hide whitespace changes'); + } + return __('Show whitespace changes'); + }, + toggleWhitespacePath() { + if (this.isWhitespaceVisible) { + return mergeUrlParams({ w: 1 }, window.location.href); + } + + return mergeUrlParams({ w: 0 }, window.location.href); + }, + showDropdowns() { + return !this.commit && this.mergeRequestDiffs.length; + }, + }, + methods: { + ...mapActions('diffs', [ + 'setInlineDiffViewType', + 'setParallelDiffViewType', + 'expandAllFiles', + 'toggleShowTreeList', + ]), }, }; </script> <template> <div class="mr-version-controls"> - <div class="mr-version-menus-container content-block"> - Changes between - <compare-versions-dropdown - :other-versions="mergeRequestDiffs" - :merge-request-version="mergeRequestDiff" - :show-commit-count="true" - class="mr-version-dropdown" - /> - and - <compare-versions-dropdown - :other-versions="comparableDiffs" - :start-version="startVersion" - :target-branch="targetBranch" - class="mr-version-compare-dropdown" - /> + <div + class="mr-version-menus-container content-block" + > + <button + v-tooltip.hover + type="button" + class="btn btn-default append-right-8 js-toggle-tree-list" + :class="{ + active: showTreeList + }" + :title="__('Toggle file browser')" + @click="toggleShowTreeList" + > + <icon + name="hamburger" + /> + </button> + <div + v-if="showDropdowns" + class="d-flex align-items-center compare-versions-container" + > + Changes between + <compare-versions-dropdown + :other-versions="mergeRequestDiffs" + :merge-request-version="mergeRequestDiff" + :show-commit-count="true" + class="mr-version-dropdown" + /> + and + <compare-versions-dropdown + :other-versions="comparableDiffs" + :start-version="startVersion" + :target-branch="targetBranch" + class="mr-version-compare-dropdown" + /> + </div> + <div + class="inline-parallel-buttons d-none d-md-flex ml-auto" + > + <a + v-if="areAllFilesCollapsed" + class="btn btn-default" + @click="expandAllFiles" + > + {{ __('Expand all') }} + </a> + <a + :href="toggleWhitespacePath" + class="btn btn-default" + > + {{ toggleWhitespaceText }} + </a> + <div class="btn-group prepend-left-8"> + <button + id="inline-diff-btn" + :class="{ active: isInlineView }" + type="button" + class="btn js-inline-diff-button" + data-view-type="inline" + @click="setInlineDiffViewType" + > + {{ __('Inline') }} + </button> + <button + id="parallel-diff-btn" + :class="{ active: isParallelView }" + type="button" + class="btn js-parallel-diff-button" + data-view-type="parallel" + @click="setParallelDiffViewType" + > + {{ __('Side-by-side') }} + </button> + </div> + </div> </div> </div> </template> diff --git a/app/assets/javascripts/diffs/components/compare_versions_dropdown.vue b/app/assets/javascripts/diffs/components/compare_versions_dropdown.vue index 96cccb49378..c3acc352d5e 100644 --- a/app/assets/javascripts/diffs/components/compare_versions_dropdown.vue +++ b/app/assets/javascripts/diffs/components/compare_versions_dropdown.vue @@ -108,7 +108,7 @@ export default { <template> <span class="dropdown inline"> <a - class="dropdown-toggle btn btn-default" + class="dropdown-menu-toggle btn btn-default w-100" data-toggle="dropdown" aria-expanded="false" > @@ -118,6 +118,7 @@ export default { <Icon :size="12" name="angle-down" + class="position-absolute" /> </a> <div class="dropdown-menu dropdown-select dropdown-menu-selectable"> @@ -163,3 +164,10 @@ export default { </div> </span> </template> + +<style> +.dropdown { + min-width: 0; + max-height: 170px; +} +</style> diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index 02d5be1821b..fb5556e3cd7 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -28,7 +28,7 @@ export default { return diffModes[diffModeKey] || diffModes.replaced; }, isTextFile() { - return this.diffFile.text; + return this.diffFile.viewer.name === 'text'; }, }, }; diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index e64d5511d78..cddbe554fbd 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -1,4 +1,5 @@ <script> +import { mapActions } from 'vuex'; import noteableDiscussion from '../../notes/components/noteable_discussion.vue'; export default { @@ -11,6 +12,14 @@ export default { required: true, }, }, + methods: { + ...mapActions('diffs', ['removeDiscussionsFromDiff']), + deleteNoteHandler(discussion) { + if (discussion.notes.length <= 1) { + this.removeDiscussionsFromDiff(discussion); + } + }, + }, }; </script> @@ -31,6 +40,7 @@ export default { :render-diff-file="false" :always-expanded="true" :discussions-by-diff-order="true" + @noteDeleted="deleteNoteHandler" /> </ul> </div> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 59e9ba08b8b..4e04e50c52a 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -1,9 +1,8 @@ <script> -import { mapActions } from 'vuex'; +import { mapActions, mapGetters, mapState } from 'vuex'; import _ from 'underscore'; import { __, sprintf } from '~/locale'; import createFlash from '~/flash'; -import LoadingIcon from '~/vue_shared/components/loading_icon.vue'; import DiffFileHeader from './diff_file_header.vue'; import DiffContent from './diff_content.vue'; @@ -11,7 +10,6 @@ export default { components: { DiffFileHeader, DiffContent, - LoadingIcon, }, props: { file: { @@ -30,6 +28,8 @@ export default { }; }, computed: { + ...mapState('diffs', ['currentDiffFileId']), + ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), isCollapsed() { return this.file.collapsed || false; }, @@ -44,23 +44,23 @@ export default { ); }, showExpandMessage() { - return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge; + return ( + this.isCollapsed || + !this.file.highlightedDiffLines && + !this.isLoadingCollapsedDiff && + !this.file.tooLarge && + this.file.text + ); }, showLoadingIcon() { return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed); }, }, methods: { - ...mapActions('diffs', ['loadCollapsedDiff']), + ...mapActions('diffs', ['loadCollapsedDiff', 'assignDiscussionsToDiff']), handleToggle() { - const { collapsed, highlightedDiffLines, parallelDiffLines } = this.file; - - if ( - collapsed && - !highlightedDiffLines && - parallelDiffLines !== undefined && - !parallelDiffLines.length - ) { + const { highlightedDiffLines, parallelDiffLines } = this.file; + if (!highlightedDiffLines && parallelDiffLines !== undefined && !parallelDiffLines.length) { this.handleLoadCollapsedDiff(); } else { this.file.collapsed = !this.file.collapsed; @@ -76,6 +76,14 @@ export default { this.file.collapsed = false; this.file.renderIt = true; }) + .then(() => { + requestIdleCallback( + () => { + this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); + }, + { timeout: 1000 }, + ); + }) .catch(() => { this.isLoadingCollapsedDiff = false; createFlash(__('Something went wrong on our end. Please try again!')); @@ -94,6 +102,9 @@ export default { <template> <div :id="file.fileHash" + :class="{ + 'is-active': currentDiffFileId === file.fileHash + }" class="diff-file file-holder" > <diff-file-header @@ -135,12 +146,12 @@ export default { :class="{ hidden: isCollapsed || file.tooLarge }" :diff-file="file" /> - <loading-icon - v-else-if="showLoadingIcon" + <gl-loading-icon + v-if="showLoadingIcon" class="diff-content loading" /> <div - v-if="showExpandMessage" + v-else-if="showExpandMessage" class="nothing-here-block diff-collapsed" > {{ __('This diff is collapsed.') }} @@ -161,3 +172,20 @@ export default { </div> </div> </template> + +<style> +@keyframes shadow-fade { + from { + box-shadow: 0 0 4px #919191; + } + + to { + box-shadow: 0 0 0 #dfdfdf; + } +} + +.diff-file.is-active { + box-shadow: 0 0 0 #dfdfdf; + animation: shadow-fade 1.2s 0.1s 1; +} +</style> diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index d3ffbe0415a..15b37243030 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -166,23 +166,21 @@ export default { :title="diffFile.oldPath" class="file-title-name" data-container="body" - > - {{ diffFile.oldPath }} - </strong> + v-html="diffFile.oldPathHtml" + ></strong> → <strong v-tooltip :title="diffFile.newPath" class="file-title-name" data-container="body" - > - {{ diffFile.newPath }} - </strong> + v-html="diffFile.newPathHtml" + ></strong> </span> <strong - v-tooltip v-else + v-tooltip :title="filePath" class="file-title-name" data-container="body" @@ -255,8 +253,8 @@ export default { </a> <a - v-tooltip v-if="diffFile.externalUrl" + v-tooltip :href="diffFile.externalUrl" :title="`View on ${diffFile.formattedExternalUrl}`" target="_blank" diff --git a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue index 7e50a0aed84..1b59777f901 100644 --- a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue +++ b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue @@ -1,15 +1,11 @@ <script> import { mapActions } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; -import tooltip from '~/vue_shared/directives/tooltip'; import { pluralize, truncate } from '~/lib/utils/text_utility'; import UserAvatarImage from '~/vue_shared/components/user_avatar/user_avatar_image.vue'; import { COUNT_OF_AVATARS_IN_GUTTER, LENGTH_OF_AVATAR_TOOLTIP } from '../constants'; export default { - directives: { - tooltip, - }, components: { Icon, UserAvatarImage, @@ -91,10 +87,10 @@ export default { @click.native="toggleDiscussions" /> <span - v-tooltip v-if="moreText" + v-gl-tooltip :title="moreText" - class="diff-comments-more-count has-tooltip js-diff-comment-avatar js-diff-comment-plus" + class="diff-comments-more-count js-diff-comment-avatar js-diff-comment-plus" data-container="body" data-placement="top" role="button" diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue index 8ad1ea34245..6eff3013dcd 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -13,6 +13,10 @@ export default { Icon, }, props: { + line: { + type: Object, + required: true, + }, fileHash: { type: String, required: true, @@ -21,31 +25,16 @@ export default { type: String, required: true, }, - lineType: { - type: String, - required: false, - default: '', - }, lineNumber: { type: Number, required: false, default: 0, }, - lineCode: { - type: String, - required: false, - default: '', - }, linePosition: { type: String, required: false, default: '', }, - metaData: { - type: Object, - required: false, - default: () => ({}), - }, showCommentButton: { type: Boolean, required: false, @@ -76,11 +65,6 @@ export default { required: false, default: false, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapState({ @@ -89,7 +73,7 @@ export default { }), ...mapGetters(['isLoggedIn']), lineHref() { - return this.lineCode ? `#${this.lineCode}` : '#'; + return `#${this.line.lineCode || ''}`; }, shouldShowCommentButton() { return ( @@ -103,20 +87,19 @@ export default { ); }, hasDiscussions() { - return this.discussions.length > 0; + return this.line.discussions && this.line.discussions.length > 0; }, shouldShowAvatarsOnGutter() { - if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) { + if (!this.line.type && this.linePosition === LINE_POSITION_RIGHT) { return false; } - return this.showCommentButton && this.hasDiscussions; }, }, methods: { ...mapActions('diffs', ['loadMoreLines', 'showCommentForm']), handleCommentButton() { - this.showCommentForm({ lineCode: this.lineCode }); + this.showCommentForm({ lineCode: this.line.lineCode }); }, handleLoadMoreLines() { if (this.isRequesting) { @@ -125,8 +108,8 @@ export default { this.isRequesting = true; const endpoint = this.contextLinesPath; - const oldLineNumber = this.metaData.oldPos || 0; - const newLineNumber = this.metaData.newPos || 0; + const oldLineNumber = this.line.metaData.oldPos || 0; + const newLineNumber = this.line.metaData.newPos || 0; const offset = newLineNumber - oldLineNumber; const bottom = this.isBottom; const { fileHash } = this; @@ -201,7 +184,7 @@ export default { </a> <diff-gutter-avatars v-if="shouldShowAvatarsOnGutter" - :discussions="discussions" + :discussions="line.discussions" /> </template> </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 cbe4551d06b..bb9bb821de3 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -1,9 +1,7 @@ <script> import { mapState, mapGetters, mapActions } from 'vuex'; -import createFlash from '~/flash'; import { s__ } from '~/locale'; import noteForm from '../../notes/components/note_form.vue'; -import { getNoteFormData } from '../store/utils'; import autosave from '../../notes/mixins/autosave'; import { DIFF_NOTE_TYPE } from '../constants'; @@ -21,7 +19,7 @@ export default { type: Object, required: true, }, - position: { + linePosition: { type: String, required: false, default: '', @@ -38,6 +36,16 @@ export default { }), ...mapGetters('diffs', ['getDiffFileByHash']), ...mapGetters(['isLoggedIn', 'noteableType', 'getNoteableData', 'getNotesDataByProp']), + formData() { + return { + noteableData: this.noteableData, + noteableType: this.noteableType, + noteTargetLine: this.noteTargetLine, + diffViewType: this.diffViewType, + diffFile: this.getDiffFileByHash(this.diffFileHash), + linePosition: this.linePosition, + }; + }, }, mounted() { if (this.isLoggedIn) { @@ -52,8 +60,7 @@ export default { } }, methods: { - ...mapActions('diffs', ['cancelCommentForm']), - ...mapActions(['saveNote', 'refetchDiscussionById']), + ...mapActions('diffs', ['cancelCommentForm', 'assignDiscussionsToDiff', 'saveDiffDiscussion']), handleCancelCommentForm(shouldConfirm, isDirty) { if (shouldConfirm && isDirty) { const msg = s__('Notes|Are you sure you want to cancel creating this comment?'); @@ -72,32 +79,9 @@ export default { }); }, handleSaveNote(note) { - const selectedDiffFile = this.getDiffFileByHash(this.diffFileHash); - const postData = getNoteFormData({ - note, - noteableData: this.noteableData, - noteableType: this.noteableType, - noteTargetLine: this.noteTargetLine, - diffViewType: this.diffViewType, - diffFile: selectedDiffFile, - linePosition: this.position, - }); - - this.saveNote(postData) - .then(result => { - const endpoint = this.getNotesDataByProp('discussionsPath'); - - this.refetchDiscussionById({ path: endpoint, discussionId: result.discussion_id }) - .then(() => { - this.handleCancelCommentForm(); - }) - .catch(() => { - createFlash(s__('MergeRequests|Updating discussions failed')); - }); - }) - .catch(() => { - createFlash(s__('MergeRequests|Saving the comment failed')); - }); + return this.saveDiffDiscussion({ note, formData: this.formData }).then(() => + this.handleCancelCommentForm(), + ); }, }, }; diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index 33bc8d9971e..5d9a0b123fe 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -11,8 +11,6 @@ import { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME, INLINE_DIFF_VIEW_TYPE, - LINE_POSITION_LEFT, - LINE_POSITION_RIGHT, } from '../constants'; export default { @@ -67,42 +65,24 @@ export default { required: false, default: false, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapGetters(['isLoggedIn']), - normalizedLine() { - let normalizedLine; - - if (this.diffViewType === INLINE_DIFF_VIEW_TYPE) { - normalizedLine = this.line; - } else if (this.linePosition === LINE_POSITION_LEFT) { - normalizedLine = this.line.left; - } else if (this.linePosition === LINE_POSITION_RIGHT) { - normalizedLine = this.line.right; - } - - return normalizedLine; - }, isMatchLine() { - return this.normalizedLine.type === MATCH_LINE_TYPE; + return this.line.type === MATCH_LINE_TYPE; }, isContextLine() { - return this.normalizedLine.type === CONTEXT_LINE_TYPE; + return this.line.type === CONTEXT_LINE_TYPE; }, isMetaLine() { - const { type } = this.normalizedLine; + const { type } = this.line; return ( type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE || type === EMPTY_CELL_TYPE ); }, classNameMap() { - const { type } = this.normalizedLine; + const { type } = this.line; return { [type]: type, @@ -116,9 +96,9 @@ export default { }; }, lineNumber() { - const { lineType, normalizedLine } = this; + const { lineType } = this; - return lineType === OLD_LINE_TYPE ? normalizedLine.oldLine : normalizedLine.newLine; + return lineType === OLD_LINE_TYPE ? this.line.oldLine : this.line.newLine; }, }, }; @@ -129,20 +109,17 @@ export default { :class="classNameMap" > <diff-line-gutter-content + :line="line" :file-hash="fileHash" :context-lines-path="contextLinesPath" - :line-type="normalizedLine.type" - :line-code="normalizedLine.lineCode" :line-position="linePosition" :line-number="lineNumber" - :meta-data="normalizedLine.metaData" :show-comment-button="showCommentButton" :is-hover="isHover" :is-bottom="isBottom" :is-match-line="isMatchLine" :is-context-line="isContentLine" :is-meta-line="isMetaLine" - :discussions="discussions" /> </td> </template> diff --git a/app/assets/javascripts/diffs/components/file_row_stats.vue b/app/assets/javascripts/diffs/components/file_row_stats.vue new file mode 100644 index 00000000000..105f7ebdbed --- /dev/null +++ b/app/assets/javascripts/diffs/components/file_row_stats.vue @@ -0,0 +1,30 @@ +<script> +export default { + props: { + file: { + type: Object, + required: true, + }, + }, +}; +</script> + +<template> + <span + v-once + class="file-row-stats" + > + <span class="cgreen"> + +{{ file.addedLines }} + </span> + <span class="cred"> + -{{ file.removedLines }} + </span> + </span> +</template> + +<style> +.file-row-stats { + font-size: 12px; +} +</style> diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue index caf84dc9573..46a51859da5 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue @@ -21,18 +21,13 @@ export default { type: Number, required: true, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), className() { - return this.discussions.length ? '' : 'js-temp-notes-holder'; + return this.line.discussions.length ? '' : 'js-temp-notes-holder'; }, }, }; @@ -44,14 +39,13 @@ export default { class="notes_holder" > <td - class="notes_line" - colspan="2" - ></td> - <td class="notes_content"> + class="notes_content" + colspan="3" + > <div class="content"> <diff-discussions - v-if="discussions.length" - :discussions="discussions" + v-if="line.discussions.length" + :discussions="line.discussions" /> <diff-line-note-form v-if="diffLineCommentForms[line.lineCode]" 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 32d65ff994f..62fa34e835a 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -1,5 +1,5 @@ <script> -import { mapGetters } from 'vuex'; +import { mapGetters, mapActions } from 'vuex'; import DiffTableCell from './diff_table_cell.vue'; import { NEW_LINE_TYPE, @@ -33,11 +33,6 @@ export default { required: false, default: false, }, - discussions: { - type: Array, - required: false, - default: () => [], - }, }, data() { return { @@ -68,7 +63,11 @@ export default { this.linePositionLeft = LINE_POSITION_LEFT; this.linePositionRight = LINE_POSITION_RIGHT; }, + mounted() { + this.scrollToLineIfNeededInline(this.line); + }, methods: { + ...mapActions('diffs', ['scrollToLineIfNeededInline']), handleMouseMove(e) { // To show the comment icon on the gutter we need to know if we hover the line. // Current table structure doesn't allow us to do this with CSS in both of the diff view types @@ -94,7 +93,6 @@ export default { :is-bottom="isBottom" :is-hover="isHover" :show-comment-button="true" - :discussions="discussions" class="diff-line-num old_line" /> <diff-table-cell @@ -104,7 +102,6 @@ export default { :line-type="newLineType" :is-bottom="isBottom" :is-hover="isHover" - :discussions="discussions" class="diff-line-num new_line" /> <td diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index e7d789734c3..fbf9e77ac07 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -2,7 +2,6 @@ import { mapGetters, mapState } from 'vuex'; import inlineDiffTableRow from './inline_diff_table_row.vue'; import inlineDiffCommentRow from './inline_diff_comment_row.vue'; -import { trimFirstCharOfLineContent } from '../store/utils'; export default { components: { @@ -20,29 +19,17 @@ export default { }, }, computed: { - ...mapGetters('diffs', [ - 'commitId', - 'shouldRenderInlineCommentRow', - 'singleDiscussionByLineCode', - ]), + ...mapGetters('diffs', ['commitId', 'shouldRenderInlineCommentRow']), ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), - normalizedDiffLines() { - return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line)); - }, diffLinesLength() { - return this.normalizedDiffLines.length; + return this.diffLines.length; }, userColorScheme() { return window.gon.user_color_scheme; }, }, - methods: { - discussionsList(line) { - return line.lineCode !== undefined ? this.singleDiscussionByLineCode(line.lineCode) : []; - }, - }, }; </script> @@ -53,23 +40,21 @@ export default { class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view"> <tbody> <template - v-for="(line, index) in normalizedDiffLines" + v-for="(line, index) in diffLines" > <inline-diff-table-row + :key="line.lineCode" :file-hash="diffFile.fileHash" :context-lines-path="diffFile.contextLinesPath" :line="line" :is-bottom="index + 1 === diffLinesLength" - :key="line.lineCode" - :discussions="discussionsList(line)" /> <inline-diff-comment-row v-if="shouldRenderInlineCommentRow(line)" + :key="index" :diff-file-hash="diffFile.fileHash" :line="line" :line-index="index" - :key="index" - :discussions="discussionsList(line)" /> </template> </tbody> diff --git a/app/assets/javascripts/diffs/components/no_changes.vue b/app/assets/javascripts/diffs/components/no_changes.vue index d817157fbcd..6905630ad8c 100644 --- a/app/assets/javascripts/diffs/components/no_changes.vue +++ b/app/assets/javascripts/diffs/components/no_changes.vue @@ -38,7 +38,7 @@ export default { <div class="text-center"> <a :href="newBlobPath" - class="btn btn-save" + class="btn btn-success" > {{ __('Create commit') }} </a> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue index 48b8feeb0b4..3339c56cbb6 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -21,51 +21,49 @@ export default { type: Number, required: true, }, - leftDiscussions: { - type: Array, - required: false, - default: () => [], - }, - rightDiscussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), leftLineCode() { - return this.line.left.lineCode; + return this.line.left && this.line.left.lineCode; }, rightLineCode() { - return this.line.right.lineCode; + return this.line.right && this.line.right.lineCode; }, hasExpandedDiscussionOnLeft() { - const discussions = this.leftDiscussions; - - return discussions ? discussions.every(discussion => discussion.expanded) : false; + return this.line.left && this.line.left.discussions + ? this.line.left.discussions.every(discussion => discussion.expanded) + : false; }, hasExpandedDiscussionOnRight() { - const discussions = this.rightDiscussions; - - return discussions ? discussions.every(discussion => discussion.expanded) : false; + return this.line.right && this.line.right.discussions + ? this.line.right.discussions.every(discussion => discussion.expanded) + : false; }, hasAnyExpandedDiscussion() { return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; }, shouldRenderDiscussionsOnLeft() { - return this.leftDiscussions && this.hasExpandedDiscussionOnLeft; + return this.line.left && this.line.left.discussions && this.hasExpandedDiscussionOnLeft; }, shouldRenderDiscussionsOnRight() { - return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type; + return ( + this.line.right && + this.line.right.discussions && + this.hasExpandedDiscussionOnRight && + this.line.right.type + ); }, showRightSideCommentForm() { - return this.line.right.type && this.diffLineCommentForms[this.rightLineCode]; + return ( + this.line.right && this.line.right.type && this.diffLineCommentForms[this.rightLineCode] + ); }, className() { - return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0 + return (this.left && this.line.left.discussions.length > 0) || + (this.right && this.line.right.discussions.length > 0) ? '' : 'js-temp-notes-holder'; }, @@ -85,8 +83,8 @@ export default { class="content" > <diff-discussions - v-if="leftDiscussions.length" - :discussions="leftDiscussions" + v-if="line.left.discussions.length" + :discussions="line.left.discussions" /> </div> <diff-line-note-form @@ -94,7 +92,7 @@ export default { :diff-file-hash="diffFileHash" :line="line.left" :note-target-line="line.left" - position="left" + line-position="left" /> </td> <td class="notes_line new"></td> @@ -104,8 +102,8 @@ export default { class="content" > <diff-discussions - v-if="rightDiscussions.length" - :discussions="rightDiscussions" + v-if="line.right.discussions.length" + :discussions="line.right.discussions" /> </div> <diff-line-note-form @@ -113,7 +111,7 @@ export default { :diff-file-hash="diffFileHash" :line="line.right" :note-target-line="line.right" - position="right" + line-position="right" /> </td> </tr> 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 d4e54c2bd00..fcc3b3e9117 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -1,6 +1,6 @@ <script> +import { mapActions } from 'vuex'; import $ from 'jquery'; -import { mapGetters } from 'vuex'; import DiffTableCell from './diff_table_cell.vue'; import { NEW_LINE_TYPE, @@ -10,8 +10,7 @@ import { OLD_NO_NEW_LINE_TYPE, PARALLEL_DIFF_VIEW_TYPE, NEW_NO_NEW_LINE_TYPE, - LINE_POSITION_LEFT, - LINE_POSITION_RIGHT, + EMPTY_CELL_TYPE, } from '../constants'; export default { @@ -36,16 +35,6 @@ export default { required: false, default: false, }, - leftDiscussions: { - type: Array, - required: false, - default: () => [], - }, - rightDiscussions: { - type: Array, - required: false, - default: () => [], - }, }, data() { return { @@ -54,32 +43,33 @@ export default { }; }, computed: { - ...mapGetters('diffs', ['isParallelView']), isContextLine() { - return this.line.left.type === CONTEXT_LINE_TYPE; + return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE; }, classNameMap() { return { [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, - [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView, + [PARALLEL_DIFF_VIEW_TYPE]: true, }; }, parallelViewLeftLineType() { - if (this.line.right.type === NEW_NO_NEW_LINE_TYPE) { + if (this.line.right && this.line.right.type === NEW_NO_NEW_LINE_TYPE) { return OLD_NO_NEW_LINE_TYPE; } - return this.line.left.type; + return this.line.left ? this.line.left.type : EMPTY_CELL_TYPE; }, }, created() { this.newLineType = NEW_LINE_TYPE; this.oldLineType = OLD_LINE_TYPE; - this.linePositionLeft = LINE_POSITION_LEFT; - this.linePositionRight = LINE_POSITION_RIGHT; this.parallelDiffViewType = PARALLEL_DIFF_VIEW_TYPE; }, + mounted() { + this.scrollToLineIfNeededParallel(this.line); + }, methods: { + ...mapActions('diffs', ['scrollToLineIfNeededParallel']), handleMouseMove(e) { const isHover = e.type === 'mouseover'; const hoveringCell = e.target.closest('td'); @@ -116,47 +106,57 @@ export default { @mouseover="handleMouseMove" @mouseout="handleMouseMove" > - <diff-table-cell - :file-hash="fileHash" - :context-lines-path="contextLinesPath" - :line="line" - :line-type="oldLineType" - :line-position="linePositionLeft" - :is-bottom="isBottom" - :is-hover="isLeftHover" - :show-comment-button="true" - :diff-view-type="parallelDiffViewType" - :discussions="leftDiscussions" - class="diff-line-num old_line" - /> - <td - :id="line.left.lineCode" - :class="parallelViewLeftLineType" - class="line_content parallel left-side" - @mousedown.native="handleParallelLineMouseDown" - v-html="line.left.richText" - > - </td> - <diff-table-cell - :file-hash="fileHash" - :context-lines-path="contextLinesPath" - :line="line" - :line-type="newLineType" - :line-position="linePositionRight" - :is-bottom="isBottom" - :is-hover="isRightHover" - :show-comment-button="true" - :diff-view-type="parallelDiffViewType" - :discussions="rightDiscussions" - class="diff-line-num new_line" - /> - <td - :id="line.right.lineCode" - :class="line.right.type" - class="line_content parallel right-side" - @mousedown.native="handleParallelLineMouseDown" - v-html="line.right.richText" - > - </td> + <template v-if="line.left"> + <diff-table-cell + :file-hash="fileHash" + :context-lines-path="contextLinesPath" + :line="line.left" + :line-type="oldLineType" + :is-bottom="isBottom" + :is-hover="isLeftHover" + :show-comment-button="true" + :diff-view-type="parallelDiffViewType" + line-position="left" + class="diff-line-num old_line" + /> + <td + :id="line.left.lineCode" + :class="parallelViewLeftLineType" + class="line_content parallel left-side" + @mousedown.native="handleParallelLineMouseDown" + v-html="line.left.richText" + > + </td> + </template> + <template v-else> + <td class="diff-line-num old_line empty-cell"></td> + <td class="line_content parallel left-side empty-cell"></td> + </template> + <template v-if="line.right"> + <diff-table-cell + :file-hash="fileHash" + :context-lines-path="contextLinesPath" + :line="line.right" + :line-type="newLineType" + :is-bottom="isBottom" + :is-hover="isRightHover" + :show-comment-button="true" + :diff-view-type="parallelDiffViewType" + line-position="right" + class="diff-line-num new_line" + /> + <td + :id="line.right.lineCode" + :class="line.right.type" + class="line_content parallel right-side" + @mousedown.native="handleParallelLineMouseDown" + v-html="line.right.richText" + > + </td> + </template> + <template v-else> + <td class="diff-line-num old_line empty-cell"></td> + <td class="line_content parallel right-side empty-cell"></td> + </template> </tr> </template> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 24ceb52a04a..3452f0d2b00 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -2,8 +2,6 @@ import { mapState, mapGetters } from 'vuex'; import parallelDiffTableRow from './parallel_diff_table_row.vue'; import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; -import { EMPTY_CELL_TYPE } from '../constants'; -import { trimFirstCharOfLineContent } from '../store/utils'; export default { components: { @@ -21,46 +19,17 @@ export default { }, }, computed: { - ...mapGetters('diffs', [ - 'commitId', - 'singleDiscussionByLineCode', - 'shouldRenderParallelCommentRow', - ]), + ...mapGetters('diffs', ['commitId', 'shouldRenderParallelCommentRow']), ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), - parallelDiffLines() { - return this.diffLines.map(line => { - const parallelLine = Object.assign({}, line); - - if (line.left) { - parallelLine.left = trimFirstCharOfLineContent(line.left); - } else { - parallelLine.left = { type: EMPTY_CELL_TYPE }; - } - - if (line.right) { - parallelLine.right = trimFirstCharOfLineContent(line.right); - } else { - parallelLine.right = { type: EMPTY_CELL_TYPE }; - } - - return parallelLine; - }); - }, diffLinesLength() { - return this.parallelDiffLines.length; + return this.diffLines.length; }, userColorScheme() { return window.gon.user_color_scheme; }, }, - methods: { - discussionsByLine(line, leftOrRight) { - return line[leftOrRight] && line[leftOrRight].lineCode !== undefined ? - this.singleDiscussionByLineCode(line[leftOrRight].lineCode) : []; - }, - }, }; </script> @@ -73,16 +42,14 @@ export default { <table> <tbody> <template - v-for="(line, index) in parallelDiffLines" + v-for="(line, index) in diffLines" > <parallel-diff-table-row + :key="index" :file-hash="diffFile.fileHash" :context-lines-path="diffFile.contextLinesPath" :line="line" :is-bottom="index + 1 === diffLinesLength" - :key="index" - :left-discussions="discussionsByLine(line, 'left')" - :right-discussions="discussionsByLine(line, 'right')" /> <parallel-diff-comment-row v-if="shouldRenderParallelCommentRow(line)" @@ -90,8 +57,6 @@ export default { :line="line" :diff-file-hash="diffFile.fileHash" :line-index="index" - :left-discussions="discussionsByLine(line, 'left')" - :right-discussions="discussionsByLine(line, 'right')" /> </template> </tbody> diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue new file mode 100644 index 00000000000..cfe4273742f --- /dev/null +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -0,0 +1,101 @@ +<script> +import { mapActions, mapGetters, mapState } from 'vuex'; +import Icon from '~/vue_shared/components/icon.vue'; +import FileRow from '~/vue_shared/components/file_row.vue'; +import FileRowStats from './file_row_stats.vue'; + +export default { + components: { + Icon, + FileRow, + }, + data() { + return { + search: '', + }; + }, + computed: { + ...mapState('diffs', ['tree', 'addedLines', 'removedLines']), + ...mapGetters('diffs', ['allBlobs', 'diffFilesLength']), + filteredTreeList() { + const search = this.search.toLowerCase().trim(); + + if (search === '') return this.tree; + + return this.allBlobs.filter(f => f.name.toLowerCase().indexOf(search) >= 0); + }, + }, + methods: { + ...mapActions('diffs', ['toggleTreeOpen', 'scrollToFile']), + clearSearch() { + this.search = ''; + }, + }, + FileRowStats, +}; +</script> + +<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" + > + <icon + name="close" + /> + </button> + </div> + <div + class="tree-list-scroll" + > + <template v-if="filteredTreeList.length"> + <file-row + v-for="file in filteredTreeList" + :key="file.key" + :file="file" + :level="0" + :hide-extra-on-tree="true" + :extra-component="$options.FileRowStats" + :show-changed-icon="true" + @toggleTreeOpen="toggleTreeOpen" + @clickFile="scrollToFile" + /> + </template> + <p + v-else + class="prepend-top-20 append-bottom-20 text-center" + > + {{ s__('MergeRequest|No files found') }} + </p> + </div> + <div + v-once + class="pt-3 pb-3 text-center" + > + {{ n__('%d changed file', '%d changed files', diffFilesLength) }} + <div> + <span class="cgreen"> + {{ n__('%d addition', '%d additions', addedLines) }} + </span> + <span class="cred"> + {{ n__('%d deleted', '%d deletions', removedLines) }} + </span> + </div> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index f68afa44837..6a50d2c1426 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -7,6 +7,7 @@ export const CONTEXT_LINE_TYPE = 'context'; export const EMPTY_CELL_TYPE = 'empty-cell'; export const COMMENT_FORM_TYPE = 'commentForm'; export const DIFF_NOTE_TYPE = 'DiffNote'; +export const LEGACY_DIFF_NOTE_TYPE = 'LegacyDiffNote'; export const NOTE_TYPE = 'Note'; export const NEW_LINE_TYPE = 'new'; export const OLD_LINE_TYPE = 'old'; @@ -28,3 +29,5 @@ export const LENGTH_OF_AVATAR_TOOLTIP = 17; export const LINES_TO_BE_RENDERED_DIRECTLY = 100; export const MAX_LINES_TO_BE_RENDERED = 2000; + +export const MR_TREE_SHOW_KEY = 'mr_tree_show'; diff --git a/app/assets/javascripts/diffs/mixins/changed_files.js b/app/assets/javascripts/diffs/mixins/changed_files.js deleted file mode 100644 index da1339f0ffa..00000000000 --- a/app/assets/javascripts/diffs/mixins/changed_files.js +++ /dev/null @@ -1,38 +0,0 @@ -export default { - props: { - diffFiles: { - type: Array, - required: true, - }, - }, - methods: { - fileChangedIcon(diffFile) { - if (diffFile.deletedFile) { - return 'file-deletion'; - } else if (diffFile.newFile) { - return 'file-addition'; - } - return 'file-modified'; - }, - fileChangedClass(diffFile) { - if (diffFile.deletedFile) { - return 'cred'; - } else if (diffFile.newFile) { - return 'cgreen'; - } - - return ''; - }, - truncatedDiffPath(path) { - const maxLength = 60; - - if (path.length > maxLength) { - const start = path.length - maxLength; - const end = start + maxLength; - return `...${path.slice(start, end)}`; - } - - return path; - }, - }, -}; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 4ab6ceb249a..1e0b27b538d 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -1,13 +1,18 @@ import Vue from 'vue'; import axios from '~/lib/utils/axios_utils'; import Cookies from 'js-cookie'; +import createFlash from '~/flash'; +import { s__ } from '~/locale'; import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils'; -import { mergeUrlParams } from '~/lib/utils/url_utility'; +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 { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE, DIFF_VIEW_COOKIE_NAME, + MR_TREE_SHOW_KEY, } from '../constants'; export const setBaseConfig = ({ commit }, options) => { @@ -29,25 +34,53 @@ export const fetchDiffFiles = ({ state, commit }) => { .then(handleLocationHash); }; -export const startRenderDiffsQueue = ({ state, commit }) => { - const checkItem = () => { - const nextFile = state.diffFiles.find( - file => !file.renderIt && (!file.collapsed || !file.text), - ); - if (nextFile) { - requestAnimationFrame(() => { - commit(types.RENDER_FILE, nextFile); +// 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) => { + 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, }); - requestIdleCallback( - () => { - checkItem(); - }, - { timeout: 1000 }, - ); } - }; + }); +}; - checkItem(); +export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => { + const { fileHash, line_code } = removeDiscussion; + commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code }); +}; + +export const startRenderDiffsQueue = ({ state, commit }) => { + const checkItem = () => + new Promise(resolve => { + const nextFile = state.diffFiles.find( + file => !file.renderIt && (!file.collapsed || !file.text), + ); + + if (nextFile) { + requestAnimationFrame(() => { + commit(types.RENDER_FILE, nextFile); + }); + requestIdleCallback( + () => { + checkItem() + .then(resolve) + .catch(() => {}); + }, + { timeout: 1000 }, + ); + } else { + resolve(); + } + }); + + return checkItem(); }; export const setInlineDiffViewType = ({ commit }) => { @@ -91,6 +124,25 @@ export const loadMoreLines = ({ commit }, options) => { }); }; +export const scrollToLineIfNeededInline = (_, line) => { + const hash = getLocationHash(); + + if (hash && line.lineCode === hash) { + handleLocationHash(); + } +}; + +export const scrollToLineIfNeededParallel = (_, line) => { + const hash = getLocationHash(); + + if ( + hash && + ((line.left && line.left.lineCode === hash) || (line.right && line.right.lineCode === hash)) + ) { + handleLocationHash(); + } +}; + export const loadCollapsedDiff = ({ commit }, file) => axios.get(file.loadCollapsedDiffUrl).then(res => { commit(types.ADD_COLLAPSED_DIFFS, { @@ -130,5 +182,37 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => { }); }; +export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { + const postData = getNoteFormData({ + note, + ...formData, + }); + + return dispatch('saveNote', postData, { root: true }) + .then(result => dispatch('updateDiscussion', result.discussion, { root: true })) + .then(discussion => + dispatch('assignDiscussionsToDiff', reduceDiscussionsToLineCodes([discussion])), + ) + .catch(() => createFlash(s__('MergeRequests|Saving the comment failed'))); +}; + +export const toggleTreeOpen = ({ commit }, path) => { + commit(types.TOGGLE_FOLDER_OPEN, path); +}; + +export const scrollToFile = ({ state, commit }, path) => { + const { fileHash } = state.treeEntries[path]; + document.location.hash = fileHash; + + commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash); + + setTimeout(() => commit(types.UPDATE_CURRENT_DIFF_FILE_ID, ''), 1000); +}; + +export const toggleShowTreeList = ({ commit, state }) => { + commit(types.TOGGLE_SHOW_TREE_LIST); + localStorage.setItem(MR_TREE_SHOW_KEY, state.showTreeList); +}; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 4a47646d7fa..d4c205882ff 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -17,7 +17,10 @@ export const commitId = state => (state.commit && state.commit.id ? state.commit export const diffHasAllExpandedDiscussions = (state, getters) => diff => { const discussions = getters.getDiffFileDiscussions(diff); - return (discussions.length && discussions.every(discussion => discussion.expanded)) || false; + return ( + (discussions && discussions.length && discussions.every(discussion => discussion.expanded)) || + false + ); }; /** @@ -28,7 +31,10 @@ export const diffHasAllExpandedDiscussions = (state, getters) => diff => { export const diffHasAllCollpasedDiscussions = (state, getters) => diff => { const discussions = getters.getDiffFileDiscussions(diff); - return (discussions.length && discussions.every(discussion => !discussion.expanded)) || false; + return ( + (discussions && discussions.length && discussions.every(discussion => !discussion.expanded)) || + false + ); }; /** @@ -40,7 +46,9 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => { const discussions = getters.getDiffFileDiscussions(diff); return ( - (discussions.length && discussions.find(discussion => discussion.expanded) !== undefined) || + (discussions && + discussions.length && + discussions.find(discussion => discussion.expanded) !== undefined) || false ); }; @@ -64,50 +72,47 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) = discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash), ) || []; -export const singleDiscussionByLineCode = (state, getters, rootState, rootGetters) => lineCode => { - if (!lineCode || lineCode === undefined) return []; - const discussions = rootGetters.discussionsByLineCode; - return discussions[lineCode] || []; -}; - -export const shouldRenderParallelCommentRow = (state, getters) => line => { - const leftLineCode = line.left.lineCode; - const rightLineCode = line.right.lineCode; - const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode); - const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode); - const hasDiscussion = leftDiscussions.length || rightDiscussions.length; +export const shouldRenderParallelCommentRow = state => line => { + const hasDiscussion = + (line.left && line.left.discussions && line.left.discussions.length) || + (line.right && line.right.discussions && line.right.discussions.length); - const hasExpandedDiscussionOnLeft = leftDiscussions.length - ? leftDiscussions.every(discussion => discussion.expanded) - : false; - const hasExpandedDiscussionOnRight = rightDiscussions.length - ? rightDiscussions.every(discussion => discussion.expanded) - : false; + const hasExpandedDiscussionOnLeft = + line.left && line.left.discussions && line.left.discussions.length + ? line.left.discussions.every(discussion => discussion.expanded) + : false; + const hasExpandedDiscussionOnRight = + line.right && line.right.discussions && line.right.discussions.length + ? line.right.discussions.every(discussion => discussion.expanded) + : false; if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) { return true; } - const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode]; - const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode]; + const hasCommentFormOnLeft = line.left && state.diffLineCommentForms[line.left.lineCode]; + const hasCommentFormOnRight = line.right && state.diffLineCommentForms[line.right.lineCode]; return hasCommentFormOnLeft || hasCommentFormOnRight; }; -export const shouldRenderInlineCommentRow = (state, getters) => line => { +export const shouldRenderInlineCommentRow = state => line => { if (state.diffLineCommentForms[line.lineCode]) return true; - const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode); - if (lineDiscussions.length === 0) { + if (!line.discussions || line.discussions.length === 0) { return false; } - return lineDiscussions.every(discussion => discussion.expanded); + return line.discussions.every(discussion => discussion.expanded); }; // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests export const getDiffFileByHash = state => fileHash => state.diffFiles.find(file => file.fileHash === fileHash); +export const allBlobs = state => Object.values(state.treeEntries).filter(f => f.type === 'blob'); + +export const diffFilesLength = state => state.diffFiles.length; + // 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 39d90a64aab..ae8930c8968 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -1,18 +1,25 @@ 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, MR_TREE_SHOW_KEY } from '../../constants'; const viewTypeFromQueryString = getParameterValues('view')[0]; const viewTypeFromCookie = Cookies.get(DIFF_VIEW_COOKIE_NAME); const defaultViewType = INLINE_DIFF_VIEW_TYPE; +const storedTreeShow = localStorage.getItem(MR_TREE_SHOW_KEY); export default () => ({ isLoading: true, endpoint: '', basePath: '', commit: null, + startVersion: null, diffFiles: [], mergeRequestDiffs: [], + mergeRequestDiff: null, diffLineCommentForms: {}, diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType, + tree: [], + treeEntries: {}, + showTreeList: storedTreeShow === null ? true : storedTreeShow === 'true', + currentDiffFileId: '', }); diff --git a/app/assets/javascripts/diffs/store/modules/index.js b/app/assets/javascripts/diffs/store/modules/index.js index 20d1ebbe049..6860e24db6b 100644 --- a/app/assets/javascripts/diffs/store/modules/index.js +++ b/app/assets/javascripts/diffs/store/modules/index.js @@ -3,10 +3,10 @@ import * as getters from '../getters'; import mutations from '../mutations'; import createState from './diff_state'; -export default { +export default () => ({ namespaced: true, state: createState(), getters, actions, mutations, -}; +}); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index c999d637d50..6474ee628e2 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -9,3 +9,8 @@ export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES'; export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS'; export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES'; export const RENDER_FILE = 'RENDER_FILE'; +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 TOGGLE_SHOW_TREE_LIST = 'TOGGLE_SHOW_TREE_LIST'; +export const UPDATE_CURRENT_DIFF_FILE_ID = 'UPDATE_CURRENT_DIFF_FILE_ID'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 0522e32c410..0b4485ecdb5 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -1,8 +1,15 @@ import Vue from 'vue'; -import _ from 'underscore'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; -import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils'; -import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants'; +import { sortTree } from '~/ide/stores/utils'; +import { + findDiffFile, + addLineReferences, + removeMatchLine, + addContextLines, + prepareDiffData, + isDiscussionApplicableToLine, + generateTreeList, +} from './utils'; import * as types from './mutation_types'; export default { @@ -17,41 +24,13 @@ export default { [types.SET_DIFF_DATA](state, data) { const diffData = convertObjectPropsToCamelCase(data, { deep: true }); - let showingLines = 0; - const filesLength = diffData.diffFiles.length; - let i; - for (i = 0; i < filesLength; i += 1) { - const file = diffData.diffFiles[i]; - if (file.parallelDiffLines) { - const linesLength = file.parallelDiffLines.length; - let u = 0; - for (u = 0; u < linesLength; u += 1) { - const line = file.parallelDiffLines[u]; - if (line.left) delete line.left.text; - if (line.right) delete line.right.text; - } - } - - if (file.highlightedDiffLines) { - const linesLength = file.highlightedDiffLines.length; - let u; - for (u = 0; u < linesLength; u += 1) { - const line = file.highlightedDiffLines[u]; - delete line.text; - } - } - - if (file.highlightedDiffLines) { - showingLines += file.parallelDiffLines.length; - } - Object.assign(file, { - renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, - collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, - }); - } + prepareDiffData(diffData); + const { tree, treeEntries } = generateTreeList(diffData.diffFiles); Object.assign(state, { ...diffData, + tree: sortTree(tree), + treeEntries, }); }, @@ -98,19 +77,104 @@ export default { [types.ADD_COLLAPSED_DIFFS](state, { file, data }) { const normalizedData = convertObjectPropsToCamelCase(data, { deep: true }); + prepareDiffData(normalizedData); const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash); - - if (newFileData) { - const index = _.findIndex(state.diffFiles, f => f.fileHash === file.fileHash); - state.diffFiles.splice(index, 1, newFileData); - } + const selectedFile = state.diffFiles.find(f => f.fileHash === file.fileHash); + Object.assign(selectedFile, { ...newFileData }); }, [types.EXPAND_ALL_FILES](state) { - // eslint-disable-next-line no-param-reassign state.diffFiles = state.diffFiles.map(file => ({ ...file, collapsed: false, })); }, + + [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 && + 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, + }); + } + } + + if (selectedFile.highlightedDiffLines) { + const targetInlineLine = selectedFile.highlightedDiffLines.find( + line => line.lineCode === firstDiscussion.line_code, + ); + + if (targetInlineLine) { + Object.assign(targetInlineLine, { + discussions, + }); + } + } + } + }, + + [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) { + const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); + if (selectedFile) { + const targetLine = selectedFile.parallelDiffLines.find( + line => + (line.left && line.left.lineCode === lineCode) || + (line.right && line.right.lineCode === lineCode), + ); + if (targetLine) { + const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right'; + + Object.assign(targetLine[side], { + discussions: [], + }); + } + + if (selectedFile.highlightedDiffLines) { + const targetInlineLine = selectedFile.highlightedDiffLines.find( + line => line.lineCode === lineCode, + ); + + if (targetInlineLine) { + Object.assign(targetInlineLine, { + discussions: [], + }); + } + } + } + }, + [types.TOGGLE_FOLDER_OPEN](state, path) { + state.treeEntries[path].opened = !state.treeEntries[path].opened; + }, + [types.TOGGLE_SHOW_TREE_LIST](state) { + state.showTreeList = !state.showTreeList; + }, + [types.UPDATE_CURRENT_DIFF_FILE_ID](state, fileId) { + state.currentDiffFileId = fileId; + }, }; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 82082ac508a..a482a2b82c0 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -4,10 +4,13 @@ import { LINE_POSITION_LEFT, LINE_POSITION_RIGHT, TEXT_DIFF_POSITION_TYPE, + LEGACY_DIFF_NOTE_TYPE, DIFF_NOTE_TYPE, NEW_LINE_TYPE, OLD_LINE_TYPE, MATCH_LINE_TYPE, + LINES_TO_BE_RENDERED_DIRECTLY, + MAX_LINES_TO_BE_RENDERED, } from '../constants'; export function findDiffFile(files, hash) { @@ -22,7 +25,7 @@ export const getReversePosition = linePosition => { return LINE_POSITION_RIGHT; }; -export function getNoteFormData(params) { +export function getFormData(params) { const { note, noteableType, @@ -52,20 +55,30 @@ export function getNoteFormData(params) { note_project_id: '', target_type: noteableData.targetType, target_id: noteableData.id, + return_discussion: true, note: { note, position, noteable_type: noteableType, noteable_id: noteableData.id, commit_id: '', - type: DIFF_NOTE_TYPE, + type: + diffFile.diffRefs.startSha && diffFile.diffRefs.headSha + ? DIFF_NOTE_TYPE + : LEGACY_DIFF_NOTE_TYPE, line_code: noteTargetLine.lineCode, }, }; + return postData; +} + +export function getNoteFormData(params) { + const data = getFormData(params); + return { - endpoint: noteableData.create_note_path, - data: postData, + endpoint: params.noteableData.create_note_path, + data, }; } @@ -161,6 +174,11 @@ export function addContextLines(options) { * @returns {Object} */ export function trimFirstCharOfLineContent(line = {}) { + // eslint-disable-next-line no-param-reassign + delete line.text; + // eslint-disable-next-line no-param-reassign + line.discussions = []; + const parsedLine = Object.assign({}, line); if (line.richText) { @@ -174,7 +192,44 @@ export function trimFirstCharOfLineContent(line = {}) { return parsedLine; } -export function getDiffRefsByLineCode(diffFiles) { +// This prepares and optimizes the incoming diff data from the server +// by setting up incremental rendering and removing unneeded data +export function prepareDiffData(diffData) { + const filesLength = diffData.diffFiles.length; + let showingLines = 0; + for (let i = 0; i < filesLength; i += 1) { + const file = diffData.diffFiles[i]; + + if (file.parallelDiffLines) { + const linesLength = file.parallelDiffLines.length; + for (let u = 0; u < linesLength; u += 1) { + const line = file.parallelDiffLines[u]; + if (line.left) { + line.left = trimFirstCharOfLineContent(line.left); + } + if (line.right) { + line.right = trimFirstCharOfLineContent(line.right); + } + } + } + + if (file.highlightedDiffLines) { + const linesLength = file.highlightedDiffLines.length; + for (let u = 0; u < linesLength; u += 1) { + const line = file.highlightedDiffLines[u]; + Object.assign(line, { ...trimFirstCharOfLineContent(line) }); + } + showingLines += file.parallelDiffLines.length; + } + + Object.assign(file, { + renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, + collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, + }); + } +} + +export function getDiffPositionByLineCode(diffFiles) { return diffFiles.reduce((acc, diffFile) => { const { baseSha, headSha, startSha } = diffFile.diffRefs; const { newPath, oldPath } = diffFile; @@ -186,7 +241,17 @@ export function getDiffRefsByLineCode(diffFiles) { const { lineCode, oldLine, newLine } = line; if (lineCode) { - acc[lineCode] = { baseSha, headSha, startSha, newPath, oldPath, oldLine, newLine }; + acc[lineCode] = { + baseSha, + headSha, + startSha, + newPath, + oldPath, + oldLine, + newLine, + lineCode, + positionType: 'text', + }; } }); } @@ -194,3 +259,64 @@ export function getDiffRefsByLineCode(diffFiles) { return acc; }, {}); } + +// This method will check whether the discussion is still applicable +// to the diff line in question regarding different versions of the MR +export function isDiscussionApplicableToLine({ discussion, diffPosition, latestDiff }) { + const { lineCode, ...diffPositionCopy } = diffPosition; + + if (discussion.original_position && discussion.position) { + const originalRefs = convertObjectPropsToCamelCase(discussion.original_position); + const refs = convertObjectPropsToCamelCase(discussion.position); + + return _.isEqual(refs, diffPositionCopy) || _.isEqual(originalRefs, diffPositionCopy); + } + + return latestDiff && discussion.active && lineCode === discussion.line_code; +} + +export const generateTreeList = files => + files.reduce( + (acc, file) => { + const { fileHash, addedLines, removedLines, newFile, deletedFile, newPath } = file; + const split = newPath.split('/'); + + split.forEach((name, i) => { + const parent = acc.treeEntries[split.slice(0, i).join('/')]; + const path = `${parent ? `${parent.path}/` : ''}${name}`; + + if (!acc.treeEntries[path]) { + const type = path === newPath ? 'blob' : 'tree'; + acc.treeEntries[path] = { + key: path, + path, + name, + type, + tree: [], + }; + + const entry = acc.treeEntries[path]; + + if (type === 'blob') { + Object.assign(entry, { + changed: true, + tempFile: newFile, + deleted: deletedFile, + fileHash, + addedLines, + removedLines, + }); + } else { + Object.assign(entry, { + opened: true, + }); + } + + (parent ? parent.tree : acc.tree).push(entry); + } + }); + + return acc; + }, + { treeEntries: {}, tree: [] }, + ); |