diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2018-10-03 10:30:15 +0000 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2018-10-03 10:30:15 +0000 |
commit | c375171bfd4b25d8cf4d7a95e2c65c655c7d647c (patch) | |
tree | 36989aaa96c9da61dc8dc7c6e724749353ec4779 | |
parent | a5cfacc281855e3d2f1da4b08d4579a089c3d311 (diff) | |
parent | 33c4c5b8f30c07ff30de4cd26494becd3ad058c0 (diff) | |
download | gitlab-ce-c375171bfd4b25d8cf4d7a95e2c65c655c7d647c.tar.gz |
Merge branch 'mr-file-tree-data' into 'master'
Merge Request file tree
Closes #14249
See merge request gitlab-org/gitlab-ce!21833
44 files changed, 1038 insertions, 556 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index fc41ee4b777..e60c53338fe 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -5,22 +5,22 @@ import { __ } from '~/locale'; import createFlash from '~/flash'; import eventHub from '../../notes/event_hub'; 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, CompareVersions, - ChangedFiles, DiffFile, NoChanges, HiddenFilesWarning, CommitWidget, + TreeList, }, props: { endpoint: { @@ -58,6 +58,7 @@ export default { plainDiffPath: state => state.diffs.plainDiffPath, emailPatchPath: state => state.diffs.emailPatchPath, }), + ...mapState('diffs', ['showTreeList']), ...mapGetters('diffs', ['isParallelView']), ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), targetBranch() { @@ -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 }); @@ -152,10 +158,11 @@ export default { } }, 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); + }); } }, }, @@ -177,7 +184,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" @@ -215,22 +222,26 @@ export default { :commit="commit" /> - <changed-files - :diff-files="diffFiles" - /> - - <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 0ec6b8b7f21..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.stop.prevent="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/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_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index bcbe374a90c..4e04e50c52a 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -1,5 +1,5 @@ <script> -import { mapActions, mapGetters } from 'vuex'; +import { mapActions, mapGetters, mapState } from 'vuex'; import _ from 'underscore'; import { __, sprintf } from '~/locale'; import createFlash from '~/flash'; @@ -28,6 +28,7 @@ export default { }; }, computed: { + ...mapState('diffs', ['currentDiffFileId']), ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), isCollapsed() { return this.file.collapsed || false; @@ -101,6 +102,9 @@ export default { <template> <div :id="file.fileHash" + :class="{ + 'is-active': currentDiffFileId === file.fileHash + }" class="diff-file file-holder" > <diff-file-header @@ -168,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/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/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 2795dddfc48..6a50d2c1426 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -29,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 98d8d5943f9..1e0b27b538d 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -12,6 +12,7 @@ import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE, DIFF_VIEW_COOKIE_NAME, + MR_TREE_SHOW_KEY, } from '../constants'; export const setBaseConfig = ({ commit }, options) => { @@ -195,5 +196,23 @@ export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { .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 968ba3c5e13..d4c205882ff 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -110,5 +110,9 @@ export const shouldRenderInlineCommentRow = state => line => { 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 eb596b251c1..ae8930c8968 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -1,10 +1,11 @@ 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, @@ -17,4 +18,8 @@ export default () => ({ mergeRequestDiff: null, diffLineCommentForms: {}, diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType, + tree: [], + treeEntries: {}, + showTreeList: storedTreeShow === null ? true : storedTreeShow === 'true', + currentDiffFileId: '', }); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index f61efbe6e1e..6474ee628e2 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -11,3 +11,6 @@ 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 59a2c09e54f..0b4485ecdb5 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -1,5 +1,6 @@ import Vue from 'vue'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; +import { sortTree } from '~/ide/stores/utils'; import { findDiffFile, addLineReferences, @@ -7,6 +8,7 @@ import { addContextLines, prepareDiffData, isDiscussionApplicableToLine, + generateTreeList, } from './utils'; import * as types from './mutation_types'; @@ -23,9 +25,12 @@ export default { [types.SET_DIFF_DATA](state, data) { const diffData = convertObjectPropsToCamelCase(data, { deep: true }); prepareDiffData(diffData); + const { tree, treeEntries } = generateTreeList(diffData.diffFiles); Object.assign(state, { ...diffData, + tree: sortTree(tree), + treeEntries, }); }, @@ -163,4 +168,13 @@ export default { } } }, + [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 631e3de311e..4ae588042e4 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -267,3 +267,49 @@ export function isDiscussionApplicableToLine({ discussion, diffPosition, latestD 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: [] }, + ); diff --git a/app/assets/javascripts/ide/components/commit_sidebar/editor_header.vue b/app/assets/javascripts/ide/components/commit_sidebar/editor_header.vue index 3aca38399fb..b0e60edcbe5 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/editor_header.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/editor_header.vue @@ -3,7 +3,7 @@ import $ from 'jquery'; import { mapActions } from 'vuex'; import { __ } from '~/locale'; import FileIcon from '~/vue_shared/components/file_icon.vue'; -import ChangedFileIcon from '../changed_file_icon.vue'; +import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue'; export default { components: { diff --git a/app/assets/javascripts/ide/components/file_finder/item.vue b/app/assets/javascripts/ide/components/file_finder/item.vue index a612739d641..72ce37be63a 100644 --- a/app/assets/javascripts/ide/components/file_finder/item.vue +++ b/app/assets/javascripts/ide/components/file_finder/item.vue @@ -1,7 +1,7 @@ <script> import fuzzaldrinPlus from 'fuzzaldrin-plus'; import FileIcon from '../../../vue_shared/components/file_icon.vue'; -import ChangedFileIcon from '../changed_file_icon.vue'; +import ChangedFileIcon from '../../../vue_shared/components/changed_file_icon.vue'; const MAX_PATH_LENGTH = 60; diff --git a/app/assets/javascripts/ide/components/file_row_extra.vue b/app/assets/javascripts/ide/components/file_row_extra.vue index 44a360ab909..2ad14b88410 100644 --- a/app/assets/javascripts/ide/components/file_row_extra.vue +++ b/app/assets/javascripts/ide/components/file_row_extra.vue @@ -3,8 +3,8 @@ import { mapGetters } from 'vuex'; import { n__, __, sprintf } from '~/locale'; import tooltip from '~/vue_shared/directives/tooltip'; import Icon from '~/vue_shared/components/icon.vue'; +import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue'; import NewDropdown from './new_dropdown/index.vue'; -import ChangedFileIcon from './changed_file_icon.vue'; import MrFileIcon from './mr_file_icon.vue'; export default { diff --git a/app/assets/javascripts/ide/components/repo_tab.vue b/app/assets/javascripts/ide/components/repo_tab.vue index db47b75ec5c..d621653d6fd 100644 --- a/app/assets/javascripts/ide/components/repo_tab.vue +++ b/app/assets/javascripts/ide/components/repo_tab.vue @@ -3,8 +3,8 @@ import { mapActions } from 'vuex'; import FileIcon from '~/vue_shared/components/file_icon.vue'; import Icon from '~/vue_shared/components/icon.vue'; +import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue'; import FileStatusIcon from './repo_file_status_icon.vue'; -import ChangedFileIcon from './changed_file_icon.vue'; export default { components: { diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 31faa11ea72..e14fff7a610 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -88,6 +88,7 @@ export const handleLocationHash = () => { const fixedDiffStats = document.querySelector('.js-diff-files-changed'); const fixedNav = document.querySelector('.navbar-gitlab'); const performanceBar = document.querySelector('#js-peek'); + const topPadding = 8; let adjustment = 0; if (fixedNav) adjustment -= fixedNav.offsetHeight; @@ -108,6 +109,10 @@ export const handleLocationHash = () => { adjustment -= performanceBar.offsetHeight; } + if (isInMRPage()) { + adjustment -= topPadding; + } + window.scrollBy(0, adjustment); }; @@ -381,8 +386,11 @@ export const objectToQueryString = (params = {}) => .map(param => `${param}=${params[param]}`) .join('&'); -export const buildUrlWithCurrentLocation = param => - (param ? `${window.location.pathname}${param}` : window.location.pathname); +export const buildUrlWithCurrentLocation = param => { + if (param) return `${window.location.pathname}${param}`; + + return window.location.pathname; +}; /** * Based on the current location and the string parameters provided diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 763429d7242..78f56ab57ff 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -194,9 +194,7 @@ export default class MergeRequestTabs { if (bp.getBreakpointSize() !== 'lg') { this.shrinkView(); } - if (this.diffViewType() === 'parallel') { - this.expandViewContainer(); - } + this.expandViewContainer(); this.destroyPipelinesView(); this.commitsTab.classList.remove('active'); } else if (action === 'pipelines') { @@ -355,7 +353,7 @@ export default class MergeRequestTabs { localTimeAgo($('.js-timeago', 'div#diffs')); syntaxHighlight($('#diffs .js-syntax-highlight')); - if (this.diffViewType() === 'parallel' && this.isDiffAction(this.currentAction)) { + if (this.isDiffAction(this.currentAction)) { this.expandViewContainer(); } this.diffsLoaded = true; @@ -408,19 +406,23 @@ export default class MergeRequestTabs { } diffViewType() { - return $('.inline-parallel-buttons a.active').data('viewType'); + return $('.inline-parallel-buttons button.active').data('viewType'); } isDiffAction(action) { return action === 'diffs' || action === 'new/diffs'; } - expandViewContainer() { + expandViewContainer(removeLimited = true) { const $wrapper = $('.content-wrapper .container-fluid').not('.breadcrumbs'); if (this.fixedLayoutPref === null) { this.fixedLayoutPref = $wrapper.hasClass('container-limited'); } - $wrapper.removeClass('container-limited'); + if (this.diffViewType() === 'parallel' || removeLimited) { + $wrapper.removeClass('container-limited'); + } else { + $wrapper.addClass('container-limited'); + } } resetViewContainer() { diff --git a/app/assets/javascripts/ide/components/changed_file_icon.vue b/app/assets/javascripts/vue_shared/components/changed_file_icon.vue index 720ae11aaa6..8684005e0fb 100644 --- a/app/assets/javascripts/ide/components/changed_file_icon.vue +++ b/app/assets/javascripts/vue_shared/components/changed_file_icon.vue @@ -3,7 +3,7 @@ import tooltip from '~/vue_shared/directives/tooltip'; import Icon from '~/vue_shared/components/icon.vue'; import { pluralize } from '~/lib/utils/text_utility'; import { __, sprintf } from '~/locale'; -import { getCommitIconMap } from '../utils'; +import { getCommitIconMap } from '~/ide/utils'; export default { components: { @@ -32,6 +32,11 @@ export default { required: false, default: false, }, + size: { + type: Number, + required: false, + default: 12, + }, }, computed: { changedIcon() { @@ -42,7 +47,7 @@ export default { return `${getCommitIconMap(this.file).icon}${suffix}`; }, changedIconClass() { - return `ide-${this.changedIcon} float-left`; + return `${this.changedIcon} float-left d-block`; }, tooltipTitle() { if (!this.showTooltip) return undefined; @@ -78,13 +83,30 @@ export default { :title="tooltipTitle" data-container="body" data-placement="right" - class="ide-file-changed-icon" + class="file-changed-icon ml-auto" > <icon v-if="showIcon" :name="changedIcon" - :size="12" + :size="size" :css-classes="changedIconClass" /> </span> </template> + +<style> +.file-addition, +.file-addition-solid { + color: #1aaa55; +} + +.file-modified, +.file-modified-solid { + color: #fc9403; +} + +.file-deletion, +.file-deletion-solid { + color: #db3b21; +} +</style> diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index c797ad62a5d..36a345130c0 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -1,12 +1,14 @@ <script> import Icon from '~/vue_shared/components/icon.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue'; +import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue'; export default { name: 'FileRow', components: { FileIcon, Icon, + ChangedFileIcon, }, props: { file: { @@ -22,6 +24,16 @@ export default { required: false, default: null, }, + hideExtraOnTree: { + type: Boolean, + required: false, + default: false, + }, + showChangedIcon: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { @@ -65,6 +77,9 @@ export default { toggleTreeOpen(path) { this.$emit('toggleTreeOpen', path); }, + clickedFile(path) { + this.$emit('clickFile', path); + }, clickFile() { // Manual Action if a tree is selected/opened if (this.isTree && this.hasUrlAtCurrentRoute()) { @@ -72,6 +87,8 @@ export default { } if (this.$router) this.$router.push(`/project${this.file.url}`); + + if (this.isBlob) this.clickedFile(this.file.path); }, scrollIntoView(isInit = false) { const block = isInit && this.isTree ? 'center' : 'nearest'; @@ -126,17 +143,24 @@ export default { class="file-row-name str-truncated" > <file-icon + v-if="!showChangedIcon || file.type === 'tree'" :file-name="file.name" :loading="file.loading" :folder="isTree" :opened="file.opened" :size="16" /> + <changed-file-icon + v-else + :file="file" + :size="16" + class="append-right-5" + /> {{ file.name }} </span> <component :is="extraComponent" - v-if="extraComponent" + v-if="extraComponent && !(hideExtraOnTree && file.type === 'tree')" :file="file" :mouse-over="mouseOver" /> @@ -148,8 +172,11 @@ export default { :key="childFile.key" :file="childFile" :level="level + 1" + :hide-extra-on-tree="hideExtraOnTree" :extra-component="extraComponent" + :show-changed-icon="showChangedIcon" @toggleTreeOpen="toggleTreeOpen" + @clickFile="clickedFile" /> </template> </div> diff --git a/app/assets/stylesheets/page_bundles/ide.scss b/app/assets/stylesheets/page_bundles/ide.scss index 65f0a0d18e2..07d82e984ba 100644 --- a/app/assets/stylesheets/page_bundles/ide.scss +++ b/app/assets/stylesheets/page_bundles/ide.scss @@ -517,21 +517,6 @@ $ide-commit-header-height: 48px; } } -.ide-file-addition, -.ide-file-addition-solid { - color: $green-500; -} - -.ide-file-modified, -.ide-file-modified-solid { - color: $orange-500; -} - -.ide-file-deletion, -.ide-file-deletion-solid { - color: $red-500; -} - .multi-file-commit-list-collapsed { display: flex; flex-direction: column; @@ -1399,14 +1384,6 @@ $ide-commit-header-height: 48px; color: $theme-gray-700; } -.ide-file-changed-icon { - margin-left: auto; - - > svg { - display: block; - } -} - .file-row:hover, .file-row:focus { .ide-new-btn { diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 987dcd32e3a..5035714b95f 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -571,8 +571,6 @@ } .files { - margin-top: 1px; - .diff-file:last-child { margin-bottom: 0; } @@ -987,3 +985,63 @@ .discussion-body .image .frame { position: relative; } + +.diff-tree-list { + width: 320px; +} + +.diff-files-holder { + flex: 1; + min-width: 0; +} + +.compare-versions-container { + min-width: 0; +} + +.tree-list-holder { + position: sticky; + top: 100px; + max-height: calc(100vh - 100px); + padding-right: $gl-padding; + + .file-row { + margin-left: 0; + margin-right: 0; + } + + .with-performance-bar & { + top: 135px; + } +} + +.tree-list-scroll { + max-height: 100%; + padding-top: $grid-size; + padding-bottom: $grid-size; + border-top: 1px solid $border-color; + border-bottom: 1px solid $border-color; + overflow-y: scroll; + overflow-x: auto; +} + +.tree-list-search .form-control { + padding-left: 30px; +} + +.tree-list-icon { + top: 50%; + left: 10px; + transform: translateY(-50%); + + &, + svg { + fill: $gl-text-color-tertiary; + } +} + +.tree-list-clear-icon { + right: 10px; + left: auto; + line-height: 0; +} diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 97b131687d3..45382d4ea43 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -723,6 +723,17 @@ align-items: center; padding: 16px; z-index: 199; + white-space: nowrap; + + .dropdown-menu-toggle { + width: auto; + max-width: 170px; + + svg { + top: 10px; + right: 8px; + } + } } .content-block { diff --git a/changelogs/unreleased/mr-file-tree-data.yml b/changelogs/unreleased/mr-file-tree-data.yml new file mode 100644 index 00000000000..a82087ea148 --- /dev/null +++ b/changelogs/unreleased/mr-file-tree-data.yml @@ -0,0 +1,5 @@ +--- +title: Added tree of changed files to merge request diffs +merge_request: 21833 +author: +type: added diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 646397b7757..a256d87dfa8 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -19,6 +19,11 @@ msgstr "" msgid " Status" msgstr "" +msgid "%d addition" +msgid_plural "%d additions" +msgstr[0] "" +msgstr[1] "" + msgid "%d changed file" msgid_plural "%d changed files" msgstr[0] "" @@ -34,6 +39,11 @@ msgid_plural "%d commits behind" msgstr[0] "" msgstr[1] "" +msgid "%d deleted" +msgid_plural "%d deletions" +msgstr[0] "" +msgstr[1] "" + msgid "%d exporter" msgid_plural "%d exporters" msgstr[0] "" @@ -1281,6 +1291,9 @@ msgstr "" msgid "CircuitBreakerApiLink|circuitbreaker api" msgstr "" +msgid "Clear search" +msgstr "" + msgid "Clear search input" msgstr "" @@ -3738,6 +3751,12 @@ msgstr "" msgid "MergeRequest| %{paragraphStart}changed the description %{descriptionChangedTimes} times %{timeDifferenceMinutes}%{paragraphEnd}" msgstr "" +msgid "MergeRequest|Filter files" +msgstr "" + +msgid "MergeRequest|No files found" +msgstr "" + msgid "Merged" msgstr "" @@ -3998,9 +4017,6 @@ msgstr "" msgid "No file chosen" msgstr "" -msgid "No files found" -msgstr "" - msgid "No files found." msgstr "" @@ -6397,6 +6413,9 @@ msgstr "" msgid "Toggle discussion" msgstr "" +msgid "Toggle file browser" +msgstr "" + msgid "Toggle navigation" msgstr "" diff --git a/spec/features/merge_request/user_comments_on_diff_spec.rb b/spec/features/merge_request/user_comments_on_diff_spec.rb index 441b080bee5..00cf368e8c9 100644 --- a/spec/features/merge_request/user_comments_on_diff_spec.rb +++ b/spec/features/merge_request/user_comments_on_diff_spec.rb @@ -28,7 +28,7 @@ describe 'User comments on a diff', :js do click_button('Comment') end - page.within('.files > div:nth-child(3)') do + page.within('.diff-files-holder > div:nth-child(3)') do expect(page).to have_content('Line is wrong') find('.js-btn-vue-toggle-comments').click @@ -49,7 +49,7 @@ describe 'User comments on a diff', :js do wait_for_requests - page.within('.files > div:nth-child(2) .note-body > .note-text') do + page.within('.diff-files-holder > div:nth-child(2) .note-body > .note-text') do expect(page).to have_content('Line is correct') end @@ -63,7 +63,7 @@ describe 'User comments on a diff', :js do wait_for_requests # Hide the comment. - page.within('.files > div:nth-child(3)') do + page.within('.diff-files-holder > div:nth-child(3)') do find('.js-btn-vue-toggle-comments').click expect(page).not_to have_content('Line is wrong') @@ -71,21 +71,21 @@ describe 'User comments on a diff', :js do # At this moment a user should see only one comment. # The other one should be hidden. - page.within('.files > div:nth-child(2) .note-body > .note-text') do + page.within('.diff-files-holder > div:nth-child(2) .note-body > .note-text') do expect(page).to have_content('Line is correct') end # Show the comment. - page.within('.files > div:nth-child(3)') do + page.within('.diff-files-holder > div:nth-child(3)') do find('.js-btn-vue-toggle-comments').click end # Now both the comments should be shown. - page.within('.files > div:nth-child(3) .note-body > .note-text') do + page.within('.diff-files-holder > div:nth-child(3) .note-body > .note-text') do expect(page).to have_content('Line is wrong') end - page.within('.files > div:nth-child(2) .note-body > .note-text') do + page.within('.diff-files-holder > div:nth-child(2) .note-body > .note-text') do expect(page).to have_content('Line is correct') end @@ -95,11 +95,11 @@ describe 'User comments on a diff', :js do wait_for_requests - page.within('.files > div:nth-child(3) .parallel .note-body > .note-text') do + page.within('.diff-files-holder > div:nth-child(3) .parallel .note-body > .note-text') do expect(page).to have_content('Line is wrong') end - page.within('.files > div:nth-child(2) .parallel .note-body > .note-text') do + page.within('.diff-files-holder > div:nth-child(2) .parallel .note-body > .note-text') do expect(page).to have_content('Line is correct') end end diff --git a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb index 428eb414274..d3da8cc6752 100644 --- a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb +++ b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb @@ -81,6 +81,8 @@ describe 'Merge request > User sees avatars on diff notes', :js do visit diffs_project_merge_request_path(project, merge_request, view: view) wait_for_requests + + find('.js-toggle-tree-list').click end it 'shows note avatar' do diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb index f42b4dcbb47..92db4f44098 100644 --- a/spec/features/merge_request/user_sees_versions_spec.rb +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -110,7 +110,8 @@ describe 'Merge request > User sees versions', :js do diff_id: merge_request_diff3.id, start_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' ) - expect(page).to have_content '4 changed files with 15 additions and 6 deletions' + expect(page).to have_content '4 changed files' + expect(page).to have_content '15 additions 6 deletions' expect(page).to have_content 'Not all comments are displayed' position = Gitlab::Diff::Position.new( @@ -131,7 +132,8 @@ describe 'Merge request > User sees versions', :js do end it 'show diff between new and old version' do - expect(page).to have_content '4 changed files with 15 additions and 6 deletions' + expect(page).to have_content '4 changed files' + expect(page).to have_content '15 additions 6 deletions' end it 'returns to latest version when "Show latest version" button is clicked' do @@ -158,7 +160,7 @@ describe 'Merge request > User sees versions', :js do it 'has 0 chages between versions' do page.within '.mr-version-compare-dropdown' do - expect(find('.dropdown-toggle')).to have_content 'version 1' + expect(find('.dropdown-menu-toggle')).to have_content 'version 1' end page.within '.mr-version-dropdown' do @@ -179,7 +181,7 @@ describe 'Merge request > User sees versions', :js do it 'sets the compared versions to be the same' do page.within '.mr-version-compare-dropdown' do - expect(find('.dropdown-toggle')).to have_content 'version 2' + expect(find('.dropdown-menu-toggle')).to have_content 'version 2' end page.within '.mr-version-dropdown' do diff --git a/spec/features/merge_request/user_views_diffs_spec.rb b/spec/features/merge_request/user_views_diffs_spec.rb index b1bfe9e5de3..7f95a1282f9 100644 --- a/spec/features/merge_request/user_views_diffs_spec.rb +++ b/spec/features/merge_request/user_views_diffs_spec.rb @@ -10,6 +10,8 @@ describe 'User views diffs', :js do visit(diffs_project_merge_request_path(project, merge_request)) wait_for_requests + + find('.js-toggle-tree-list').click end shared_examples 'unfold diffs' do diff --git a/spec/javascripts/diffs/components/app_spec.js b/spec/javascripts/diffs/components/app_spec.js index cf7d8df5405..a3a714678af 100644 --- a/spec/javascripts/diffs/components/app_spec.js +++ b/spec/javascripts/diffs/components/app_spec.js @@ -44,7 +44,8 @@ describe('diffs/components/app', () => { it('shows comments message, with commit', done => { vm.$store.state.diffs.commit = getDiffWithCommit().commit; - vm.$nextTick() + vm + .$nextTick() .then(() => { expect(vm.$el).toContainText('Only comments from the following commit are shown below'); expect(vm.$el).toContainElement('.blob-commit-info'); @@ -55,10 +56,14 @@ describe('diffs/components/app', () => { it('shows comments message, with old mergeRequestDiff', done => { vm.$store.state.diffs.mergeRequestDiff = { latest: false }; + vm.$store.state.diffs.targetBranch = 'master'; - vm.$nextTick() + vm + .$nextTick() .then(() => { - expect(vm.$el).toContainText("Not all comments are displayed because you're viewing an old version of the diff."); + expect(vm.$el).toContainText( + "Not all comments are displayed because you're viewing an old version of the diff.", + ); }) .then(done) .catch(done.fail); @@ -67,9 +72,12 @@ describe('diffs/components/app', () => { it('shows comments message, with startVersion', done => { vm.$store.state.diffs.startVersion = 'test'; - vm.$nextTick() + vm + .$nextTick() .then(() => { - expect(vm.$el).toContainText("Not all comments are displayed because you're comparing two versions of the diff."); + expect(vm.$el).toContainText( + "Not all comments are displayed because you're comparing two versions of the diff.", + ); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/diffs/components/changed_files_spec.js b/spec/javascripts/diffs/components/changed_files_spec.js deleted file mode 100644 index 7f21273a991..00000000000 --- a/spec/javascripts/diffs/components/changed_files_spec.js +++ /dev/null @@ -1,105 +0,0 @@ -import Vue from 'vue'; -import Vuex from 'vuex'; -import { mountComponentWithStore } from 'spec/helpers'; -import diffsModule from '~/diffs/store/modules'; -import changedFiles from '~/diffs/components/changed_files.vue'; - -describe('ChangedFiles', () => { - const Component = Vue.extend(changedFiles); - const store = new Vuex.Store({ - modules: { - diffs: diffsModule(), - }, - }); - - let vm; - - beforeEach(() => { - setFixtures(` - <div id="dummy-element"></div> - <div class="js-tabs-affix"></div> - `); - - const props = { - diffFiles: [ - { - addedLines: 10, - removedLines: 20, - blob: { - path: 'some/code.txt', - }, - filePath: 'some/code.txt', - }, - ], - }; - - vm = mountComponentWithStore(Component, { props, store }); - }); - - describe('with single file added', () => { - it('shows files changes', () => { - expect(vm.$el).toContainText('1 changed file'); - }); - - it('shows file additions and deletions', () => { - expect(vm.$el).toContainText('10 additions'); - expect(vm.$el).toContainText('20 deletions'); - }); - }); - - describe('diff view mode buttons', () => { - let inlineButton; - let parallelButton; - - beforeEach(() => { - inlineButton = vm.$el.querySelector('.js-inline-diff-button'); - parallelButton = vm.$el.querySelector('.js-parallel-diff-button'); - }); - - it('should have Inline and Side-by-side buttons', () => { - expect(inlineButton).toBeDefined(); - expect(parallelButton).toBeDefined(); - }); - - it('should add active class to Inline button', done => { - vm.$store.state.diffs.diffViewType = 'inline'; - - vm.$nextTick(() => { - expect(inlineButton.classList.contains('active')).toEqual(true); - expect(parallelButton.classList.contains('active')).toEqual(false); - - done(); - }); - }); - - it('should toggle active state of buttons when diff view type changed', done => { - vm.$store.state.diffs.diffViewType = 'parallel'; - - vm.$nextTick(() => { - expect(inlineButton.classList.contains('active')).toEqual(false); - expect(parallelButton.classList.contains('active')).toEqual(true); - - done(); - }); - }); - - describe('clicking them', () => { - it('should toggle the diff view type', done => { - parallelButton.click(); - - vm.$nextTick(() => { - expect(inlineButton.classList.contains('active')).toEqual(false); - expect(parallelButton.classList.contains('active')).toEqual(true); - - inlineButton.click(); - - vm.$nextTick(() => { - expect(inlineButton.classList.contains('active')).toEqual(true); - expect(parallelButton.classList.contains('active')).toEqual(false); - done(); - }); - }); - }); - }); - }); -}); diff --git a/spec/javascripts/diffs/components/file_row_stats_spec.js b/spec/javascripts/diffs/components/file_row_stats_spec.js new file mode 100644 index 00000000000..a8a7f3f1d82 --- /dev/null +++ b/spec/javascripts/diffs/components/file_row_stats_spec.js @@ -0,0 +1,33 @@ +import Vue from 'vue'; +import FileRowStats from '~/diffs/components/file_row_stats.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; + +describe('Diff file row stats', () => { + let Component; + let vm; + + beforeAll(() => { + Component = Vue.extend(FileRowStats); + }); + + beforeEach(() => { + vm = mountComponent(Component, { + file: { + addedLines: 20, + removedLines: 10, + }, + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders added lines count', () => { + expect(vm.$el.querySelector('.cgreen').textContent).toContain('+20'); + }); + + it('renders removed lines count', () => { + expect(vm.$el.querySelector('.cred').textContent).toContain('-10'); + }); +}); diff --git a/spec/javascripts/diffs/components/tree_list_spec.js b/spec/javascripts/diffs/components/tree_list_spec.js new file mode 100644 index 00000000000..08e25d2004e --- /dev/null +++ b/spec/javascripts/diffs/components/tree_list_spec.js @@ -0,0 +1,120 @@ +import Vue from 'vue'; +import Vuex from 'vuex'; +import TreeList from '~/diffs/components/tree_list.vue'; +import createStore from '~/diffs/store/modules'; +import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; + +describe('Diffs tree list component', () => { + let Component; + let vm; + + beforeAll(() => { + Component = Vue.extend(TreeList); + }); + + beforeEach(() => { + Vue.use(Vuex); + + const store = new Vuex.Store({ + modules: { + diffs: createStore(), + }, + }); + + // Setup initial state + store.state.diffs.addedLines = 10; + store.state.diffs.removedLines = 20; + store.state.diffs.diffFiles.push('test'); + + vm = mountComponentWithStore(Component, { store }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders diff stats', () => { + expect(vm.$el.textContent).toContain('1 changed file'); + expect(vm.$el.textContent).toContain('10 additions'); + expect(vm.$el.textContent).toContain('20 deletions'); + }); + + it('renders empty text', () => { + expect(vm.$el.textContent).toContain('No files found'); + }); + + describe('with files', () => { + beforeEach(done => { + Object.assign(vm.$store.state.diffs.treeEntries, { + 'index.js': { + addedLines: 0, + changed: true, + deleted: false, + fileHash: 'test', + key: 'index.js', + name: 'index.js', + path: 'index.js', + removedLines: 0, + tempFile: true, + type: 'blob', + }, + app: { + key: 'app', + path: 'app', + name: 'app', + type: 'tree', + tree: [], + }, + }); + vm.$store.state.diffs.tree = [ + vm.$store.state.diffs.treeEntries['index.js'], + vm.$store.state.diffs.treeEntries.app, + ]; + + vm.$nextTick(done); + }); + + it('renders tree', () => { + expect(vm.$el.querySelectorAll('.file-row').length).toBe(2); + expect(vm.$el.querySelectorAll('.file-row')[0].textContent).toContain('index.js'); + expect(vm.$el.querySelectorAll('.file-row')[1].textContent).toContain('app'); + }); + + it('filters tree list to blobs matching search', done => { + vm.search = 'index'; + + vm.$nextTick(() => { + expect(vm.$el.querySelectorAll('.file-row').length).toBe(1); + expect(vm.$el.querySelectorAll('.file-row')[0].textContent).toContain('index.js'); + + done(); + }); + }); + + it('calls toggleTreeOpen when clicking folder', () => { + spyOn(vm.$store, 'dispatch').and.stub(); + + vm.$el.querySelectorAll('.file-row')[1].click(); + + expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/toggleTreeOpen', 'app'); + }); + + it('calls scrollToFile when clicking blob', () => { + spyOn(vm.$store, 'dispatch').and.stub(); + + vm.$el.querySelector('.file-row').click(); + + expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/scrollToFile', 'index.js'); + }); + }); + + describe('clearSearch', () => { + it('resets search', () => { + vm.search = 'test'; + + vm.$el.querySelector('.tree-list-clear-icon').click(); + + expect(vm.search).toBe(''); + }); + }); +}); diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index 05b39bad6ea..aacad7a479b 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -22,6 +22,9 @@ import actions, { expandAllFiles, toggleFileDiscussions, saveDiffDiscussion, + toggleTreeOpen, + scrollToFile, + toggleShowTreeList, } from '~/diffs/store/actions'; import * as types from '~/diffs/store/mutation_types'; import { reduceDiscussionsToLineCodes } from '~/notes/stores/utils'; @@ -608,4 +611,88 @@ describe('DiffsStoreActions', () => { .catch(done.fail); }); }); + + describe('toggleTreeOpen', () => { + it('commits TOGGLE_FOLDER_OPEN', done => { + testAction( + toggleTreeOpen, + 'path', + {}, + [{ type: types.TOGGLE_FOLDER_OPEN, payload: 'path' }], + [], + done, + ); + }); + }); + + describe('scrollToFile', () => { + let commit; + + beforeEach(() => { + commit = jasmine.createSpy(); + jasmine.clock().install(); + }); + + afterEach(() => { + jasmine.clock().uninstall(); + }); + + it('updates location hash', () => { + const state = { + treeEntries: { + path: { + fileHash: 'test', + }, + }, + }; + + scrollToFile({ state, commit }, 'path'); + + expect(document.location.hash).toBe('#test'); + }); + + it('commits UPDATE_CURRENT_DIFF_FILE_ID', () => { + const state = { + treeEntries: { + path: { + fileHash: 'test', + }, + }, + }; + + scrollToFile({ state, commit }, 'path'); + + expect(commit).toHaveBeenCalledWith(types.UPDATE_CURRENT_DIFF_FILE_ID, 'test'); + }); + + it('resets currentDiffId after timeout', () => { + const state = { + treeEntries: { + path: { + fileHash: 'test', + }, + }, + }; + + scrollToFile({ state, commit }, 'path'); + + jasmine.clock().tick(1000); + + expect(commit.calls.argsFor(1)).toEqual([types.UPDATE_CURRENT_DIFF_FILE_ID, '']); + }); + }); + + describe('toggleShowTreeList', () => { + it('commits toggle', done => { + testAction(toggleShowTreeList, null, {}, [{ type: types.TOGGLE_SHOW_TREE_LIST }], [], done); + }); + + it('updates localStorage', () => { + spyOn(localStorage, 'setItem'); + + toggleShowTreeList({ commit() {}, state: { showTreeList: true } }); + + expect(localStorage.setItem).toHaveBeenCalledWith('mr_tree_show', true); + }); + }); }); diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js index 4747e437c4e..cfeaaec6980 100644 --- a/spec/javascripts/diffs/store/getters_spec.js +++ b/spec/javascripts/diffs/store/getters_spec.js @@ -291,4 +291,31 @@ describe('Diffs Module Getters', () => { expect(getters.getDiffFileByHash(localState)('123')).toBeUndefined(); }); }); + + describe('allBlobs', () => { + it('returns an array of blobs', () => { + localState.treeEntries = { + file: { + type: 'blob', + }, + tree: { + type: 'tree', + }, + }; + + expect(getters.allBlobs(localState)).toEqual([ + { + type: 'blob', + }, + ]); + }); + }); + + describe('diffFilesLength', () => { + it('returns length of diff files', () => { + localState.diffFiles.push('test', 'test 2'); + + expect(getters.diffFilesLength(localState)).toBe(2); + }); + }); }); diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 9a5d8dfbd15..cc8d5dc4bac 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -1,3 +1,4 @@ +import createState from '~/diffs/store/modules/diff_state'; import mutations from '~/diffs/store/mutations'; import * as types from '~/diffs/store/mutation_types'; import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; @@ -356,4 +357,44 @@ describe('DiffsStoreMutations', () => { expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(0); }); }); + + describe('TOGGLE_FOLDER_OPEN', () => { + it('toggles entry opened prop', () => { + const state = { + treeEntries: { + path: { + opened: false, + }, + }, + }; + + mutations[types.TOGGLE_FOLDER_OPEN](state, 'path'); + + expect(state.treeEntries.path.opened).toBe(true); + }); + }); + + describe('TOGGLE_SHOW_TREE_LIST', () => { + it('toggles showTreeList', () => { + const state = createState(); + + mutations[types.TOGGLE_SHOW_TREE_LIST](state); + + expect(state.showTreeList).toBe(false, 'Failed to toggle showTreeList to false'); + + mutations[types.TOGGLE_SHOW_TREE_LIST](state); + + expect(state.showTreeList).toBe(true, 'Failed to toggle showTreeList to true'); + }); + }); + + describe('UPDATE_CURRENT_DIFF_FILE_ID', () => { + it('updates currentDiffFileId', () => { + const state = createState(); + + mutations[types.UPDATE_CURRENT_DIFF_FILE_ID](state, 'somefileid'); + + expect(state.currentDiffFileId).toBe('somefileid'); + }); + }); }); diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index 897cd1483aa..e660f94c72e 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -421,4 +421,113 @@ describe('DiffsStoreUtils', () => { ).toBe(false); }); }); + + describe('generateTreeList', () => { + let files; + + beforeAll(() => { + files = [ + { + newPath: 'app/index.js', + deletedFile: false, + newFile: false, + removedLines: 10, + addedLines: 0, + fileHash: 'test', + }, + { + newPath: 'app/test/index.js', + deletedFile: false, + newFile: true, + removedLines: 0, + addedLines: 0, + fileHash: 'test', + }, + { + newPath: 'package.json', + deletedFile: true, + newFile: false, + removedLines: 0, + addedLines: 0, + fileHash: 'test', + }, + ]; + }); + + it('creates a tree of files', () => { + const { tree } = utils.generateTreeList(files); + + expect(tree).toEqual([ + { + key: 'app', + path: 'app', + name: 'app', + type: 'tree', + tree: [ + { + addedLines: 0, + changed: true, + deleted: false, + fileHash: 'test', + key: 'app/index.js', + name: 'index.js', + path: 'app/index.js', + removedLines: 10, + tempFile: false, + type: 'blob', + tree: [], + }, + { + key: 'app/test', + path: 'app/test', + name: 'test', + type: 'tree', + opened: true, + tree: [ + { + addedLines: 0, + changed: true, + deleted: false, + fileHash: 'test', + key: 'app/test/index.js', + name: 'index.js', + path: 'app/test/index.js', + removedLines: 0, + tempFile: true, + type: 'blob', + tree: [], + }, + ], + }, + ], + opened: true, + }, + { + key: 'package.json', + path: 'package.json', + name: 'package.json', + type: 'blob', + changed: true, + tempFile: false, + deleted: true, + fileHash: 'test', + addedLines: 0, + removedLines: 0, + tree: [], + }, + ]); + }); + + it('creates flat list of blobs & folders', () => { + const { treeEntries } = utils.generateTreeList(files); + + expect(Object.keys(treeEntries)).toEqual([ + 'app', + 'app/index.js', + 'app/test', + 'app/test/index.js', + 'package.json', + ]); + }); + }); }); diff --git a/spec/javascripts/ide/components/file_row_extra_spec.js b/spec/javascripts/ide/components/file_row_extra_spec.js index 60dabe28045..c93a939ad71 100644 --- a/spec/javascripts/ide/components/file_row_extra_spec.js +++ b/spec/javascripts/ide/components/file_row_extra_spec.js @@ -107,14 +107,14 @@ describe('IDE extra file row component', () => { describe('changes file icon', () => { it('hides when file is not changed', () => { - expect(vm.$el.querySelector('.ide-file-changed-icon')).toBe(null); + expect(vm.$el.querySelector('.file-changed-icon')).toBe(null); }); it('shows when file is changed', done => { vm.file.changed = true; vm.$nextTick(() => { - expect(vm.$el.querySelector('.ide-file-changed-icon')).not.toBe(null); + expect(vm.$el.querySelector('.file-changed-icon')).not.toBe(null); done(); }); @@ -124,7 +124,7 @@ describe('IDE extra file row component', () => { vm.file.staged = true; vm.$nextTick(() => { - expect(vm.$el.querySelector('.ide-file-changed-icon')).not.toBe(null); + expect(vm.$el.querySelector('.file-changed-icon')).not.toBe(null); done(); }); @@ -134,7 +134,7 @@ describe('IDE extra file row component', () => { vm.file.tempFile = true; vm.$nextTick(() => { - expect(vm.$el.querySelector('.ide-file-changed-icon')).not.toBe(null); + expect(vm.$el.querySelector('.file-changed-icon')).not.toBe(null); done(); }); diff --git a/spec/javascripts/ide/components/repo_tab_spec.js b/spec/javascripts/ide/components/repo_tab_spec.js index 278a0753322..3b52f279bf2 100644 --- a/spec/javascripts/ide/components/repo_tab_spec.js +++ b/spec/javascripts/ide/components/repo_tab_spec.js @@ -93,13 +93,13 @@ describe('RepoTab', () => { Vue.nextTick() .then(() => { - expect(vm.$el.querySelector('.ide-file-modified')).toBeNull(); + expect(vm.$el.querySelector('.file-modified')).toBeNull(); vm.$el.dispatchEvent(new Event('mouseout')); }) .then(Vue.nextTick) .then(() => { - expect(vm.$el.querySelector('.ide-file-modified')).not.toBeNull(); + expect(vm.$el.querySelector('.file-modified')).not.toBeNull(); done(); }) diff --git a/spec/javascripts/ide/components/changed_file_icon_spec.js b/spec/javascripts/vue_shared/components/changed_file_icon_spec.js index 7308219f705..5b1038840c7 100644 --- a/spec/javascripts/ide/components/changed_file_icon_spec.js +++ b/spec/javascripts/vue_shared/components/changed_file_icon_spec.js @@ -1,8 +1,8 @@ import Vue from 'vue'; -import changedFileIcon from '~/ide/components/changed_file_icon.vue'; +import changedFileIcon from '~/vue_shared/components/changed_file_icon.vue'; import createComponent from 'spec/helpers/vue_mount_component_helper'; -describe('IDE changed file icon', () => { +describe('Changed file icon', () => { let vm; beforeEach(() => { @@ -33,14 +33,14 @@ describe('IDE changed file icon', () => { }); describe('changedIconClass', () => { - it('includes ide-file-modified when not a temp file', () => { - expect(vm.changedIconClass).toContain('ide-file-modified'); + it('includes file-modified when not a temp file', () => { + expect(vm.changedIconClass).toContain('file-modified'); }); - it('includes ide-file-addition when a temp file', () => { + it('includes file-addition when a temp file', () => { vm.file.tempFile = true; - expect(vm.changedIconClass).toContain('ide-file-addition'); + expect(vm.changedIconClass).toContain('file-addition'); }); }); }); |