summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhil Hughes <me@iamphill.com>2019-01-08 09:31:23 +0000
committerPhil Hughes <me@iamphill.com>2019-01-08 09:31:23 +0000
commit12edecd002163e7dedff6fcdf10043b7d1967962 (patch)
tree45bd3c1eee15e911fd43733f1af77a10e4f0fc98
parent1d2ef4c6557846eb531f4d0e80cf43dea99b037b (diff)
downloadgitlab-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
-rw-r--r--app/assets/javascripts/diffs/components/tree_list.vue28
-rw-r--r--app/assets/javascripts/diffs/store/getters.js19
-rw-r--r--app/assets/javascripts/diffs/store/utils.js1
-rw-r--r--app/assets/javascripts/lib/utils/text_utility.js23
-rw-r--r--app/assets/javascripts/vue_shared/components/file_row.vue44
-rw-r--r--app/assets/javascripts/vue_shared/components/file_row_header.vue25
-rw-r--r--changelogs/unreleased/mr-file-tree-blob-truncate-improvements.yml5
-rw-r--r--spec/frontend/vue_shared/components/__snapshots__/file_row_header_spec.js.snap37
-rw-r--r--spec/frontend/vue_shared/components/file_row_header_spec.js36
-rw-r--r--spec/javascripts/diffs/components/tree_list_spec.js5
-rw-r--r--spec/javascripts/diffs/store/getters_spec.js17
-rw-r--r--spec/javascripts/diffs/store/utils_spec.js4
-rw-r--r--spec/javascripts/lib/utils/text_utility_spec.js16
-rw-r--r--spec/javascripts/vue_shared/components/file_row_spec.js43
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);
});
});