Merge pull request #42992 from nextcloud/fix/files-navigation-not-active

fix(files): Make the navigation reactive to view changes and show also sub routes as active

apps/files/lib/Activity/Helper.php

@@ -4,6 +4,7 @@
  *
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
  * @author Joas Schilling <coding@schilljs.com>
+ * @author Ferdinand Thiessen <opensource@fthiessen.de>
  *
  * @license AGPL-3.0
  *
@@ -23,30 +24,30 @@
 namespace OCA\Files\Activity;
 
 use OCP\Files\Folder;
+use OCP\Files\IRootFolder;
+use OCP\Files\Node;
 use OCP\ITagManager;
 
 class Helper {
 	/** If a user has a lot of favorites the query might get too slow and long */
 	public const FAVORITE_LIMIT = 50;
 
-	/** @var ITagManager */
-	protected $tagManager;
-
-	/**
-	 * @param ITagManager $tagManager
-	 */
-	public function __construct(ITagManager $tagManager) {
-		$this->tagManager = $tagManager;
+	public function __construct(
+		protected ITagManager $tagManager,
+		protected IRootFolder $rootFolder,
+	) {
 	}
 
 	/**
-	 * Returns an array with the favorites
+	 * Return an array with nodes marked as favorites
 	 *
-	 * @param string $user
-	 * @return array
+	 * @param string $user User ID
+	 * @param bool $foldersOnly Only return folders (default false)
+	 * @return Node[]
+	 * @psalm-return ($foldersOnly is true ? Folder[] : Node[])
 	 * @throws \RuntimeException when too many or no favorites where found
 	 */
-	public function getFavoriteFilePaths($user) {
+	public function getFavoriteNodes(string $user, bool $foldersOnly = false): array {
 		$tags = $this->tagManager->load('files', [], false, $user);
 		$favorites = $tags->getFavorites();
 
@@ -57,26 +58,45 @@ class Helper {
 		}
 
 		// Can not DI because the user is not known on instantiation
-		$rootFolder = \OC::$server->getUserFolder($user);
-		$folders = $items = [];
+		$userFolder = $this->rootFolder->getUserFolder($user);
+		$favoriteNodes = [];
 		foreach ($favorites as $favorite) {
-			$nodes = $rootFolder->getById($favorite);
+			$nodes = $userFolder->getById($favorite);
 			if (!empty($nodes)) {
-				/** @var \OCP\Files\Node $node */
 				$node = array_shift($nodes);
-				$path = substr($node->getPath(), strlen($user . '/files/'));
-
-				$items[] = $path;
-				if ($node instanceof Folder) {
-					$folders[] = $path;
+				if (!$foldersOnly || $node instanceof Folder) {
+					$favoriteNodes[] = $node;
 				}
 			}
 		}
 
-		if (empty($items)) {
+		if (empty($favoriteNodes)) {
 			throw new \RuntimeException('No favorites', 1);
 		}
 
+		return $favoriteNodes;
+	}
+
+	/**
+	 * Returns an array with the favorites
+	 *
+	 * @param string $user
+	 * @return array
+	 * @throws \RuntimeException when too many or no favorites where found
+	 */
+	public function getFavoriteFilePaths(string $user): array {
+		$userFolder = $this->rootFolder->getUserFolder($user);
+		$nodes = $this->getFavoriteNodes($user);
+		$folders = $items = [];
+		foreach ($nodes as $node) {
+			$path = $userFolder->getRelativePath($node->getPath());
+
+			$items[] = $path;
+			if ($node instanceof Folder) {
+				$folders[] = $path;
+			}
+		}
+
 		return [
 			'items' => $items,
 			'folders' => $folders,

apps/files/lib/Controller/ViewController.php

@@ -226,9 +226,14 @@ class ViewController extends Controller {
 
 		// Get all the user favorites to create a submenu
 		try {
-			$favElements = $this->activityHelper->getFavoriteFilePaths($userId);
+			$userFolder = $this->rootFolder->getUserFolder($userId);
+			$favElements = $this->activityHelper->getFavoriteNodes($userId, true);
+			$favElements = array_map(fn (Folder $node) => [
+				'fileid' => $node->getId(),
+				'path' => $userFolder->getRelativePath($node->getPath()),
+			], $favElements);
 		} catch (\RuntimeException $e) {
-			$favElements['folders'] = [];
+			$favElements = [];
 		}
 
 		// If the file doesn't exists in the folder and
@@ -260,7 +265,7 @@ class ViewController extends Controller {
 		$this->initialState->provideInitialState('storageStats', $storageInfo);
 		$this->initialState->provideInitialState('config', $this->userConfig->getConfigs());
 		$this->initialState->provideInitialState('viewConfigs', $this->viewConfig->getConfigs());
-		$this->initialState->provideInitialState('favoriteFolders', $favElements['folders'] ?? []);
+		$this->initialState->provideInitialState('favoriteFolders', $favElements);
 
 		// File sorting user config
 		$filesSortingConfig = json_decode($this->config->getUserValue($userId, 'files', 'files_sorting_configs', '{}'), true);

apps/files/src/actions/openFolderAction.ts

@@ -19,7 +19,6 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
-import { join } from 'path'
 import { Permission, Node, FileType, View, FileAction, DefaultType } from '@nextcloud/files'
 import { translate as t } from '@nextcloud/l10n'
 import FolderSvg from '@mdi/svg/svg/folder.svg?raw'
@@ -49,7 +48,7 @@ export const action = new FileAction({
 			&& (node.permissions & Permission.READ) !== 0
 	},
 
-	async exec(node: Node, view: View, dir: string) {
+	async exec(node: Node, view: View) {
 		if (!node || node.type !== FileType.Folder) {
 			return false
 		}
@@ -57,7 +56,7 @@ export const action = new FileAction({
 		window.OCP.Files.Router.goToRoute(
 			null,
 			{ view: view.id, fileid: node.fileid },
-			{ dir: join(dir, node.basename) },
+			{ dir: node.path },
 		)
 		return null
 	},

apps/files/src/main.ts

@@ -3,8 +3,6 @@ import { createPinia, PiniaVuePlugin } from 'pinia'
 import { getNavigation } from '@nextcloud/files'
 import { getRequestToken } from '@nextcloud/auth'
 
-import FilesListView from './views/FilesList.vue'
-import NavigationView from './views/Navigation.vue'
 import router from './router/router'
 import RouterService from './services/RouterService'
 import SettingsModel from './models/Setting.js'
@@ -35,7 +33,8 @@ Vue.use(PiniaVuePlugin)
 const pinia = createPinia()
 
 // Init Navigation Service
-const Navigation = getNavigation()
+// This only works with Vue 2 - with Vue 3 this will not modify the source but return just a oberserver
+const Navigation = Vue.observable(getNavigation())
 Vue.prototype.$navigation = Navigation
 
 // Init Files App Settings Service

apps/files/src/router/router.ts

@@ -19,9 +19,11 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
+import type { RawLocation, Route } from 'vue-router'
+
 import { generateUrl } from '@nextcloud/router'
 import queryString from 'query-string'
-import Router, { RawLocation, Route } from 'vue-router'
+import Router from 'vue-router'
 import Vue from 'vue'
 import { ErrorHandler } from 'vue-router/types/router'
 
@@ -46,10 +48,10 @@ const router = new Router({
 		{
 			path: '/',
 			// Pretending we're using the default view
-			redirect: { name: 'filelist' },
+			redirect: { name: 'filelist', params: { view: 'files' } },
 		},
 		{
-			path: '/:view/:fileid?',
+			path: '/:view/:fileid(\\d+)?',
 			name: 'filelist',
 			props: true,
 		},

apps/files/src/views/FilesList.vue

@@ -222,8 +222,7 @@ export default defineComponent({
 		},
 
 		currentView(): View {
-			return (this.$navigation.active
-				|| this.$navigation.views.find(view => view.id === 'files')) as View
+			return this.$navigation.active || this.$navigation.views.find((view) => view.id === (this.$route.params?.view ?? 'files'))
 		},
 
 		/**

apps/files/src/views/Navigation.vue

@@ -27,7 +27,7 @@
 				:key="view.id"
 				:allow-collapse="true"
 				:data-cy-files-navigation-item="view.id"
-				:exact="true"
+				:exact="useExactRouteMatching(view)"
 				:icon="view.iconClass"
 				:name="view.name"
 				:open="isExpanded(view)"
@@ -41,7 +41,7 @@
 				<NcAppNavigationItem v-for="child in childViews[view.id]"
 					:key="child.id"
 					:data-cy-files-navigation-item="child.id"
-					:exact="true"
+					:exact-path="true"
 					:icon="child.iconClass"
 					:name="child.name"
 					:to="generateToNavigation(child)">
@@ -75,6 +75,8 @@
 </template>
 
 <script lang="ts">
+import type { View } from '@nextcloud/files'
+
 import { emit } from '@nextcloud/event-bus'
 import { translate } from '@nextcloud/l10n'
 import Cog from 'vue-material-design-icons/Cog.vue'
@@ -85,7 +87,6 @@ import NcIconSvgWrapper from '@nextcloud/vue/dist/Components/NcIconSvgWrapper.js
 import { setPageHeading } from '../../../../core/src/OCP/accessibility.js'
 import { useViewConfigStore } from '../store/viewConfig.ts'
 import logger from '../logger.js'
-import type { View } from '@nextcloud/files'
 import NavigationQuota from '../components/NavigationQuota.vue'
 import SettingsModal from './Settings.vue'
 
@@ -120,7 +121,7 @@ export default {
 		},
 
 		currentView(): View {
-			return this.views.find(view => view.id === this.currentViewId)
+			return this.views.find(view => view.id === this.currentViewId)!
 		},
 
 		views(): View[] {
@@ -137,19 +138,19 @@ export default {
 				})
 		},
 
-		childViews(): View[] {
+		childViews(): Record<string, View[]> {
 			return this.views
 				// filter parent views
 				.filter(view => !!view.parent)
 				// create a map of parents and their children
 				.reduce((list, view) => {
-					list[view.parent] = [...(list[view.parent] || []), view]
+					list[view.parent!] = [...(list[view.parent!] || []), view]
 					// Sort children by order
-					list[view.parent].sort((a, b) => {
+					list[view.parent!].sort((a, b) => {
 						return a.order - b.order
 					})
 					return list
-				}, {})
+				}, {} as Record<string, View[]>)
 		},
 	},
 
@@ -157,7 +158,7 @@ export default {
 		currentView(view, oldView) {
 			if (view.id !== oldView?.id) {
 				this.$navigation.setActive(view)
-				logger.debug('Navigation changed', { id: view.id, view })
+				logger.debug(`Navigation changed from ${oldView.id} to ${view.id}`, { from: oldView, to: view })
 
 				this.showView(view)
 			}
@@ -172,6 +173,16 @@ export default {
 	},
 
 	methods: {
+		/**
+		 * Only use exact route matching on routes with child views
+		 * Because if a view does not have children (like the files view) then multiple routes might be matched for it
+		 * Like for the 'files' view this does not work because of optional 'fileid' param so /files and /files/1234 are both in the 'files' view
+		 * @param view The view to check
+		 */
+		useExactRouteMatching(view: View): boolean {
+			return this.childViews[view.id]?.length > 0
+		},
+
 		showView(view: View) {
 			// Closing any opened sidebar
 			window?.OCA?.Files?.Sidebar?.close?.()
@@ -183,7 +194,7 @@ export default {
 		/**
 		 * Expand/collapse a a view with children and permanently
 		 * save this setting in the server.
-		 * @param view
+		 * @param view View to toggle
 		 */
 		onToggleExpand(view: View) {
 			// Invert state
@@ -196,7 +207,7 @@ export default {
 		/**
 		 * Check if a view is expanded by user config
 		 * or fallback to the default value.
-		 * @param view
+		 * @param view View to check if expanded
 		 */
 		isExpanded(view: View): boolean {
 			return typeof this.viewConfigStore.getConfig(view.id)?.expanded === 'boolean'
@@ -206,12 +217,12 @@ export default {
 
 		/**
 		 * Generate the route to a view
-		 * @param view
+		 * @param view View to generate "to" navigation for
 		 */
 		generateToNavigation(view: View) {
 			if (view.params) {
-				const { dir, fileid } = view.params
-				return { name: 'filelist', params: view.params, query: { dir, fileid } }
+				const { dir } = view.params
+				return { name: 'filelist', params: view.params, query: { dir } }
 			}
 			return { name: 'filelist', params: { view: view.id } }
 		},

apps/files/src/views/favorites.spec.ts

@@ -82,9 +82,9 @@ describe('Favorites view definition', () => {
 
 	test('Default with favorites', () => {
 		const favoriteFolders = [
-			'/foo',
-			'/bar',
-			'/foo/bar',
+			{ fileid: 1, path: '/foo' },
+			{ fileid: 2, path: '/bar' },
+			{ fileid: 3, path: '/foo/bar' },
 		]
 		jest.spyOn(initialState, 'loadState').mockReturnValue(favoriteFolders)
 		jest.spyOn(favoritesService, 'getContents').mockReturnValue(Promise.resolve({ folder: {} as Folder, contents: [] }))
@@ -102,11 +102,12 @@ describe('Favorites view definition', () => {
 			const favoriteView = favoriteFoldersViews[index]
 			expect(favoriteView).toBeDefined()
 			expect(favoriteView?.id).toBeDefined()
-			expect(favoriteView?.name).toBe(basename(folder))
+			expect(favoriteView?.name).toBe(basename(folder.path))
 			expect(favoriteView?.icon).toBe('<svg>SvgMock</svg>')
 			expect(favoriteView?.order).toBe(index)
 			expect(favoriteView?.params).toStrictEqual({
-				dir: folder,
+				dir: folder.path,
+				fileid: folder.fileid.toString(),
 				view: 'favorites',
 			})
 			expect(favoriteView?.parent).toBe('favorites')
@@ -157,7 +158,7 @@ describe('Dynamic update of favourite folders', () => {
 	test('Remove a favorite folder remove the entry from the navigation column', async () => {
 		jest.spyOn(eventBus, 'emit')
 		jest.spyOn(eventBus, 'subscribe')
-		jest.spyOn(initialState, 'loadState').mockReturnValue(['/Foo/Bar'])
+		jest.spyOn(initialState, 'loadState').mockReturnValue([{ fileid: 42, path: '/Foo/Bar' }])
 		jest.spyOn(favoritesService, 'getContents').mockReturnValue(Promise.resolve({ folder: {} as Folder, contents: [] }))
 
 		registerFavoritesView()

apps/files/src/views/favorites.ts

@@ -19,11 +19,13 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
-import { basename } from 'path'
-import { getLanguage, translate as t } from '@nextcloud/l10n'
-import { loadState } from '@nextcloud/initial-state'
-import { Node, FileType, View, getNavigation } from '@nextcloud/files'
+import type { Folder, Node } from '@nextcloud/files'
+
 import { subscribe } from '@nextcloud/event-bus'
+import { FileType, View, getNavigation } from '@nextcloud/files'
+import { loadState } from '@nextcloud/initial-state'
+import { getLanguage, translate as t } from '@nextcloud/l10n'
+import { basename } from 'path'
 import FolderSvg from '@mdi/svg/svg/folder.svg?raw'
 import StarSvg from '@mdi/svg/svg/star.svg?raw'
 
@@ -31,15 +33,22 @@ import { getContents } from '../services/Favorites'
 import { hashCode } from '../utils/hashUtils'
 import logger from '../logger'
 
-export const generateFolderView = function(folder: string, index = 0): View {
+// The return type of the initial state
+interface IFavoriteFolder {
+	fileid: number
+	path: string
+}
+
+export const generateFavoriteFolderView = function(folder: IFavoriteFolder, index = 0): View {
 	return new View({
-		id: generateIdFromPath(folder),
-		name: basename(folder),
+		id: generateIdFromPath(folder.path),
+		name: basename(folder.path),
 
 		icon: FolderSvg,
 		order: index,
 		params: {
-			dir: folder,
+			dir: folder.path,
+			fileid: folder.fileid.toString(),
 			view: 'favorites',
 		},
 
@@ -57,8 +66,9 @@ export const generateIdFromPath = function(path: string): string {
 
 export default () => {
 	// Load state in function for mock testing purposes
-	const favoriteFolders = loadState<string[]>('files', 'favoriteFolders', [])
-	const favoriteFoldersViews = favoriteFolders.map((folder, index) => generateFolderView(folder, index)) as View[]
+	const favoriteFolders = loadState<IFavoriteFolder[]>('files', 'favoriteFolders', [])
+	const favoriteFoldersViews = favoriteFolders.map((folder, index) => generateFavoriteFolderView(folder, index)) as View[]
+	logger.debug('Generating favorites view', { favoriteFolders })
 
 	const Navigation = getNavigation()
 	Navigation.register(new View({
@@ -93,7 +103,7 @@ export default () => {
 			return
 		}
 
-		addPathToFavorites(node.path)
+		addToFavorites(node as Folder)
 	})
 
 	/**
@@ -118,9 +128,9 @@ export default () => {
 	 * update the order property of the existing views
 	 */
 	const updateAndSortViews = function() {
-		favoriteFolders.sort((a, b) => a.localeCompare(b, getLanguage(), { ignorePunctuation: true }))
+		favoriteFolders.sort((a, b) => a.path.localeCompare(b.path, getLanguage(), { ignorePunctuation: true }))
 		favoriteFolders.forEach((folder, index) => {
-			const view = favoriteFoldersViews.find(view => view.id === generateIdFromPath(folder))
+			const view = favoriteFoldersViews.find((view) => view.id === generateIdFromPath(folder.path))
 			if (view) {
 				view.order = index
 			}
@@ -128,16 +138,17 @@ export default () => {
 	}
 
 	// Add a folder to the favorites paths array and update the views
-	const addPathToFavorites = function(path: string) {
-		const view = generateFolderView(path)
+	const addToFavorites = function(node: Folder) {
+		const newFavoriteFolder: IFavoriteFolder = { path: node.path, fileid: node.fileid! }
+		const view = generateFavoriteFolderView(newFavoriteFolder)
 
 		// Skip if already exists
-		if (favoriteFolders.find(folder => folder === path)) {
+		if (favoriteFolders.find((folder) => folder.path === node.path)) {
 			return
 		}
 
 		// Update arrays
-		favoriteFolders.push(path)
+		favoriteFolders.push(newFavoriteFolder)
 		favoriteFoldersViews.push(view)
 
 		// Update and sort views
@@ -148,7 +159,7 @@ export default () => {
 	// Remove a folder from the favorites paths array and update the views
 	const removePathFromFavorites = function(path: string) {
 		const id = generateIdFromPath(path)
-		const index = favoriteFolders.findIndex(folder => folder === path)
+		const index = favoriteFolders.findIndex((folder) => folder.path === path)
 
 		// Skip if not exists
 		if (index === -1) {

apps/files/src/vue.d.ts

@@ -0,0 +1,7 @@
+import type { Navigation } from '@nextcloud/files'
+
+declare module 'vue/types/vue' {
+	interface Vue {
+		$navigation: Navigation
+	}
+}