From 36112c64135341657d71a7eb3994d95df07aef88 Mon Sep 17 00:00:00 2001 From: jerasmus Date: Mon, 10 Nov 2025 15:58:56 +0100 Subject: [PATCH 1/8] Optimize FTB with (lazy loading + computed props) Improve performance for large file trees by deferring component rendering and optimizing reactivity. Key changes: - Use IntersectionObserver to lazily render file-row components only when they appear in viewport - Replace placeholders with actual components as they become visible - Convert flatFilesList from data to computed property for better caching and fewer recalculations - Replace deep watchers on directoriesCache and expandedPathsMap with a single watcher on filteredFlatFilesList - Add proper semantic HTML with UL/LI structure and ARIA tree roles This significantly reduces initial render cost and improves performance for repositories with large directory structures. --- .../components/tree_list.vue | 97 +++++++++++-------- .../repository/file_tree_browser/utils.js | 24 +++++ .../repository/file_tree_browser_spec.rb | 6 +- .../components/tree_list_spec.js | 55 +++++++++-- .../file_tree_browser/utils_spec.js | 33 +++++++ 5 files changed, 167 insertions(+), 48 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 a5edfc454ead1b..3a804d0acc481e 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 @@ -23,9 +23,12 @@ import { hasMorePages, isExpandable, handleTreeKeydown, + createItemVisibilityObserver, + observeElements, } from '../utils'; export default { + name: 'FileTreeBrowser', FOCUS_FILE_TREE_BROWSER_FILTER_BAR, directives: { GlTooltip: GlTooltipDirective, @@ -61,10 +64,15 @@ export default { directoriesCache: {}, expandedPathsMap: {}, loadingPathsMap: {}, - flatFilesList: [], + appearedItems: {}, + itemObserver: null, }; }, computed: { + flatFilesList() { + if (this.isRootLoading) return []; + return this.buildList('/', 0); + }, isRootLoading() { return this.isDirectoryLoading('/'); }, @@ -106,10 +114,14 @@ export default { }, }, watch: { - directoriesCache: { deep: true, handler: 'updateFlatFilesList' }, - expandedPathsMap: { deep: true, handler: 'updateFlatFilesList' }, + filteredFlatFilesList() { + this.$nextTick(() => this.observeListItems()); + }, }, mounted() { + this.itemObserver = createItemVisibilityObserver((itemId, isVisible) => { + this.appearedItems = { ...this.appearedItems, [itemId]: isVisible }; + }); this.expandPathAncestors(this.currentRouterPath || '/'); this.mousetrap = new Mousetrap(); @@ -118,14 +130,10 @@ export default { } }, beforeDestroy() { + this.itemObserver?.disconnect(); this.mousetrap.unbind(keysFor(FOCUS_FILE_TREE_BROWSER_FILTER_BAR)); }, methods: { - updateFlatFilesList() { - if (this.isRootLoading) return; - // Replace array contents in-place to maintain reactivity - this.flatFilesList.splice(0, this.flatFilesList.length, ...this.buildList('/', 0)); - }, isCurrentPath(path) { if (!this.$route.params.path) return path === '/'; return path === this.currentRouterPath; @@ -156,7 +164,7 @@ export default { directoryList.push(generateShowMoreItem(tree.id, path, level)); // Recursively add children for expanded directories - if (this.expandedPathsMap[treePath] && !this.isDirectoryLoading(treePath)) { + if (this.expandedPathsMap[treePath]) { directoryList.push(...this.buildList(treePath, level + 1)); } }); @@ -355,6 +363,9 @@ export default { onTreeKeydown(event) { handleTreeKeydown(event); }, + observeListItems() { + this.$nextTick(() => observeElements(this.$refs.fileTreeList, this.itemObserver)); + }, }, filterPlaceholder: s__('Repository|Filter files (*.vue, *.rb...)'), }; @@ -399,40 +410,48 @@ export default { class="repository-tree-list gl-mt-2 gl-flex gl-min-h-0 gl-flex-col" :aria-label="__('File tree')" > -
- -
+
  • + +
    +
    • +

    {{ __('No files found') }}

    diff --git a/app/assets/javascripts/repository/file_tree_browser/utils.js b/app/assets/javascripts/repository/file_tree_browser/utils.js index 4c6c8d28ce7d97..415b5a265f808a 100644 --- a/app/assets/javascripts/repository/file_tree_browser/utils.js +++ b/app/assets/javascripts/repository/file_tree_browser/utils.js @@ -106,3 +106,27 @@ export const handleTreeKeydown = (event) => { event.preventDefault(); items[nextIndex]?.focus(); }; + +/** + * Creates an IntersectionObserver that toggles item visibility based on viewport intersection + * @param {Function} setItemVisibility - Callback to update item visibility (itemId, isVisible) + * @returns {IntersectionObserver} + */ +export const createItemVisibilityObserver = (setItemVisibility) => + new IntersectionObserver((entries) => + entries?.forEach(({ target, isIntersecting }) => { + setItemVisibility(target.dataset?.itemId, isIntersecting); + const isFocussed = target.querySelector('[data-placeholder-item]') === document.activeElement; + if (isIntersecting && isFocussed) + requestAnimationFrame(() => target.querySelector('button')?.focus()); + }), + ); + +/** + * Observes all elements matching the selector + * @param {HTMLElement} container - Container element to query within + * @param {IntersectionObserver} observer - The observer instance + * @param {string} selector - CSS selector for elements to observe + */ +export const observeElements = (container, observer, selector = 'li[data-item-id]') => + container?.querySelectorAll(selector).forEach((el) => observer?.observe(el)); diff --git a/spec/features/projects/repository/file_tree_browser_spec.rb b/spec/features/projects/repository/file_tree_browser_spec.rb index 063d7740928c73..089b70ada33769 100644 --- a/spec/features/projects/repository/file_tree_browser_spec.rb +++ b/spec/features/projects/repository/file_tree_browser_spec.rb @@ -38,7 +38,7 @@ it 'displays files and directories' do within('.file-tree-browser') do - expect(page).to have_file('README.md') + expect(page).to have_file('CONTRIBUTING.md') expect(page).to have_file('files') end end @@ -49,10 +49,10 @@ it 'navigates to a file' do within('.file-tree-browser') do - click_file('README.md') + click_file('CONTRIBUTING.md') end - expect(page).to have_current_path(project_blob_path(project, "#{project.default_branch}/README.md")) + expect(page).to have_current_path(project_blob_path(project, "#{project.default_branch}/CONTRIBUTING.md")) end it 'expands and collapses directories' do 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 b8a4c8f8ab2153..5ddd5142e4dd00 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 @@ -5,6 +5,7 @@ import { cloneDeep } from 'lodash'; import { PiniaVuePlugin } from 'pinia'; import waitForPromises from 'helpers/wait_for_promises'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import { useMockIntersectionObserver } from 'helpers/mock_dom_observer'; import TreeList from '~/repository/file_tree_browser/components/tree_list.vue'; import FileRow from '~/vue_shared/components/file_row.vue'; import { FOCUS_FILE_TREE_BROWSER_FILTER_BAR, keysFor } from '~/behaviors/shortcuts/keybindings'; @@ -30,6 +31,14 @@ describe('Tree List', () => { let pinia; let getQueryHandlerSuccess; + const { trigger: triggerIntersection } = useMockIntersectionObserver(); + const triggerIntersectionForAll = () => { + const listItems = wrapper.element.querySelectorAll('[data-item-id]'); + listItems.forEach((item) => { + triggerIntersection(item, { entry: { isIntersecting: true } }); + }); + }; + const createComponent = async (apiResponse = mockResponse) => { getQueryHandlerSuccess = jest.fn().mockResolvedValue(apiResponse); @@ -43,6 +52,7 @@ describe('Tree List', () => { }); await waitForPromises(); + triggerIntersectionForAll(); }; beforeEach(() => createComponent()); @@ -51,6 +61,7 @@ describe('Tree List', () => { const findTree = () => wrapper.find('[role="tree"]'); const findHeader = () => wrapper.find('h3'); const findFileRows = () => wrapper.findAllComponents(FileRow); + const findFileRowPlaceholders = () => wrapper.findAll('[data-placeholder-item]'); const findFilterInput = () => wrapper.findComponent(GlFormInput); const findFilterIcon = () => wrapper.findComponent(GlIcon); const findNoFilesMessage = () => wrapper.findByText('No files found'); @@ -112,6 +123,7 @@ describe('Tree List', () => { it('fetches directory contents when tree row is clicked', async () => { const subdirResponse = cloneDeep(mockResponse); subdirResponse.data.project.repository.paginatedTree.nodes[0].trees.nodes = []; + subdirResponse.data.project.repository.paginatedTree.nodes[0].blobs.nodes = []; getQueryHandlerSuccess.mockResolvedValueOnce(subdirResponse); findFileRows().at(0).vm.$emit('clickTree'); @@ -133,6 +145,7 @@ describe('Tree List', () => { await createComponent(); const subdirResponse = cloneDeep(mockResponse); subdirResponse.data.project.repository.paginatedTree.nodes[0].trees.nodes = []; + subdirResponse.data.project.repository.paginatedTree.nodes[0].blobs.nodes = []; getQueryHandlerSuccess.mockResolvedValueOnce(subdirResponse); findFileRows().at(0).vm.$emit('clickTree'); @@ -171,6 +184,10 @@ describe('Tree List', () => { }); it('fetches the next page', () => { + const secondPageResponse = cloneDeep(mockResponse); + secondPageResponse.data.project.repository.paginatedTree.nodes[0].trees.nodes = []; + secondPageResponse.data.project.repository.paginatedTree.nodes[0].blobs.nodes = []; + getQueryHandlerSuccess.mockResolvedValueOnce(secondPageResponse); findFileRows().at(2).vm.$emit('showMore'); expect(getQueryHandlerSuccess).toHaveBeenCalledWith({ @@ -334,9 +351,7 @@ describe('Tree List', () => { describe('deep path navigation with pagination', () => { it('paginates to find directories not on first page', async () => { const page1 = cloneDeep(mockResponse); - page1.data.project.repository.paginatedTree.nodes[0].trees.nodes = [ - { id: 'gid://dir_1', name: 'dir_1', path: 'dir_1', webPath: 'dir_1' }, - ]; + page1.data.project.repository.paginatedTree.nodes[0].blobs.nodes = []; page1.data.project.repository.paginatedTree.pageInfo = { __typename: 'PageInfo', hasNextPage: true, @@ -386,9 +401,9 @@ describe('Tree List', () => { startCursor: null, endCursor: `cursor_${i}`, }; - page.data.project.repository.paginatedTree.nodes[0].trees.nodes = [ - { id: `gid://dir_${i}`, name: `dir_${i}`, path: `dir_${i}`, webPath: `dir_${i}` }, - ]; + + page.data.project.repository.paginatedTree.nodes[0].blobs.nodes = []; + page.data.project.repository.paginatedTree.nodes[0].trees.nodes = []; getQueryHandlerSuccess.mockResolvedValueOnce(page); } @@ -459,4 +474,32 @@ describe('Tree List', () => { ); }); }); + + describe('intersection observer', () => { + it('renders placeholders before intersection and FileRows after', async () => { + getQueryHandlerSuccess = jest.fn().mockResolvedValue(mockResponse); + apolloProvider = createMockApollo([[paginatedTreeQuery, getQueryHandlerSuccess]]); + + wrapper = shallowMountExtended(TreeList, { + apolloProvider, + pinia, + propsData: { projectPath: 'group/project', currentRef: 'main', refType: 'branch' }, + mocks: { $route: { params: {} } }, + }); + + await waitForPromises(); + await nextTick(); + + // Before intersection: placeholders only + expect(findFileRowPlaceholders()).toHaveLength(2); + expect(findFileRows()).toHaveLength(0); + + // After intersection: FileRows rendered + triggerIntersectionForAll(); + await nextTick(); + + expect(findFileRows()).toHaveLength(2); + expect(findFileRowPlaceholders()).toHaveLength(0); + }); + }); }); diff --git a/spec/frontend/repository/file_tree_browser/utils_spec.js b/spec/frontend/repository/file_tree_browser/utils_spec.js index ce6a342532cf3e..84d110025f7b96 100644 --- a/spec/frontend/repository/file_tree_browser/utils_spec.js +++ b/spec/frontend/repository/file_tree_browser/utils_spec.js @@ -7,6 +7,8 @@ import { hasMorePages, isExpandable, handleTreeKeydown, + createItemVisibilityObserver, + observeElements, } from '~/repository/file_tree_browser/utils'; import { FTB_MAX_PAGES, FTB_MAX_DEPTH } from '~/repository/constants'; @@ -189,4 +191,35 @@ describe('File tree browser utilities', () => { }); }); }); + + describe('createItemVisibilityObserver', () => { + it('creates an IntersectionObserver instance', () => { + const observer = createItemVisibilityObserver(jest.fn()); + + expect(observer).toBeInstanceOf(IntersectionObserver); + observer.disconnect(); + }); + }); + + describe('observeElements', () => { + it('observes all elements matching the default selector', () => { + const container = document.createElement('div'); + container.innerHTML = '
  • '; + const observer = { observe: jest.fn() }; + + observeElements(container, observer); + + expect(observer.observe).toHaveBeenCalledTimes(2); + }); + + it('observes elements matching a custom selector', () => { + const container = document.createElement('div'); + container.innerHTML = '
    '; + const observer = { observe: jest.fn() }; + + observeElements(container, observer, '.item'); + + expect(observer.observe).toHaveBeenCalledTimes(2); + }); + }); }); -- GitLab From b1119887a140af4b5a045722cf96bd702b075196 Mon Sep 17 00:00:00 2001 From: jerasmus Date: Tue, 11 Nov 2025 12:08:38 +0100 Subject: [PATCH 2/8] Fix axe violation Moved aria attributes to the li --- .../file_tree_browser/components/tree_list.vue | 18 +++++++++--------- .../components/tree_list_spec.js | 10 +++++----- 2 files changed, 14 insertions(+), 14 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 3a804d0acc481e..b707ada7c4abe5 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 @@ -421,7 +421,15 @@ export default { v-for="item in filteredFlatFilesList" :key="`${item.path}-${item.type}`" :data-item-id="item.id" - role="none" + :aria-current="isCurrentPath(item.path)" + role="treeitem" + :aria-expanded="item.opened" + :aria-selected="isCurrentPath(item.path)" + :aria-level="item.level + 1" + :aria-setsize="siblingInfo(item)[0]" + :aria-posinset="siblingInfo(item)[1]" + :aria-label="item.name" + tabindex="-1" > { -- GitLab From f9d156e37118ab86f9d91aeacbb588c925927c01 Mon Sep 17 00:00:00 2001 From: jerasmus Date: Wed, 12 Nov 2025 14:31:52 +0100 Subject: [PATCH 3/8] Add scrollMargin option to intersection observer Adds a margin to pre-render items --- .../repository/file_tree_browser/utils.js | 17 ++++++++++------- .../repository/file_tree_browser/utils_spec.js | 10 ++++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/repository/file_tree_browser/utils.js b/app/assets/javascripts/repository/file_tree_browser/utils.js index 415b5a265f808a..bdb1f7b08bfe75 100644 --- a/app/assets/javascripts/repository/file_tree_browser/utils.js +++ b/app/assets/javascripts/repository/file_tree_browser/utils.js @@ -113,13 +113,16 @@ export const handleTreeKeydown = (event) => { * @returns {IntersectionObserver} */ export const createItemVisibilityObserver = (setItemVisibility) => - new IntersectionObserver((entries) => - entries?.forEach(({ target, isIntersecting }) => { - setItemVisibility(target.dataset?.itemId, isIntersecting); - const isFocussed = target.querySelector('[data-placeholder-item]') === document.activeElement; - if (isIntersecting && isFocussed) - requestAnimationFrame(() => target.querySelector('button')?.focus()); - }), + new IntersectionObserver( + (entries) => + entries?.forEach(({ target, isIntersecting }) => { + setItemVisibility(target.dataset?.itemId, isIntersecting); + const isFocussed = + target.querySelector('[data-placeholder-item]') === document.activeElement; + if (isIntersecting && isFocussed) + requestAnimationFrame(() => target.querySelector('button')?.focus()); + }), + { scrollMargin: '500px' }, // Pre-render items before scrolling into view (prevent white flashing) ); /** diff --git a/spec/frontend/repository/file_tree_browser/utils_spec.js b/spec/frontend/repository/file_tree_browser/utils_spec.js index 84d110025f7b96..e812632f8e5be3 100644 --- a/spec/frontend/repository/file_tree_browser/utils_spec.js +++ b/spec/frontend/repository/file_tree_browser/utils_spec.js @@ -199,6 +199,16 @@ describe('File tree browser utilities', () => { expect(observer).toBeInstanceOf(IntersectionObserver); observer.disconnect(); }); + + it('creates observer with scrollMargin option', () => { + const mockIntersectionObserver = jest.fn(); + global.IntersectionObserver = mockIntersectionObserver; + createItemVisibilityObserver(jest.fn()); + + expect(mockIntersectionObserver).toHaveBeenCalledWith(expect.any(Function), { + scrollMargin: '500px', + }); + }); }); describe('observeElements', () => { -- GitLab From 789a5336964a2015750baf2b164f3036d6aec029 Mon Sep 17 00:00:00 2001 From: jerasmus Date: Wed, 12 Nov 2025 17:55:55 +0100 Subject: [PATCH 4/8] Fix file tree browser scrolling flash in panelled design Use the panel root as the root for the intersectionObserver --- .../repository/file_tree_browser/utils.js | 5 ++++- .../file_tree_browser/utils_spec.js | 20 +++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/repository/file_tree_browser/utils.js b/app/assets/javascripts/repository/file_tree_browser/utils.js index bdb1f7b08bfe75..ba64fd9d6ed57c 100644 --- a/app/assets/javascripts/repository/file_tree_browser/utils.js +++ b/app/assets/javascripts/repository/file_tree_browser/utils.js @@ -122,7 +122,10 @@ export const createItemVisibilityObserver = (setItemVisibility) => if (isIntersecting && isFocussed) requestAnimationFrame(() => target.querySelector('button')?.focus()); }), - { scrollMargin: '500px' }, // Pre-render items before scrolling into view (prevent white flashing) + { + root: document.querySelector('.js-static-panel-inner'), // Use panel root if available + scrollMargin: '1500px', // Pre-render items before scrolling into view (prevent white flashing) + }, ); /** diff --git a/spec/frontend/repository/file_tree_browser/utils_spec.js b/spec/frontend/repository/file_tree_browser/utils_spec.js index e812632f8e5be3..bbc9dffe104795 100644 --- a/spec/frontend/repository/file_tree_browser/utils_spec.js +++ b/spec/frontend/repository/file_tree_browser/utils_spec.js @@ -200,13 +200,29 @@ describe('File tree browser utilities', () => { observer.disconnect(); }); - it('creates observer with scrollMargin option', () => { + it('creates observer with scrollMargin option and panel root when panel exists', () => { + const panelContainer = document.createElement('div'); + panelContainer.className = 'js-static-panel-inner'; + document.body.appendChild(panelContainer); + + const mockIntersectionObserver = jest.fn(); + global.IntersectionObserver = mockIntersectionObserver; + createItemVisibilityObserver(jest.fn()); + + expect(mockIntersectionObserver).toHaveBeenCalledWith(expect.any(Function), { + root: panelContainer, + scrollMargin: '1500px', + }); + }); + + it('creates observer with null root when panel does not exist', () => { const mockIntersectionObserver = jest.fn(); global.IntersectionObserver = mockIntersectionObserver; createItemVisibilityObserver(jest.fn()); expect(mockIntersectionObserver).toHaveBeenCalledWith(expect.any(Function), { - scrollMargin: '500px', + root: null, + scrollMargin: '1500px', }); }); }); -- GitLab From fd27ea662a9c707d8b3cef2dfed1232000118a1d Mon Sep 17 00:00:00 2001 From: jerasmus Date: Thu, 13 Nov 2025 05:26:14 +0100 Subject: [PATCH 5/8] Fix broken test Removes test element from DOM --- spec/frontend/repository/file_tree_browser/utils_spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/frontend/repository/file_tree_browser/utils_spec.js b/spec/frontend/repository/file_tree_browser/utils_spec.js index bbc9dffe104795..3990f443b7f7ee 100644 --- a/spec/frontend/repository/file_tree_browser/utils_spec.js +++ b/spec/frontend/repository/file_tree_browser/utils_spec.js @@ -193,6 +193,8 @@ describe('File tree browser utilities', () => { }); describe('createItemVisibilityObserver', () => { + afterEach(() => document.querySelector('.js-static-panel-inner')?.remove()); + it('creates an IntersectionObserver instance', () => { const observer = createItemVisibilityObserver(jest.fn()); -- GitLab From c312814f66c6835ecdd1e3340d3604c0e48f4f2c Mon Sep 17 00:00:00 2001 From: jerasmus Date: Mon, 17 Nov 2025 10:02:23 +0100 Subject: [PATCH 6/8] Address maintainer review comments - Fixed overflow issue for deeply nested structures - Fixed Show more focus issue - Removed redundant padding - Removed redundant label - Change the root of the observer in peak mode --- .../components/tree_list.vue | 34 +++++++++++++++---- .../repository/file_tree_browser/utils.js | 6 ++-- .../vue_shared/components/file_row.vue | 3 +- .../components/tree_list_spec.js | 28 +++++++++++++-- .../file_tree_browser/utils_spec.js | 19 +++++------ 5 files changed, 64 insertions(+), 26 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 b707ada7c4abe5..807a16b18e8c54 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 @@ -1,4 +1,5 @@