From 2007491e11c8c59c62118d2348dfca292f79e770 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 13 Feb 2026 15:41:01 +0100 Subject: [PATCH 1/2] fix(files): correctly sort views The condition to sort by order was missing, so the views were only sorted by name. Added the most important order condition and proper tests for this. Signed-off-by: Ferdinand Thiessen --- .../components/FilesNavigationList.spec.ts | 96 +++++++++++++++++++ .../src/components/FilesNavigationList.vue | 6 +- .../components/FilesNavigationListItem.vue | 25 +++-- 3 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 apps/files/src/components/FilesNavigationList.spec.ts diff --git a/apps/files/src/components/FilesNavigationList.spec.ts b/apps/files/src/components/FilesNavigationList.spec.ts new file mode 100644 index 0000000000000..9d728a59f8ff6 --- /dev/null +++ b/apps/files/src/components/FilesNavigationList.spec.ts @@ -0,0 +1,96 @@ +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { getNavigation, View } from '@nextcloud/files' +import { enableAutoDestroy, shallowMount } from '@vue/test-utils' +import { afterEach, beforeEach, describe, expect, test } from 'vitest' +import { nextTick } from 'vue' +import FilesNavigationList from './FilesNavigationList.vue' + +enableAutoDestroy(afterEach) + +describe('FilesNavigationList.vue', () => { + beforeEach(() => { + const navigation = getNavigation() + const views = [...navigation.views] + for (const view of views) { + navigation.remove(view.id) + } + }) + + test('views are added reactivly', async () => { + const navigation = getNavigation() + const view1 = new View({ getContents: () => Promise.reject(new Error()), icon: '', id: 'view-1', name: 'My View 1', order: 1 }) + const view2 = new View({ getContents: () => Promise.reject(new Error()), icon: '', id: 'view-2', name: 'My View 2', order: 9 }) + + navigation.register(view1) + + const wrapper = shallowMount(FilesNavigationList) + let items = wrapper.findAllComponents({ name: 'FilesNavigationListItem' }) + expect(items).toHaveLength(1) + expect(items.at(0).props('view').id).toBe('view-1') + + navigation.register(view2) + await nextTick() + + items = wrapper.findAllComponents({ name: 'FilesNavigationListItem' }) + expect(items).toHaveLength(2) + expect(items.at(0).props('view').id).toBe('view-1') + expect(items.at(1).props('view').id).toBe('view-2') + }) + + test('views are correctly sorted', () => { + const navigation = getNavigation() + const view1 = new View({ getContents: () => Promise.reject(new Error()), icon: '', id: 'view-1', name: 'Z - first', order: 1 }) + const view2 = new View({ getContents: () => Promise.reject(new Error()), icon: '', id: 'view-2', name: 'A - last', order: 9 }) + + navigation.register(view2) + navigation.register(view1) + + const wrapper = shallowMount(FilesNavigationList) + const items = wrapper.findAllComponents({ name: 'FilesNavigationListItem' }) + expect(items).toHaveLength(2) + expect(items.at(0).props('view').id).toBe('view-1') + expect(items.at(1).props('view').id).toBe('view-2') + }) + + /** + * Idea here is that there are two views: + * - "100 second" + * - "2 first" + * + * When sorting by string "10" would be before "2 " (because 1 is before 2), + * but we want natural sorting so "2" is before "100" just like humans would expect. + */ + test('views without order property are correctly sorted using natural sort', () => { + const navigation = getNavigation() + const view1 = new View({ getContents: () => Promise.reject(new Error()), icon: '', id: 'view-1', name: '2 first' }) + const view2 = new View({ getContents: () => Promise.reject(new Error()), icon: '', id: 'view-2', name: '100 second' }) + + navigation.register(view2) + navigation.register(view1) + + const wrapper = shallowMount(FilesNavigationList) + const items = wrapper.findAllComponents({ name: 'FilesNavigationListItem' }) + expect(items).toHaveLength(2) + expect(items.at(0).props('view').id).toBe('view-1') + expect(items.at(1).props('view').id).toBe('view-2') + }) + + test('views without order are always sorted behind views with order property', () => { + const navigation = getNavigation() + const view1 = new View({ getContents: () => Promise.reject(new Error()), icon: '', id: 'view-1', name: '2 first', order: 0 }) + const view2 = new View({ getContents: () => Promise.reject(new Error()), icon: '', id: 'view-2', name: '1 second' }) + + navigation.register(view2) + navigation.register(view1) + + const wrapper = shallowMount(FilesNavigationList) + const items = wrapper.findAllComponents({ name: 'FilesNavigationListItem' }) + expect(items).toHaveLength(2) + expect(items.at(0).props('view').id).toBe('view-1') + expect(items.at(1).props('view').id).toBe('view-2') + }) +}) diff --git a/apps/files/src/components/FilesNavigationList.vue b/apps/files/src/components/FilesNavigationList.vue index ec911e1436697..ddf7ca7897dff 100644 --- a/apps/files/src/components/FilesNavigationList.vue +++ b/apps/files/src/components/FilesNavigationList.vue @@ -1,5 +1,5 @@ @@ -29,7 +29,9 @@ const collator = Intl.Collator( * @param b - second view */ function sortViews(a: IView, b: IView): number { - if (a.order !== undefined && b.order === undefined) { + if (a.order !== undefined && b.order !== undefined) { + return a.order - b.order + } else if (a.order !== undefined && b.order === undefined) { return -1 } else if (a.order === undefined && b.order !== undefined) { return 1 diff --git a/apps/files/src/components/FilesNavigationListItem.vue b/apps/files/src/components/FilesNavigationListItem.vue index 0eed07c64e406..a4bda51a19140 100644 --- a/apps/files/src/components/FilesNavigationListItem.vue +++ b/apps/files/src/components/FilesNavigationListItem.vue @@ -21,16 +21,6 @@ const props = withDefaults(defineProps<{ level: 0, }) -/** - * Load child views on mount if the view is expanded by default - * but has no child views loaded yet. - */ -onMounted(() => { - if (isExpanded.value && !hasChildViews.value) { - loadChildViews() - } -}) - const maxLevel = 6 // Limit nesting to not exceed max call stack size const viewConfigStore = useViewConfigStore() const viewConfig = computed(() => viewConfigStore.viewConfigs[props.view.id]) @@ -84,6 +74,16 @@ const navigationRoute = computed(() => { const isLoading = ref(false) const childViewsLoaded = ref(false) +/** + * Load child views on mount if the view is expanded by default + * but has no child views loaded yet. + */ +onMounted(() => { + if (isExpanded.value && !hasChildViews.value) { + loadChildViews() + } +}) + /** * Handle expanding/collapsing the navigation item. * @@ -128,6 +128,11 @@ const collator = Intl.Collator( [getLanguage(), getCanonicalLocale()], { numeric: true, usage: 'sort' }, ) + +// TODO: Remove this with Vue 3 - the name is inferred by the filename! +export default { + name: 'FilesNavigationListItem', +}