From 08db570f418b36fb9e05f575b9fea3430cd9a8ed Mon Sep 17 00:00:00 2001 From: jerasmus Date: Thu, 23 Oct 2025 11:14:08 +0200 Subject: [PATCH 1/6] Add expand/collapse chevron to FTB directories Adds a dedicated chevron button next to directories in the file tree browser that allows expanding/collapsing directories without navigating to them. --- .../components/tree_list.vue | 1 + .../vue_shared/components/file_row.vue | 22 +++++++++ app/assets/stylesheets/framework/files.scss | 2 +- locale/gitlab.pot | 6 +++ .../components/tree_list_spec.js | 31 +++++++++++++ .../vue_shared/components/file_row_spec.js | 45 +++++++++++++++++++ 6 files changed, 106 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue b/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue index 1380f332b4407c..78cf90a4252ab4 100644 --- a/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue +++ b/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue @@ -413,6 +413,7 @@ export default { :level="item.level" :opened="item.opened" :loading="item.loading" + :show-tree-toggle="true" :tabindex="item.loading ? -1 : 0" :aria-current="isCurrentPath(item.path)" role="treeitem" diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index aede0f21d15ff2..db3ef7a9702249 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -42,6 +42,11 @@ export default { required: false, default: false, }, + showTreeToggle: { + type: Boolean, + required: false, + default: false, + }, }, computed: { isTree() { @@ -66,6 +71,9 @@ export default { fileRouterUrl() { return this.fileUrl || `/project${this.file.url}`; }, + chevronIcon() { + return this.file.opened ? 'chevron-down' : 'chevron-right'; + }, }, watch: { 'file.active': function fileActiveWatch(active) { @@ -83,6 +91,10 @@ export default { toggleTreeOpen(path) { this.$emit('toggleTreeOpen', path); }, + onChevronClick(event) { + event.stopPropagation(); + this.$emit('clickTree', this.file.path); + }, clickFile() { this.trackEvent('click_file_tree_browser_on_repository_page'); @@ -158,6 +170,16 @@ export default { data-testid="file-row-name-container" :class="[fileClasses, { 'str-truncated': !truncateMiddle, 'gl-min-w-0': truncateMiddle }]" > + + { ); }); }); + + describe('Tree toggle', () => { + it('passes show-tree-toggle="true" prop to all FileRow components', () => { + findFileRows().wrappers.forEach((fileRow) => + expect(fileRow.props('showTreeToggle')).toBe(true), + ); + }); + + it('fetches directory contents when chevron is clicked', async () => { + const subdirResponse = cloneDeep(mockResponse); + subdirResponse.data.project.repository.paginatedTree.nodes[0].blobs.nodes = [ + { + id: 'gid://file1', + name: 'subfile.txt', + path: 'dir_1/dir_2/subfile.txt', + sha: 'xyz789', + webPath: 'dir_1/dir_2/subfile.txt', + }, + ]; + getQueryHandlerSuccess.mockResolvedValueOnce(subdirResponse); + + const treeFileRow = findFileRows().at(0); // First row is the tree based on mockResponse + treeFileRow.vm.$emit('clickTree', treeFileRow.props('file').path); + + await waitForPromises(); + + expect(getQueryHandlerSuccess).toHaveBeenLastCalledWith( + expect.objectContaining({ path: 'dir_1/dir_2' }), + ); + }); + }); }); diff --git a/spec/frontend/vue_shared/components/file_row_spec.js b/spec/frontend/vue_shared/components/file_row_spec.js index 17454bec6f7642..70d660cdd8db07 100644 --- a/spec/frontend/vue_shared/components/file_row_spec.js +++ b/spec/frontend/vue_shared/components/file_row_spec.js @@ -250,4 +250,49 @@ describe('File row component', () => { expect(findShowMoreButton().props('loading')).toBe(true); }); }); + + describe('Tree toggle chevron button', () => { + const findChevronButton = () => wrapper.findComponent(GlButton); + const folderPath = 'path/to/folder'; + const mockFile = { ...file(folderPath), type: 'tree', opened: false }; + + beforeEach(() => { + createComponent({ + file: mockFile, + level: 0, + showTreeToggle: true, + }); + }); + + it('renders chevron button with correct icon and text text', () => { + expect(findChevronButton().props()).toMatchObject({ + category: 'tertiary', + size: 'small', + icon: 'chevron-right', + }); + + expect(findChevronButton().attributes('aria-label')).toBe('Expand directory'); + + // Ensure correct icon and aria-label when folder is expanded + createComponent({ file: { ...mockFile, opened: true }, level: 0, showTreeToggle: true }); + expect(findChevronButton().props('icon')).toBe('chevron-down'); + expect(findChevronButton().attributes('aria-label')).toBe('Collapse directory'); + }); + + it('renders chevron button for trees and emits clickTree when clicked', () => { + findChevronButton().vm.$emit('click', { stopPropagation: jest.fn() }); + + expect(wrapper.emitted('clickTree')[0][0]).toBe(folderPath); + }); + + it('does not render when showTreeToggle is false', () => { + createComponent({ + file: mockFile, + level: 0, + showTreeToggle: false, + }); + + expect(findChevronButton().exists()).toBe(false); + }); + }); }); -- GitLab From a10802dfc0564c0d62031990f2c06c177b8073f3 Mon Sep 17 00:00:00 2001 From: jerasmus Date: Mon, 27 Oct 2025 12:17:49 +0100 Subject: [PATCH 2/6] Remove nested buttons Remove nested buttons for improved accessibility --- .../components/tree_list.vue | 2 +- .../vue_shared/components/file_row.vue | 127 ++++++++++-------- app/assets/stylesheets/framework/files.scss | 2 +- .../repository/file_tree_browser_spec.rb | 3 +- .../vue_shared/components/file_row_spec.js | 16 ++- 5 files changed, 83 insertions(+), 67 deletions(-) diff --git a/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue b/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue index 78cf90a4252ab4..eea074ffef1e1f 100644 --- a/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue +++ b/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue @@ -414,7 +414,7 @@ export default { :opened="item.opened" :loading="item.loading" :show-tree-toggle="true" - :tabindex="item.loading ? -1 : 0" + :tabindex="-1" :aria-current="isCurrentPath(item.path)" role="treeitem" :aria-expanded="item.opened" diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index db3ef7a9702249..abfbe83ffa30cc 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -62,6 +62,7 @@ export default { folder: this.isTree, 'is-open': this.file.opened, 'is-linked': this.file.linked, + 'pl-3': !this.isTree && this.showTreeToggle, }; }, textForTitle() { @@ -152,62 +153,73 @@ export default { > {{ __('Show more') }} - + + + + + + + + + diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 9731898379a75d..32add9e91e2a11 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -477,7 +477,7 @@ span.idiff { z-index: 1; pointer-events: none; top: -4px; - left: 15px; + left: 14px; width: calc(var(--level) * var(--file-row-level-padding)); bottom: 0; // The virtual scroller has a flat HTML structure so instead of the ::before diff --git a/spec/features/projects/repository/file_tree_browser_spec.rb b/spec/features/projects/repository/file_tree_browser_spec.rb index 740357145f1b64..ad148e3b0bc1de 100644 --- a/spec/features/projects/repository/file_tree_browser_spec.rb +++ b/spec/features/projects/repository/file_tree_browser_spec.rb @@ -78,8 +78,7 @@ expect(ruby_folder[:class]).to include('is-open') # Should highlight the current file - popen_row = find_button('popen.rb') - expect(popen_row['aria-current']).to eq('true') + expect(find('[aria-current="true"]')).to be_present end end end diff --git a/spec/frontend/vue_shared/components/file_row_spec.js b/spec/frontend/vue_shared/components/file_row_spec.js index 70d660cdd8db07..a6313dc696591c 100644 --- a/spec/frontend/vue_shared/components/file_row_spec.js +++ b/spec/frontend/vue_shared/components/file_row_spec.js @@ -1,6 +1,6 @@ -import { shallowMount } from '@vue/test-utils'; import { GlIcon, GlButton } from '@gitlab/ui'; import { nextTick } from 'vue'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import { file } from 'jest/ide/helpers'; import { useMockInternalEventsTracking } from 'helpers/tracking_internal_events_helper'; @@ -15,7 +15,7 @@ describe('File row component', () => { let wrapper; function createComponent(propsData, $router = undefined) { - wrapper = shallowMount(FileRow, { + wrapper = shallowMountExtended(FileRow, { propsData, mocks: { $router, @@ -25,6 +25,8 @@ describe('File row component', () => { const { bindInternalEventDocument } = useMockInternalEventsTracking(); + const findFileButton = () => wrapper.findByTestId('file-row'); + it('renders name', () => { const fileName = 't4'; createComponent({ @@ -62,7 +64,7 @@ describe('File row component', () => { level: 1, }); - expect(wrapper.element.title.trim()).toEqual('path/to/file/with a very long folder name/'); + expect(findFileButton().attributes('title')).toBe('path/to/file/with a very long folder name/'); }); it('does not render a title attribute if no tree present', () => { @@ -84,7 +86,7 @@ describe('File row component', () => { level: 0, }); - wrapper.element.click(); + findFileButton().trigger('click'); expect(wrapper.emitted('toggleTreeOpen')[0][0]).toEqual(fileName); }); @@ -96,7 +98,7 @@ describe('File row component', () => { const filePath = 'path/to/folder'; createComponent({ file: { ...file(fileName), type: 'tree', path: filePath }, level: 0 }); - wrapper.element.click(); + findFileButton().trigger('click'); expect(wrapper.emitted('clickTree')[0][0]).toEqual(filePath); expect(trackEventSpy).toHaveBeenCalledWith( @@ -119,7 +121,7 @@ describe('File row component', () => { level: 1, }); - wrapper.element.click(); + findFileButton().trigger('click'); expect(wrapper.emitted('clickFile')[0][0]).toEqual(fileProp); expect(trackEventSpy).toHaveBeenCalledWith( @@ -252,7 +254,7 @@ describe('File row component', () => { }); describe('Tree toggle chevron button', () => { - const findChevronButton = () => wrapper.findComponent(GlButton); + const findChevronButton = () => wrapper.findByTestId('tree-toggle-button'); const folderPath = 'path/to/folder'; const mockFile = { ...file(folderPath), type: 'tree', opened: false }; -- GitLab From 7533963765fc78e9aa78f6821b804256125cb4ba Mon Sep 17 00:00:00 2001 From: jerasmus Date: Tue, 28 Oct 2025 11:54:02 +0100 Subject: [PATCH 3/6] Remove redundant margin + improve readability Removed the padding and stored padding check in a variable --- app/assets/javascripts/vue_shared/components/file_row.vue | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index abfbe83ffa30cc..ce9ff5b86b7019 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -56,13 +56,14 @@ export default { return this.file.type === 'blob'; }, fileClass() { + const addFilePadding = !this.isTree && this.showTreeToggle; return { 'file-open': this.isBlob && this.file.opened, 'is-active': this.isBlob && this.file.active, folder: this.isTree, 'is-open': this.file.opened, 'is-linked': this.file.linked, - 'pl-3': !this.isTree && this.showTreeToggle, + 'pl-3': addFilePadding, }; }, textForTitle() { @@ -161,7 +162,7 @@ export default { size="small" :icon="chevronIcon" data-testid="tree-toggle-button" - class="file-row-indentation -gl-ml-2 gl-mr-1 gl-shrink-0" + class="file-row-indentation gl-mr-1 gl-shrink-0" :aria-label="file.opened ? __('Collapse directory') : __('Expand directory')" @click="onChevronClick" /> -- GitLab From 4e4c4de9ff2c7e5399bb6781e206a06b61f24998 Mon Sep 17 00:00:00 2001 From: jerasmus Date: Thu, 30 Oct 2025 10:19:33 +0100 Subject: [PATCH 4/6] Prevent collapse when clicking on an open directory Prevents the collapse, only toggles when chevron is clicked --- .../components/tree_list.vue | 12 +++--- .../vue_shared/components/file_row.vue | 4 +- .../components/tree_list_spec.js | 42 ++++++++++++++++--- .../vue_shared/components/file_row_spec.js | 6 +-- 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue b/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue index eea074ffef1e1f..20035b2d563b78 100644 --- a/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue +++ b/app/assets/javascripts/repository/file_tree_browser/components/tree_list.vue @@ -299,7 +299,7 @@ export default { await expand(); }, - toggleDirectory(normalizedPath) { + toggleDirectory(normalizedPath, { toggleClose = true } = {}) { if (!this.expandedPathsMap[normalizedPath]) { // If directory is collapsed, expand it this.expandedPathsMap = { @@ -307,8 +307,8 @@ export default { [normalizedPath]: true, }; this.fetchDirectory(normalizedPath); - } else { - // If directory is already expanded, collapse it + } else if (toggleClose) { + // If directory is already expanded and toggleClose=true, collapse it const newExpandedPaths = { ...this.expandedPathsMap }; delete newExpandedPaths[normalizedPath]; this.expandedPathsMap = newExpandedPaths; @@ -413,8 +413,8 @@ export default { :level="item.level" :opened="item.opened" :loading="item.loading" - :show-tree-toggle="true" - :tabindex="-1" + show-tree-toggle + tabindex="-1" :aria-current="isCurrentPath(item.path)" role="treeitem" :aria-expanded="item.opened" @@ -429,7 +429,7 @@ export default { }" class="gl-relative !gl-mx-0" truncate-middle - @clickTree="toggleDirectory(item.path)" + @clickTree="(options) => toggleDirectory(item.path, options)" @showMore="fetchDirectory(item.parentPath)" /> diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index ce9ff5b86b7019..a88757670f3e67 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -95,13 +95,13 @@ export default { }, onChevronClick(event) { event.stopPropagation(); - this.$emit('clickTree', this.file.path); + this.$emit('clickTree'); }, clickFile() { this.trackEvent('click_file_tree_browser_on_repository_page'); // Manual Action if a tree is selected/opened - if (this.isTree) this.$emit('clickTree', this.file.path); + if (this.isTree) this.$emit('clickTree', { toggleClose: false }); if (this.isTree && this.hasUrlAtCurrentRoute()) { this.toggleTreeOpen(this.file.path); } diff --git a/spec/frontend/repository/file_tree_browser/components/tree_list_spec.js b/spec/frontend/repository/file_tree_browser/components/tree_list_spec.js index 1eb233fc1a122e..b8a4c8f8ab2153 100644 --- a/spec/frontend/repository/file_tree_browser/components/tree_list_spec.js +++ b/spec/frontend/repository/file_tree_browser/components/tree_list_spec.js @@ -109,13 +109,45 @@ describe('Tree List', () => { }); }); - it('calls toggleDirectory with correct path when clickTree is emitted from FileRow', () => { - const toggleDirectory = jest.spyOn(wrapper.vm, 'toggleDirectory').mockImplementation(() => {}); - const path = '/dir_1/dir_2'; - findFileRows().at(0).vm.$emit('clickTree', path); - expect(toggleDirectory).toHaveBeenCalledWith(path); + it('fetches directory contents when tree row is clicked', async () => { + const subdirResponse = cloneDeep(mockResponse); + subdirResponse.data.project.repository.paginatedTree.nodes[0].trees.nodes = []; + getQueryHandlerSuccess.mockResolvedValueOnce(subdirResponse); + + findFileRows().at(0).vm.$emit('clickTree'); + await waitForPromises(); + + expect(getQueryHandlerSuccess).toHaveBeenLastCalledWith( + expect.objectContaining({ path: 'dir_1/dir_2' }), + ); }); + it.each` + toggleClose | expectedOpened | description + ${true} | ${false} | ${'collapses'} + ${false} | ${true} | ${'stays expanded'} + ${undefined} | ${false} | ${'collapses (default behavior)'} + `( + '$description when clicked with toggleClose: $toggleClose', + async ({ toggleClose, expectedOpened }) => { + await createComponent(); + const subdirResponse = cloneDeep(mockResponse); + subdirResponse.data.project.repository.paginatedTree.nodes[0].trees.nodes = []; + getQueryHandlerSuccess.mockResolvedValueOnce(subdirResponse); + + findFileRows().at(0).vm.$emit('clickTree'); + await waitForPromises(); + + expect(findFileRows().at(0).props('file').opened).toBe(true); + + const options = toggleClose === undefined ? undefined : { toggleClose }; + findFileRows().at(0).vm.$emit('clickTree', options); + await nextTick(); + + expect(findFileRows().at(0).props('file').opened).toBe(expectedOpened); + }, + ); + it('sets aria-setsize and aria-posinset relative to siblings at same level', async () => { await createComponent(); const fileRows = findFileRows(); diff --git a/spec/frontend/vue_shared/components/file_row_spec.js b/spec/frontend/vue_shared/components/file_row_spec.js index a6313dc696591c..ccb238296a6cd1 100644 --- a/spec/frontend/vue_shared/components/file_row_spec.js +++ b/spec/frontend/vue_shared/components/file_row_spec.js @@ -91,7 +91,7 @@ describe('File row component', () => { expect(wrapper.emitted('toggleTreeOpen')[0][0]).toEqual(fileName); }); - it('emits clickTree on tree click with file path', () => { + it('emits clickTree on tree click with correct options', () => { const { trackEventSpy } = bindInternalEventDocument(wrapper.element); const fileName = 'folder'; @@ -100,7 +100,7 @@ describe('File row component', () => { findFileButton().trigger('click'); - expect(wrapper.emitted('clickTree')[0][0]).toEqual(filePath); + expect(wrapper.emitted('clickTree')[0][0]).toEqual({ toggleClose: false }); expect(trackEventSpy).toHaveBeenCalledWith( 'click_file_tree_browser_on_repository_page', {}, @@ -284,7 +284,7 @@ describe('File row component', () => { it('renders chevron button for trees and emits clickTree when clicked', () => { findChevronButton().vm.$emit('click', { stopPropagation: jest.fn() }); - expect(wrapper.emitted('clickTree')[0][0]).toBe(folderPath); + expect(wrapper.emitted('clickTree')).toHaveLength(1); }); it('does not render when showTreeToggle is false', () => { -- GitLab From 2bf27153c9ec885d63bf4cf2ff40521d587962e2 Mon Sep 17 00:00:00 2001 From: jerasmus Date: Thu, 30 Oct 2025 10:51:56 +0100 Subject: [PATCH 5/6] Improve aria labeling on the tree toggle Adds more context to the toggle --- .../javascripts/vue_shared/components/file_row.vue | 9 ++++++++- locale/gitlab.pot | 12 ++++++------ spec/frontend/vue_shared/components/file_row_spec.js | 6 ++++-- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index a88757670f3e67..2796818addacf5 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -1,6 +1,7 @@