From c06723c44afbc16cf85d316fb086887c18de09c7 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Thu, 14 Nov 2024 23:24:47 +0100
Subject: [PATCH] refactor(files): Adjust `useNavigation` composable to enforce
 active view
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

In some cases it is guaranteed that we have a proper active view,
e.g. when the file list is loaded (so within FileEntry* components).
This does not change anything but the Typescript types,
so the `currentView` is typed as `View` instead of `View | null´.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 apps/files/src/components/FileEntry.vue        |  7 ++++---
 .../components/FileEntry/FileEntryActions.vue  | 10 +++++-----
 .../src/components/FileEntry/FileEntryName.vue |  9 +++++----
 apps/files/src/components/FileEntryGrid.vue    |  3 ++-
 .../src/composables/useNavigation.spec.ts      | 18 +++++++++---------
 apps/files/src/composables/useNavigation.ts    |  9 ++++++---
 6 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/apps/files/src/components/FileEntry.vue b/apps/files/src/components/FileEntry.vue
index f6f93be1dc8..fc5c7fb97f6 100644
--- a/apps/files/src/components/FileEntry.vue
+++ b/apps/files/src/components/FileEntry.vue
@@ -74,7 +74,7 @@
 		<!-- View columns -->
 		<td v-for="column in columns"
 			:key="column.id"
-			:class="`files-list__row-${currentView?.id}-${column.id}`"
+			:class="`files-list__row-${currentView.id}-${column.id}`"
 			class="files-list__row-column-custom"
 			:data-cy-files-list-row-column-custom="column.id"
 			@click="openDetailsIfAvailable">
@@ -135,7 +135,8 @@ export default defineComponent({
 		const filesStore = useFilesStore()
 		const renamingStore = useRenamingStore()
 		const selectionStore = useSelectionStore()
-		const { currentView } = useNavigation()
+		// The file list is guaranteed to be only shown with active view - thus we can set the `loaded` flag
+		const { currentView } = useNavigation(true)
 		const {
 			directory: currentDir,
 			fileId: currentFileId,
@@ -180,7 +181,7 @@ export default defineComponent({
 			if (this.filesListWidth < 512 || this.compact) {
 				return []
 			}
-			return this.currentView?.columns || []
+			return this.currentView.columns || []
 		},
 
 		size() {
diff --git a/apps/files/src/components/FileEntry/FileEntryActions.vue b/apps/files/src/components/FileEntry/FileEntryActions.vue
index 06b447295eb..8c150b78087 100644
--- a/apps/files/src/components/FileEntry/FileEntryActions.vue
+++ b/apps/files/src/components/FileEntry/FileEntryActions.vue
@@ -77,8 +77,8 @@
 </template>
 
 <script lang="ts">
-import type { PropType, ShallowRef } from 'vue'
-import type { FileAction, Node, View } from '@nextcloud/files'
+import type { PropType } from 'vue'
+import type { FileAction, Node } from '@nextcloud/files'
 
 import { DefaultType, NodeStatus } from '@nextcloud/files'
 import { showError, showSuccess } from '@nextcloud/dialogs'
@@ -133,12 +133,12 @@ export default defineComponent({
 	},
 
 	setup() {
-		const { currentView } = useNavigation()
+		// The file list is guaranteed to be only shown with active view - thus we can set the `loaded` flag
+		const { currentView } = useNavigation(true)
 		const enabledFileActions = inject<FileAction[]>('enabledFileActions', [])
 
 		return {
-			// The file list is guaranteed to be only shown with active view
-			currentView: currentView as ShallowRef<View>,
+			currentView,
 			enabledFileActions,
 		}
 	},
diff --git a/apps/files/src/components/FileEntry/FileEntryName.vue b/apps/files/src/components/FileEntry/FileEntryName.vue
index 34092a54201..e4cffba32b7 100644
--- a/apps/files/src/components/FileEntry/FileEntryName.vue
+++ b/apps/files/src/components/FileEntry/FileEntryName.vue
@@ -94,7 +94,8 @@ export default defineComponent({
 	},
 
 	setup() {
-		const { currentView } = useNavigation()
+		// The file list is guaranteed to be only shown with active view - thus we can set the `loaded` flag
+		const { currentView } = useNavigation(true)
 		const { directory } = useRouteParameters()
 		const renamingStore = useRenamingStore()
 
@@ -143,7 +144,7 @@ export default defineComponent({
 				}
 			}
 
-			if (this.defaultFileAction && this.currentView) {
+			if (this.defaultFileAction) {
 				const displayName = this.defaultFileAction.displayName([this.source], this.currentView)
 				return {
 					is: 'button',
@@ -247,8 +248,8 @@ export default defineComponent({
 				if (status) {
 					showSuccess(t('files', 'Renamed "{oldName}" to "{newName}"', { oldName, newName }))
 					this.$nextTick(() => {
-						const nameContainter = this.$refs.basename as HTMLElement | undefined
-						nameContainter?.focus()
+						const nameContainer = this.$refs.basename as HTMLElement | undefined
+						nameContainer?.focus()
 					})
 				} else {
 					// Was cancelled - meaning the renaming state is just reset
diff --git a/apps/files/src/components/FileEntryGrid.vue b/apps/files/src/components/FileEntryGrid.vue
index 6d31542a15b..f0b086ac891 100644
--- a/apps/files/src/components/FileEntryGrid.vue
+++ b/apps/files/src/components/FileEntryGrid.vue
@@ -107,7 +107,8 @@ export default defineComponent({
 		const filesStore = useFilesStore()
 		const renamingStore = useRenamingStore()
 		const selectionStore = useSelectionStore()
-		const { currentView } = useNavigation()
+		// The file list is guaranteed to be only shown with active view - thus we can set the `loaded` flag
+		const { currentView } = useNavigation(true)
 		const {
 			directory: currentDir,
 			fileId: currentFileId,
diff --git a/apps/files/src/composables/useNavigation.spec.ts b/apps/files/src/composables/useNavigation.spec.ts
index efea7103126..569e61825e1 100644
--- a/apps/files/src/composables/useNavigation.spec.ts
+++ b/apps/files/src/composables/useNavigation.spec.ts
@@ -2,6 +2,8 @@
  * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
+import type { Navigation, View } from '@nextcloud/files'
+
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { mount } from '@vue/test-utils'
 import { defineComponent } from 'vue'
@@ -9,8 +11,6 @@ import { defineComponent } from 'vue'
 import { useNavigation } from './useNavigation'
 import * as nextcloudFiles from '@nextcloud/files'
 
-const { Navigation, View } = nextcloudFiles
-
 // Just a wrapper so we can test the composable
 const TestComponent = defineComponent({
 	template: '<div></div>',
@@ -29,7 +29,7 @@ describe('Composables: useNavigation', () => {
 
 	describe('currentView', () => {
 		beforeEach(() => {
-			navigation = new Navigation()
+			navigation = new nextcloudFiles.Navigation()
 			spy.mockImplementation(() => navigation)
 		})
 
@@ -39,7 +39,7 @@ describe('Composables: useNavigation', () => {
 		})
 
 		it('should return already active navigation', async () => {
-			const view = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
+			const view = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
 			navigation.register(view)
 			navigation.setActive(view)
 			// Now the navigation is already set it should take the active navigation
@@ -48,7 +48,7 @@ describe('Composables: useNavigation', () => {
 		})
 
 		it('should be reactive on updating active navigation', async () => {
-			const view = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
+			const view = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
 			navigation.register(view)
 			const wrapper = mount(TestComponent)
 
@@ -63,7 +63,7 @@ describe('Composables: useNavigation', () => {
 
 	describe('views', () => {
 		beforeEach(() => {
-			navigation = new Navigation()
+			navigation = new nextcloudFiles.Navigation()
 			spy.mockImplementation(() => navigation)
 		})
 
@@ -73,7 +73,7 @@ describe('Composables: useNavigation', () => {
 		})
 
 		it('should return already registered views', () => {
-			const view = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
+			const view = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
 			// register before mount
 			navigation.register(view)
 			// now mount and check that the view is listed
@@ -82,8 +82,8 @@ describe('Composables: useNavigation', () => {
 		})
 
 		it('should be reactive on registering new views', () => {
-			const view = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
-			const view2 = new View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-2', name: 'My View 2', order: 1 })
+			const view = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-1', name: 'My View 1', order: 0 })
+			const view2 = new nextcloudFiles.View({ getContents: () => Promise.reject(new Error()), icon: '<svg></svg>', id: 'view-2', name: 'My View 2', order: 1 })
 
 			// register before mount
 			navigation.register(view)
diff --git a/apps/files/src/composables/useNavigation.ts b/apps/files/src/composables/useNavigation.ts
index 714b3a6d7b2..2a6f22a1232 100644
--- a/apps/files/src/composables/useNavigation.ts
+++ b/apps/files/src/composables/useNavigation.ts
@@ -11,18 +11,21 @@ import { onMounted, onUnmounted, shallowRef, triggerRef } from 'vue'
 
 /**
  * Composable to get the currently active files view from the files navigation
+ * @param _loaded If set enforce a current view is loaded
  */
-export function useNavigation() {
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+export function useNavigation<T extends boolean>(_loaded?: T) {
+	type MaybeView = T extends true ? View : (View | null);
 	const navigation = getNavigation()
 	const views: ShallowRef<View[]> = shallowRef(navigation.views)
-	const currentView: ShallowRef<View | null> = shallowRef(navigation.active)
+	const currentView: ShallowRef<MaybeView> = shallowRef(navigation.active as MaybeView)
 
 	/**
 	 * Event listener to update the `currentView`
 	 * @param event The update event
 	 */
 	function onUpdateActive(event: CustomEvent<View|null>) {
-		currentView.value = event.detail
+		currentView.value = event.detail as MaybeView
 	}
 
 	/**
-- 
2.39.5