diff options
author | Phil Hughes <me@iamphill.com> | 2018-10-08 11:50:22 +0100 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2018-10-23 09:12:36 +0100 |
commit | 2d00e7fce5b33f2a8c89dccd33d5d1758cc846c7 (patch) | |
tree | 146ece7201a5ec88b4c99053f53fb18cc2b9f1b6 | |
parent | 10bb8297ebe5fc01540b20c3fd365234779b6837 (diff) | |
download | gitlab-ce-2d00e7fce5b33f2a8c89dccd33d5d1758cc846c7.tar.gz |
Add list mode to file browser in diffs
This adds toggle buttons to switch between file & tree list.
For file list, it renders the truncated paths with the ellipsis
at the start of the path.
When focusing the input, it hides the toggle buttons.
On blur, the buttons get shown again.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/51859
-rw-r--r-- | app/assets/javascripts/diffs/components/tree_list.vue | 101 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/store/utils.js | 13 | ||||
-rw-r--r-- | app/assets/javascripts/vue_shared/components/file_row.vue | 8 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/diff.scss | 14 | ||||
-rw-r--r-- | changelogs/unreleased/mr-file-list.yml | 5 | ||||
-rw-r--r-- | locale/gitlab.pot | 6 | ||||
-rw-r--r-- | spec/javascripts/diffs/components/tree_list_spec.js | 70 | ||||
-rw-r--r-- | spec/javascripts/diffs/store/utils_spec.js | 28 |
8 files changed, 221 insertions, 24 deletions
diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index cfe4273742f..ea1a73d40cd 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -1,10 +1,14 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; +import { TooltipDirective as Tooltip } from '@gitlab-org/gitlab-ui'; 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 { + directives: { + Tooltip, + }, components: { Icon, FileRow, @@ -12,6 +16,8 @@ export default { data() { return { search: '', + renderTreeList: true, + focusSearch: false, }; }, computed: { @@ -20,16 +26,29 @@ export default { filteredTreeList() { const search = this.search.toLowerCase().trim(); - if (search === '') return this.tree; + if (search === '') return this.renderTreeList ? this.tree : this.allBlobs; return this.allBlobs.filter(f => f.name.toLowerCase().indexOf(search) >= 0); }, + rowDisplayTextKey() { + if (this.renderTreeList && this.search.trim() === '') { + return 'name'; + } + + return 'truncatedPath'; + }, }, methods: { ...mapActions('diffs', ['toggleTreeOpen', 'scrollToFile']), clearSearch() { this.search = ''; }, + toggleRenderTreeList(toggle) { + this.renderTreeList = toggle; + }, + toggleFocusSearch(toggle) { + this.focusSearch = toggle; + }, }, FileRowStats, }; @@ -37,28 +56,67 @@ export default { <template> <div class="tree-list-holder d-flex flex-column"> - <div class="append-bottom-8 position-relative tree-list-search"> - <icon - name="search" - class="position-absolute tree-list-icon" - /> - <input - v-model="search" - :placeholder="s__('MergeRequest|Filter files')" - type="search" - class="form-control" - /> - <button - v-show="search" - :aria-label="__('Clear search')" - type="button" - class="position-absolute tree-list-icon tree-list-clear-icon border-0 p-0" - @click="clearSearch" - > + <div class="append-bottom-8 position-relative tree-list-search d-flex"> + <div class="flex-fill d-flex"> <icon - name="close" + name="search" + class="position-absolute tree-list-icon" + /> + <input + v-model="search" + :placeholder="s__('MergeRequest|Filter files')" + type="search" + class="form-control" + @focus="toggleFocusSearch(true)" + @blur="toggleFocusSearch(false)" /> - </button> + <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 + v-show="!focusSearch" + class="btn-group prepend-left-8 tree-list-view-toggle" + > + <button + v-tooltip.hover + :aria-label="__('Switch to file list')" + :title="__('Switch to file list')" + :class="{ + active: !renderTreeList + }" + class="btn btn-default pt-0 pb-0 d-flex align-items-center" + type="button" + @click="toggleRenderTreeList(false)" + > + <icon + name="hamburger" + /> + </button> + <button + v-tooltip.hover + :aria-label="__('Switch to tree list')" + :title="__('Switch to tree list')" + :class="{ + active: renderTreeList + }" + class="btn btn-default pt-0 pb-0 d-flex align-items-center" + type="button" + @click="toggleRenderTreeList(true)" + > + <icon + name="hamburger" + /> + </button> + </div> </div> <div class="tree-list-scroll" @@ -72,6 +130,7 @@ export default { :hide-extra-on-tree="true" :extra-component="$options.FileRowStats" :show-changed-icon="true" + :display-text-key="rowDisplayTextKey" @toggleTreeOpen="toggleTreeOpen" @clickFile="scrollToFile" /> diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index a482a2b82c0..7c093dcf1fe 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -275,6 +275,18 @@ export function isDiscussionApplicableToLine({ discussion, diffPosition, latestD return latestDiff && discussion.active && lineCode === discussion.line_code; } +export const truncatedName = path => { + const maxLength = 30; + + if (path.length > maxLength) { + const start = path.length - maxLength; + const end = start + maxLength; + return `...${path.slice(start, end)}`; + } + + return path; +}; + export const generateTreeList = files => files.reduce( (acc, file) => { @@ -290,6 +302,7 @@ export const generateTreeList = files => acc.treeEntries[path] = { key: path, path, + truncatedPath: truncatedName(path), name, type, tree: [], diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index 36a345130c0..7c34d776c7f 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -34,6 +34,11 @@ export default { required: false, default: false, }, + displayTextKey: { + type: String, + required: false, + default: 'name', + }, }, data() { return { @@ -156,7 +161,7 @@ export default { :size="16" class="append-right-5" /> - {{ file.name }} + {{ file[displayTextKey] }} </span> <component :is="extraComponent" @@ -175,6 +180,7 @@ export default { :hide-extra-on-tree="hideExtraOnTree" :extra-component="extraComponent" :show-changed-icon="showChangedIcon" + :display-text-key="displayTextKey" @toggleTreeOpen="toggleTreeOpen" @clickFile="clickedFile" /> diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 8d884ad6891..52c91266ff4 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -1027,8 +1027,12 @@ overflow-x: auto; } -.tree-list-search .form-control { - padding-left: 30px; +.tree-list-search { + flex: 0 0 34px; + + .form-control { + padding-left: 30px; + } } .tree-list-icon { @@ -1063,3 +1067,9 @@ } } } + +.tree-list-view-toggle { + svg { + top: 0; + } +} diff --git a/changelogs/unreleased/mr-file-list.yml b/changelogs/unreleased/mr-file-list.yml new file mode 100644 index 00000000000..6596a0dd3e3 --- /dev/null +++ b/changelogs/unreleased/mr-file-list.yml @@ -0,0 +1,5 @@ +--- +title: Switch between tree list & file list in diffs file browser +merge_request: +author: +type: added diff --git a/locale/gitlab.pot b/locale/gitlab.pot index bb18d4eccd8..dfed77dbb36 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5823,6 +5823,12 @@ msgstr "" msgid "Switch branch/tag" msgstr "" +msgid "Switch to file list" +msgstr "" + +msgid "Switch to tree list" +msgstr "" + msgid "System Hooks" msgstr "" diff --git a/spec/javascripts/diffs/components/tree_list_spec.js b/spec/javascripts/diffs/components/tree_list_spec.js index 08e25d2004e..86e2b03292d 100644 --- a/spec/javascripts/diffs/components/tree_list_spec.js +++ b/spec/javascripts/diffs/components/tree_list_spec.js @@ -54,6 +54,7 @@ describe('Diffs tree list component', () => { key: 'index.js', name: 'index.js', path: 'index.js', + truncatedPath: '../index.js', removedLines: 0, tempFile: true, type: 'blob', @@ -106,6 +107,55 @@ describe('Diffs tree list component', () => { expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/scrollToFile', 'index.js'); }); + + it('renders as file list when renderTreeList is false', done => { + vm.renderTreeList = false; + + vm.$nextTick(() => { + expect(vm.$el.querySelectorAll('.file-row').length).toBe(1); + + done(); + }); + }); + + it('renders file paths when renderTreeList is false', done => { + vm.renderTreeList = false; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.file-row').textContent).toContain('../index.js'); + + done(); + }); + }); + + it('hides render buttons when input is focused', done => { + const focusEvent = new Event('focus'); + + vm.$el.querySelector('.form-control').dispatchEvent(focusEvent); + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.tree-list-view-toggle').style.display).toBe('none'); + + done(); + }); + }); + + it('shows render buttons when input is blurred', done => { + const blurEvent = new Event('blur'); + vm.focusSearch = true; + + vm + .$nextTick() + .then(() => { + vm.$el.querySelector('.form-control').dispatchEvent(blurEvent); + }) + .then(vm.$nextTick) + .then(() => { + expect(vm.$el.querySelector('.tree-list-view-toggle').style.display).not.toBe('none'); + }) + .then(done) + .catch(done.fail); + }); }); describe('clearSearch', () => { @@ -117,4 +167,24 @@ describe('Diffs tree list component', () => { expect(vm.search).toBe(''); }); }); + + describe('toggleRenderTreeList', () => { + it('updates renderTreeList', () => { + expect(vm.renderTreeList).toBe(true); + + vm.toggleRenderTreeList(false); + + expect(vm.renderTreeList).toBe(false); + }); + }); + + describe('toggleFocusSearch', () => { + it('updates focusSearch', () => { + expect(vm.focusSearch).toBe(false); + + vm.toggleFocusSearch(true); + + expect(vm.focusSearch).toBe(true); + }); + }); }); diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index ef367fc09fa..6a0a69967fa 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -445,6 +445,14 @@ describe('DiffsStoreUtils', () => { fileHash: 'test', }, { + newPath: 'app/test/filepathneedstruncating.js', + deletedFile: false, + newFile: true, + removedLines: 0, + addedLines: 0, + fileHash: 'test', + }, + { newPath: 'package.json', deletedFile: true, newFile: false, @@ -462,6 +470,7 @@ describe('DiffsStoreUtils', () => { { key: 'app', path: 'app', + truncatedPath: 'app', name: 'app', type: 'tree', tree: [ @@ -473,6 +482,7 @@ describe('DiffsStoreUtils', () => { key: 'app/index.js', name: 'index.js', path: 'app/index.js', + truncatedPath: 'app/index.js', removedLines: 10, tempFile: false, type: 'blob', @@ -481,6 +491,7 @@ describe('DiffsStoreUtils', () => { { key: 'app/test', path: 'app/test', + truncatedPath: 'app/test', name: 'test', type: 'tree', opened: true, @@ -493,6 +504,21 @@ describe('DiffsStoreUtils', () => { key: 'app/test/index.js', name: 'index.js', path: 'app/test/index.js', + truncatedPath: 'app/test/index.js', + removedLines: 0, + tempFile: true, + type: 'blob', + tree: [], + }, + { + addedLines: 0, + changed: true, + deleted: false, + fileHash: 'test', + key: 'app/test/filepathneedstruncating.js', + name: 'filepathneedstruncating.js', + path: 'app/test/filepathneedstruncating.js', + truncatedPath: '...est/filepathneedstruncating.js', removedLines: 0, tempFile: true, type: 'blob', @@ -506,6 +532,7 @@ describe('DiffsStoreUtils', () => { { key: 'package.json', path: 'package.json', + truncatedPath: 'package.json', name: 'package.json', type: 'blob', changed: true, @@ -527,6 +554,7 @@ describe('DiffsStoreUtils', () => { 'app/index.js', 'app/test', 'app/test/index.js', + 'app/test/filepathneedstruncating.js', 'package.json', ]); }); |