diff options
author | Phil Hughes <me@iamphill.com> | 2019-01-08 09:31:23 +0000 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2019-01-08 09:31:23 +0000 |
commit | 12edecd002163e7dedff6fcdf10043b7d1967962 (patch) | |
tree | 45bd3c1eee15e911fd43733f1af77a10e4f0fc98 | |
parent | 1d2ef4c6557846eb531f4d0e80cf43dea99b037b (diff) | |
download | gitlab-ce-12edecd002163e7dedff6fcdf10043b7d1967962.tar.gz |
Add headers to files in the tree list on merge requests
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/54807
14 files changed, 223 insertions, 80 deletions
diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index f40a7b25fde..eb8f274aff3 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -34,14 +34,18 @@ export default { if (search === '') return this.renderTreeList ? this.tree : this.allBlobs; - return this.allBlobs.filter(f => f.path.toLowerCase().indexOf(search) >= 0); - }, - rowDisplayTextKey() { - if (this.renderTreeList && this.search.trim() === '') { - return 'name'; - } + return this.allBlobs.reduce((acc, folder) => { + const tree = folder.tree.filter(f => f.path.toLowerCase().indexOf(search) >= 0); - return 'path'; + if (tree.length) { + return acc.concat({ + ...folder, + tree, + }); + } + + return acc; + }, []); }, }, methods: { @@ -119,7 +123,7 @@ export default { </button> </div> </div> - <div class="tree-list-scroll"> + <div :class="{ 'pt-0 tree-list-blobs': !renderTreeList }" class="tree-list-scroll"> <template v-if="filteredTreeList.length"> <file-row v-for="file in filteredTreeList" @@ -129,8 +133,6 @@ export default { :hide-extra-on-tree="true" :extra-component="$options.FileRowStats" :show-changed-icon="true" - :display-text-key="rowDisplayTextKey" - :should-truncate-start="true" @toggleTreeOpen="toggleTreeOpen" @clickFile="scrollToFile" /> @@ -148,3 +150,9 @@ export default { </div> </div> </template> + +<style> +.tree-list-blobs .file-row-name { + margin-left: 12px; +} +</style> diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index fdf1efbb10e..86c0c7190f9 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -74,7 +74,24 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) = export const getDiffFileByHash = state => fileHash => state.diffFiles.find(file => file.file_hash === fileHash); -export const allBlobs = state => Object.values(state.treeEntries).filter(f => f.type === 'blob'); +export const allBlobs = state => + Object.values(state.treeEntries) + .filter(f => f.type === 'blob') + .reduce((acc, file) => { + const { parentPath } = file; + + if (parentPath && !acc.some(f => f.path === parentPath)) { + acc.push({ + path: parentPath, + isHeader: true, + tree: [], + }); + } + + acc.find(f => f.path === parentPath).tree.push(file); + + return acc; + }, []); export const diffFilesLength = state => state.diffFiles.length; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 2fe20551642..f427367c11e 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -318,6 +318,7 @@ export const generateTreeList = files => fileHash: file.file_hash, addedLines: file.added_lines, removedLines: file.removed_lines, + parentPath: parent ? `${parent.path}/` : '/', }); } else { Object.assign(entry, { diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index 7cc7cd6d20e..c49b1bb5a2f 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -72,6 +72,29 @@ export const truncate = (string, maxLength) => `${string.substr(0, maxLength - 3 */ export const truncateSha = sha => sha.substr(0, 8); +const ELLIPSIS_CHAR = '…'; +export const truncatePathMiddleToLength = (text, maxWidth) => { + let returnText = text; + let ellipsisCount = 0; + + while (returnText.length >= maxWidth) { + const textSplit = returnText.split('/').filter(s => s !== ELLIPSIS_CHAR); + const middleIndex = Math.floor(textSplit.length / 2); + + returnText = textSplit + .slice(0, middleIndex) + .concat( + new Array(ellipsisCount + 1).fill().map(() => ELLIPSIS_CHAR), + textSplit.slice(middleIndex + 1), + ) + .join('/'); + + ellipsisCount += 1; + } + + return returnText; +}; + /** * Capitalizes first character * diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index 9e713673678..4c884c55a30 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -1,11 +1,13 @@ <script> import Icon from '~/vue_shared/components/icon.vue'; +import FileHeader from '~/vue_shared/components/file_row_header.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: { + FileHeader, FileIcon, Icon, ChangedFileIcon, @@ -34,21 +36,10 @@ export default { required: false, default: false, }, - displayTextKey: { - type: String, - required: false, - default: 'name', - }, - shouldTruncateStart: { - type: Boolean, - required: false, - default: false, - }, }, data() { return { mouseOver: false, - truncateStart: 0, }; }, computed: { @@ -60,7 +51,7 @@ export default { }, levelIndentation() { return { - marginLeft: `${this.level * 16}px`, + marginLeft: this.level ? `${this.level * 16}px` : null, }; }, fileClass() { @@ -71,14 +62,8 @@ export default { 'is-open': this.file.opened, }; }, - outputText() { - const text = this.file[this.displayTextKey]; - - if (this.truncateStart === 0) { - return text; - } - - return `...${text.substring(this.truncateStart, text.length)}`; + childFilesLevel() { + return this.file.isHeader ? 0 : this.level + 1; }, }, watch: { @@ -92,15 +77,6 @@ export default { if (this.hasPathAtCurrentRoute()) { this.scrollIntoView(true); } - - if (this.shouldTruncateStart) { - const { scrollWidth, offsetWidth } = this.$refs.textOutput; - const textOverflow = scrollWidth - offsetWidth; - - if (textOverflow > 0) { - this.truncateStart = Math.ceil(textOverflow / 5) + 3; - } - } }, methods: { toggleTreeOpen(path) { @@ -156,7 +132,9 @@ export default { <template> <div> + <file-header v-if="file.isHeader" :path="file.path" /> <div + v-else :class="fileClass" class="file-row" role="button" @@ -175,7 +153,7 @@ export default { :size="16" /> <changed-file-icon v-else :file="file" :size="16" class="append-right-5" /> - {{ outputText }} + {{ file.name }} </span> <component :is="extraComponent" @@ -185,17 +163,15 @@ export default { /> </div> </div> - <template v-if="file.opened"> + <template v-if="file.opened || file.isHeader"> <file-row v-for="childFile in file.tree" :key="childFile.key" :file="childFile" - :level="level + 1" + :level="childFilesLevel" :hide-extra-on-tree="hideExtraOnTree" :extra-component="extraComponent" :show-changed-icon="showChangedIcon" - :display-text-key="displayTextKey" - :should-truncate-start="shouldTruncateStart" @toggleTreeOpen="toggleTreeOpen" @clickFile="clickedFile" /> diff --git a/app/assets/javascripts/vue_shared/components/file_row_header.vue b/app/assets/javascripts/vue_shared/components/file_row_header.vue new file mode 100644 index 00000000000..301fd8116a9 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/file_row_header.vue @@ -0,0 +1,25 @@ +<script> +import { truncatePathMiddleToLength } from '~/lib/utils/text_utility'; + +const MAX_PATH_LENGTH = 40; + +export default { + props: { + path: { + type: String, + required: true, + }, + }, + computed: { + truncatedPath() { + return truncatePathMiddleToLength(this.path, MAX_PATH_LENGTH); + }, + }, +}; +</script> + +<template> + <div class="file-row-header bg-white sticky-top p-2 js-file-row-header"> + <span class="bold">{{ truncatedPath }}</span> + </div> +</template> diff --git a/changelogs/unreleased/mr-file-tree-blob-truncate-improvements.yml b/changelogs/unreleased/mr-file-tree-blob-truncate-improvements.yml new file mode 100644 index 00000000000..b01962591c6 --- /dev/null +++ b/changelogs/unreleased/mr-file-tree-blob-truncate-improvements.yml @@ -0,0 +1,5 @@ +--- +title: Add folder header to files in merge request tree list +merge_request: +author: +type: changed diff --git a/spec/frontend/vue_shared/components/__snapshots__/file_row_header_spec.js.snap b/spec/frontend/vue_shared/components/__snapshots__/file_row_header_spec.js.snap new file mode 100644 index 00000000000..26eae2d5e61 --- /dev/null +++ b/spec/frontend/vue_shared/components/__snapshots__/file_row_header_spec.js.snap @@ -0,0 +1,37 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`File row header component adds multiple ellipsises after 40 characters 1`] = ` +<div + class="file-row-header bg-white sticky-top p-2 js-file-row-header" +> + <span + class="bold" + > + app/assets/javascripts/…/…/diffs/notes + </span> +</div> +`; + +exports[`File row header component renders file path 1`] = ` +<div + class="file-row-header bg-white sticky-top p-2 js-file-row-header" +> + <span + class="bold" + > + app/assets + </span> +</div> +`; + +exports[`File row header component trucates path after 40 characters 1`] = ` +<div + class="file-row-header bg-white sticky-top p-2 js-file-row-header" +> + <span + class="bold" + > + app/assets/javascripts/merge_requests + </span> +</div> +`; diff --git a/spec/frontend/vue_shared/components/file_row_header_spec.js b/spec/frontend/vue_shared/components/file_row_header_spec.js new file mode 100644 index 00000000000..80f4c275dcc --- /dev/null +++ b/spec/frontend/vue_shared/components/file_row_header_spec.js @@ -0,0 +1,36 @@ +import { shallowMount } from '@vue/test-utils'; +import FileRowHeader from '~/vue_shared/components/file_row_header.vue'; + +describe('File row header component', () => { + let vm; + + function createComponent(path) { + vm = shallowMount(FileRowHeader, { + propsData: { + path, + }, + }); + } + + afterEach(() => { + vm.destroy(); + }); + + it('renders file path', () => { + createComponent('app/assets'); + + expect(vm.element).toMatchSnapshot(); + }); + + it('trucates path after 40 characters', () => { + createComponent('app/assets/javascripts/merge_requests'); + + expect(vm.element).toMatchSnapshot(); + }); + + it('adds multiple ellipsises after 40 characters', () => { + createComponent('app/assets/javascripts/merge_requests/widget/diffs/notes'); + + expect(vm.element).toMatchSnapshot(); + }); +}); diff --git a/spec/javascripts/diffs/components/tree_list_spec.js b/spec/javascripts/diffs/components/tree_list_spec.js index a0b380adfd6..0a903bb7519 100644 --- a/spec/javascripts/diffs/components/tree_list_spec.js +++ b/spec/javascripts/diffs/components/tree_list_spec.js @@ -26,6 +26,8 @@ describe('Diffs tree list component', () => { store.state.diffs.removedLines = 20; store.state.diffs.diffFiles.push('test'); + localStorage.removeItem('mr_diff_tree_list'); + vm = mountComponentWithStore(Component, { store }); }); @@ -57,6 +59,7 @@ describe('Diffs tree list component', () => { removedLines: 0, tempFile: true, type: 'blob', + parentPath: 'app', }, app: { key: 'app', @@ -121,7 +124,7 @@ describe('Diffs tree list component', () => { vm.renderTreeList = false; vm.$nextTick(() => { - expect(vm.$el.querySelector('.file-row').textContent).toContain('app/index.js'); + expect(vm.$el.querySelector('.file-row').textContent).toContain('index.js'); done(); }); diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js index 582535e0a53..190ca1230ca 100644 --- a/spec/javascripts/diffs/store/getters_spec.js +++ b/spec/javascripts/diffs/store/getters_spec.js @@ -230,15 +230,30 @@ describe('Diffs Module Getters', () => { localState.treeEntries = { file: { type: 'blob', + path: 'file', + parentPath: '/', + tree: [], }, tree: { type: 'tree', + path: 'tree', + parentPath: '/', + tree: [], }, }; expect(getters.allBlobs(localState)).toEqual([ { - type: 'blob', + isHeader: true, + path: '/', + tree: [ + { + parentPath: '/', + path: 'file', + tree: [], + type: 'blob', + }, + ], }, ]); }); diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index 4268634d302..036b320b314 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -502,6 +502,7 @@ describe('DiffsStoreUtils', () => { fileHash: 'test', key: 'app/index.js', name: 'index.js', + parentPath: 'app/', path: 'app/index.js', removedLines: 10, tempFile: false, @@ -522,6 +523,7 @@ describe('DiffsStoreUtils', () => { fileHash: 'test', key: 'app/test/index.js', name: 'index.js', + parentPath: 'app/test/', path: 'app/test/index.js', removedLines: 0, tempFile: true, @@ -535,6 +537,7 @@ describe('DiffsStoreUtils', () => { fileHash: 'test', key: 'app/test/filepathneedstruncating.js', name: 'filepathneedstruncating.js', + parentPath: 'app/test/', path: 'app/test/filepathneedstruncating.js', removedLines: 0, tempFile: true, @@ -548,6 +551,7 @@ describe('DiffsStoreUtils', () => { }, { key: 'package.json', + parentPath: '/', path: 'package.json', name: 'package.json', type: 'blob', diff --git a/spec/javascripts/lib/utils/text_utility_spec.js b/spec/javascripts/lib/utils/text_utility_spec.js index 92ebfc38722..0a266b19ea5 100644 --- a/spec/javascripts/lib/utils/text_utility_spec.js +++ b/spec/javascripts/lib/utils/text_utility_spec.js @@ -135,4 +135,20 @@ describe('text_utility', () => { expect(textUtils.getFirstCharacterCapitalized(null)).toEqual(''); }); }); + + describe('truncatePathMiddleToLength', () => { + it('does not truncate text', () => { + expect(textUtils.truncatePathMiddleToLength('app/test', 50)).toEqual('app/test'); + }); + + it('truncates middle of the path', () => { + expect(textUtils.truncatePathMiddleToLength('app/test/diff', 13)).toEqual('app/…/diff'); + }); + + it('truncates multiple times in the middle of the path', () => { + expect(textUtils.truncatePathMiddleToLength('app/test/merge_request/diff', 13)).toEqual( + 'app/…/…/diff', + ); + }); + }); }); diff --git a/spec/javascripts/vue_shared/components/file_row_spec.js b/spec/javascripts/vue_shared/components/file_row_spec.js index 67752c1c455..d1fd899c1a8 100644 --- a/spec/javascripts/vue_shared/components/file_row_spec.js +++ b/spec/javascripts/vue_shared/components/file_row_spec.js @@ -3,7 +3,7 @@ import FileRow from '~/vue_shared/components/file_row.vue'; import { file } from 'spec/ide/helpers'; import mountComponent from '../../helpers/vue_mount_component_helper'; -describe('RepoFile', () => { +describe('File row component', () => { let vm; function createComponent(propsData) { @@ -72,39 +72,16 @@ describe('RepoFile', () => { expect(vm.$el.querySelector('.file-row-name').style.marginLeft).toBe('32px'); }); - describe('outputText', () => { - beforeEach(done => { - createComponent({ - file: { - ...file(), - path: 'app/assets/index.js', - }, - level: 0, - }); - - vm.displayTextKey = 'path'; - - vm.$nextTick(done); - }); - - it('returns text if truncateStart is 0', done => { - vm.truncateStart = 0; - - vm.$nextTick(() => { - expect(vm.outputText).toBe('app/assets/index.js'); - - done(); - }); + it('renders header for file', () => { + createComponent({ + file: { + isHeader: true, + path: 'app/assets', + tree: [], + }, + level: 0, }); - it('returns text truncated at start', done => { - vm.truncateStart = 5; - - vm.$nextTick(() => { - expect(vm.outputText).toBe('...ssets/index.js'); - - done(); - }); - }); + expect(vm.$el.querySelector('.js-file-row-header')).not.toBe(null); }); }); |