From e57d42ddf6eb91c45378ff36f034198b751bc54f Mon Sep 17 00:00:00 2001 From: librelad Date: Wed, 27 May 2026 01:40:05 +0100 Subject: [PATCH] refactor(webui): path-based URLs for app tabs + config sub-tabs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app-detail page was the last corner of the SPA still using query parameters for navigation state. Two related complaints surfaced it: - `/app/adguard?tab=tasks` should mirror admin (`/admin/tools/peers`, `/admin/config/network`) and be `/app/adguard/tasks`. - The config sub-tab (general / advanced / features / network / …) had no URL representation at all — `showTab` was a pure visual swap with no history push, so refreshing a deep config sub-tab sent the user back to the default first category. New URL shape: /app/ → config tab, default sub-tab /app// → non-config main tab (tasks, backups, …) /app//config/ → config tab + specific sub-tab …?task= → optional deep-link to a single task Mirrors `adminPath` / `adminCategoryFromPath`. Two new helpers in spa.js carry the convention: window.appPath(name, tab, sub, taskId) → URL window.appPartsFromPath(pathname) → { app, tab, sub } Every URL constructor in the WebUI was replaced with `window.appPath`: spa.js — handleAppDetail back-compat redirect app-tabbed-manager.js — getTabFromURL + new getConfigSubFromURL (path first, ?tab= fallback for legacy) updateURL + updateApp use appPath the inline task-deep-link constructor apps-manager.js — showAppDetail + showAppDetailWithConfig showTab now pushes /app//config/ renderAppDetail picks the sub-tab out of the URL on first load 4 fallback task-URL constructors tasks-manager.js — completion-notification URL task-actions.js — start-notification URL notifications.js — 2 task deep-link URLs Back-compat: handleAppDetail detects legacy `?tab=` / `?config=` / `?task=` queries and replaceState()s the URL to the canonical path shape BEFORE anything else reads URL state — old bookmarks land on the right page and end up with a clean URL. Verified by running every appPath / appPartsFromPath case (including the `logs` → `tasks` legacy alias) and confirming the round-trip is identity. JS syntax checks clean on all six files. No remaining hardcoded `/app/?tab=` strings outside the back-compat comment. Co-Authored-By: Claude Opus 4.7 Signed-off-by: librelad --- .../js/components/app/app-tabbed-manager.js | 67 +++++++++++-------- .../js/components/app/apps-manager.js | 39 +++++++---- .../frontend/js/components/notifications.js | 4 +- .../js/components/task/task-actions.js | 2 +- .../js/components/tasks/tasks-manager.js | 2 +- containers/libreportal/frontend/js/spa.js | 47 ++++++++++++- 6 files changed, 116 insertions(+), 45 deletions(-) diff --git a/containers/libreportal/frontend/js/components/app/app-tabbed-manager.js b/containers/libreportal/frontend/js/components/app/app-tabbed-manager.js index 8cff938..c4046a7 100755 --- a/containers/libreportal/frontend/js/components/app/app-tabbed-manager.js +++ b/containers/libreportal/frontend/js/components/app/app-tabbed-manager.js @@ -135,22 +135,32 @@ class AppTabbedManager { return appName; } - // Get tab from URL parameter + // Get the main tab from the URL. Prefers the path-based shape + // /app// (where defaults to "config" when absent) + // and falls back to the legacy `?tab=` query for older bookmarks / + // links that haven't been migrated yet. Legacy `logs` is aliased to `tasks`. getTabFromURL() { - const currentUrl = window.location.href; + if (window.appPartsFromPath) { + const parts = window.appPartsFromPath(window.location.pathname); + if (parts.app) return parts.tab; // path wins on app pages + } const urlParams = new URLSearchParams(window.location.search); const tab = urlParams.get('tab') || 'config'; - // console.log('🔍 getTabFromURL debug:', { - //currentUrl: currentUrl, - //search: window.location.search, - //tabParam: urlParams.get('tab'), - //defaultTab: 'config', - //finalTab: tab === 'logs' ? 'tasks' : tab - //}); - // Convert "logs" to "tasks" for backward compatibility return tab === 'logs' ? 'tasks' : tab; } + // Get the config sub-tab category from the URL. Only meaningful when the + // main tab is "config" — returns null otherwise (and when no sub-tab is set). + getConfigSubFromURL() { + if (window.appPartsFromPath) { + const parts = window.appPartsFromPath(window.location.pathname); + if (parts.tab === 'config' && parts.sub) return parts.sub; + } + // Legacy `?config=` query support. + const urlParams = new URLSearchParams(window.location.search); + return urlParams.get('config') || null; + } + // Check if we're on an app page before doing anything isAppPage() { const pathname = window.location.pathname; @@ -163,34 +173,38 @@ class AppTabbedManager { search.includes('?=')); // Old format app pages } - // Update URL with app and tab — path-based: /app/?tab=&task=. + // Update URL with app and tab — path-based: + // /app/ — config tab, no sub + // /app// — non-config main tab + // /app//config/ — config tab + sub-category + // …?task= — optional deep-link updateURL(app = null, tab = null) { // Only update URLs on app pages - prevent interference with other pages. if (!this.isAppPage()) return; - const params = new URLSearchParams(window.location.search); - const fromPath = window.location.pathname.replace(/^\/app\/?/, '').split('/')[0]; - const currentApp = app || (fromPath ? decodeURIComponent(fromPath) : '') || this.currentApp; + const currentParts = window.appPartsFromPath + ? window.appPartsFromPath(window.location.pathname) + : { app: '', tab: 'config', sub: null }; + const currentApp = app || currentParts.app || this.currentApp; if (!currentApp) return; - const q = new URLSearchParams(); - const finalTab = tab || params.get('tab'); - if (finalTab) q.set('tab', finalTab); + const finalTab = tab || currentParts.tab || 'config'; + // Keep the config sub-tab only if we're STAYING on config (and on the same + // app). Switching main tab or app drops it; staying on config preserves it. + const finalSub = (!app && finalTab === 'config') ? currentParts.sub : null; // Keep a deep-linked task only when staying on the same app (tab-only update). - if (!app) { - const task = params.get('task'); - if (task) q.set('task', task); - } + const params = new URLSearchParams(window.location.search); + const taskId = !app ? params.get('task') : null; - const qs = q.toString(); - window.history.replaceState({}, '', `/app/${encodeURIComponent(currentApp)}${qs ? '?' + qs : ''}`); + const url = window.appPath(currentApp, finalTab, finalSub, taskId); + window.history.replaceState({}, '', url); } // Update current app and refresh content updateApp(newAppName) { this.setCurrentApp(newAppName); // Reset to the config tab on the path-based app URL. - history.replaceState({}, '', `/app/${encodeURIComponent(newAppName)}?tab=config`); + history.replaceState({}, '', window.appPath(newAppName, 'config')); this.switchTab('config'); } @@ -538,10 +552,7 @@ class AppTabbedManager { // Get current app from AppTabbedManager const currentApp = this.currentApp || ''; - // Construct proper URL with correct parameter order - const newUrl = `/app/${currentApp}?tab=tasks&task=${taskId}`; - // console.log('🔍 Updating URL with task parameter:', newUrl); - history.pushState({}, '', newUrl); + history.pushState({}, '', window.appPath(currentApp, 'tasks', null, taskId)); } } else { console.warn('⚠️ App task details not found for:', taskId); diff --git a/containers/libreportal/frontend/js/components/app/apps-manager.js b/containers/libreportal/frontend/js/components/app/apps-manager.js index 6313f6d..9493598 100755 --- a/containers/libreportal/frontend/js/components/app/apps-manager.js +++ b/containers/libreportal/frontend/js/components/app/apps-manager.js @@ -311,10 +311,9 @@ class AppsManager { // console.log('🔍 Preserving existing tab:', targetTab); } - const newUrl = `/app/${appName}?tab=${targetTab}`; - // console.log('🔍 Setting URL to:', newUrl); + const newUrl = window.appPath(appName, targetTab); history.pushState({}, '', newUrl); - + // Update app-tabbed-manager BEFORE rendering the DOM. If renderAppDetail or // any code it triggers calls switchTab → loadTabContent → restoreButtonState, // we need this.currentApp to already be updated so restoreButtonState checks @@ -369,7 +368,7 @@ class AppsManager { } // Set URL to target tab (config or tasks) - const newUrl = `/app/${appName}?tab=${targetTab}`; + const newUrl = window.appPath(appName, targetTab); history.pushState({}, '', newUrl); // Update app-tabbed-manager. setCurrentApp clears stale disable state from @@ -577,7 +576,12 @@ class AppsManager { // Use global preferred category if not provided if (!preferredCategory && window.preferredConfigCategory) { preferredCategory = window.preferredConfigCategory; - // console.log('🎯 Using global preferred category:', preferredCategory); + } + // …or pick it out of the path (/app//config/) so a refresh / + // deep-link lands on the sub-tab encoded in the URL. + if (!preferredCategory && window.appPartsFromPath) { + const parts = window.appPartsFromPath(window.location.pathname); + if (parts.tab === 'config' && parts.sub) preferredCategory = parts.sub; } const container = document.getElementById('app-detail-view'); @@ -1868,22 +1872,33 @@ class AppsManager { // Hide all panels const allPanels = document.querySelectorAll('.tab-panel'); allPanels.forEach(panel => panel.classList.remove('active')); - + // Remove active from all config category tabs (not main navigation tabs) const allButtons = document.querySelectorAll('.tab-panel:has(.config-section) .tab-button, .config-section .tab-button'); allButtons.forEach(button => button.classList.remove('active')); - + // Show selected panel const targetPanel = document.getElementById(`panel-${tabKey}`); if (targetPanel) { targetPanel.classList.add('active'); } - + // Add active to clicked config category button const targetButton = document.querySelector(`.config-section [data-tab="${tabKey}"], .tab-panel:has(.config-section) [data-tab="${tabKey}"]`); if (targetButton) { targetButton.classList.add('active'); } + + // Push the path-based URL so this sub-tab is shareable + back-buttonable — + // /app//config/. Skipped when there's no current app (e.g. when + // the form is rendered outside of the per-app context). + const currentApp = window.appTabbedManager?.currentApp; + if (currentApp && window.appPath) { + const newUrl = window.appPath(currentApp, 'config', tabKey); + if (window.location.pathname + window.location.search !== newUrl) { + history.pushState({}, '', newUrl); + } + } } // Initialize simple tabs (working method from app-config-original.js) @@ -3227,10 +3242,10 @@ class AppsManager { }, 500); } else if (window.librePortalSPA) { // Fallback: navigate to app with tasks tab - const taskUrl = task ? `/app/${appName}?tab=tasks&task=${task.id}` : `/app/${appName}?tab=tasks`; + const taskUrl = window.appPath(appName, 'tasks', null, task ? task.id : null); window.librePortalSPA.navigateTo(taskUrl); } else if (window.navigateToRoute) { - window.navigateToRoute(`app/${appName}?tab=tasks${task ? `&task=${task.id}` : ''}`); + window.navigateToRoute(window.appPath(appName, 'tasks', null, task ? task.id : null).replace(/^\//, '')); } }, 1000); @@ -3472,11 +3487,11 @@ class AppsManager { }, 500); } else if (window.librePortalSPA) { // Fallback: navigate to app with tasks tab - const taskUrl = task ? `/app/${appName}?tab=tasks&task=${task.id}` : `/app/${appName}?tab=tasks`; + const taskUrl = window.appPath(appName, 'tasks', null, task ? task.id : null); // console.log(`🔄 Navigating to app tasks with uninstall task: ${task?.id}`); window.librePortalSPA.navigateTo(taskUrl); } else if (window.navigateToRoute) { - window.navigateToRoute(`app/${appName}?tab=tasks${task ? `&task=${task.id}` : ''}`); + window.navigateToRoute(window.appPath(appName, 'tasks', null, task ? task.id : null).replace(/^\//, '')); } }, 1000); diff --git a/containers/libreportal/frontend/js/components/notifications.js b/containers/libreportal/frontend/js/components/notifications.js index 92f8dc3..2153b0a 100755 --- a/containers/libreportal/frontend/js/components/notifications.js +++ b/containers/libreportal/frontend/js/components/notifications.js @@ -346,7 +346,7 @@ window.handleNotificationNavigation = (url) => { // We're on an app page - navigate to the specified app and tab if (window.appTabbedManager) { // Update the URL to the target app/tab/task - const newUrl = `/app/${appName}?tab=${tab}&task=${taskId}`; + const newUrl = window.appPath(appName, tab, null, taskId); console.log('🔗 Pushing state to URL:', newUrl); window.history.pushState({}, '', newUrl); @@ -414,7 +414,7 @@ window.handleNotificationNavigation = (url) => { } else { // Not on app or tasks page - navigate to the app's tasks tab if (appName && tab) { - window.history.pushState({}, '', `/app/${appName}?tab=${tab}&task=${taskId}`); + window.history.pushState({}, '', window.appPath(appName, tab, null, taskId)); // Let the SPA handle the navigation if (window.appTabbedManager) { window.appTabbedManager.showAppDetail(appName); diff --git a/containers/libreportal/frontend/js/components/task/task-actions.js b/containers/libreportal/frontend/js/components/task/task-actions.js index c7f03d0..33c1616 100755 --- a/containers/libreportal/frontend/js/components/task/task-actions.js +++ b/containers/libreportal/frontend/js/components/task/task-actions.js @@ -296,7 +296,7 @@ async configUpdate(changes) { const currentUrl = window.location.href; if (currentUrl.includes('/app/') && appName) { - taskUrl = `/app/${appName}?tab=tasks&task=${task.id}`; + taskUrl = window.appPath(appName, 'tasks', null, task.id); } else { taskUrl = `/tasks/all?task=${task.id}`; } diff --git a/containers/libreportal/frontend/js/components/tasks/tasks-manager.js b/containers/libreportal/frontend/js/components/tasks/tasks-manager.js index 12e7503..85bdd3d 100755 --- a/containers/libreportal/frontend/js/components/tasks/tasks-manager.js +++ b/containers/libreportal/frontend/js/components/tasks/tasks-manager.js @@ -1421,7 +1421,7 @@ class TasksManager { : (appName || (task.command || `Task ${taskId}`))); const onAppPage = window.location.pathname.startsWith('/app') && !window.location.pathname.startsWith('/apps'); const url = (onAppPage && appName) - ? `/app/${appName}?tab=tasks&task=${taskId}` + ? window.appPath(appName, 'tasks', null, taskId) : `/tasks/all?task=${taskId}`; const icon = isSystemTask ? '/icons/libreportal.svg' diff --git a/containers/libreportal/frontend/js/spa.js b/containers/libreportal/frontend/js/spa.js index d605cd0..900c2a3 100755 --- a/containers/libreportal/frontend/js/spa.js +++ b/containers/libreportal/frontend/js/spa.js @@ -360,7 +360,23 @@ class LibrePortalSPAClean { this.navigate('/apps', false); return; } - + + // Back-compat: rewrite legacy /app/?tab=[&config=][&task=] + // URLs to the path-based shape (/app// | //config/) + // before the rest of the page reads URL state. The replaceState avoids + // leaving a stale query in the address bar for shareable links. + const legacyTab = url.searchParams.get('tab'); + const legacyConfig = url.searchParams.get('config'); + if (legacyTab || legacyConfig) { + const tab = legacyTab === 'logs' ? 'tasks' : (legacyTab || 'config'); + const sub = (tab === 'config') ? legacyConfig : null; + const taskId = url.searchParams.get('task'); + const canonical = window.appPath(appName, tab, sub, taskId); + if (canonical !== url.pathname + url.search) { + window.history.replaceState({}, '', canonical); + } + } + try { const html = await this.fetchContent('/html/apps-unified-layout.html'); this.loadContent(html, appName); // Will be updated after app data loads @@ -539,6 +555,35 @@ window.adminCategoryFromPath = function (pathname) { return segs[0]; }; +// App-detail path helpers — mirror adminPath but with one extra optional +// segment for the config sub-tab category. The shape is: +// /app/ — config tab, default sub-tab +// /app// — non-config main tab (tasks, backups, …) +// /app//config/ — config tab with a specific sub-tab +// plus an optional ?task= query for deep-linking a single task on the +// tasks tab (a transient deep link, not part of the navigation hierarchy). +window.appPath = function (appName, tab, sub, taskId) { + if (!appName) return '/apps'; + let p = '/app/' + encodeURIComponent(appName); + if (tab && tab !== 'config') { + p += '/' + encodeURIComponent(tab); + } else if (sub) { + p += '/config/' + encodeURIComponent(sub); + } + if (taskId) p += '?task=' + encodeURIComponent(taskId); + return p; +}; +window.appPartsFromPath = function (pathname) { + const segs = String(pathname || '').replace(/^\/app\/?/, '').split('/').filter(Boolean); + const app = segs[0] ? decodeURIComponent(segs[0]) : ''; + let tab = segs[1] || 'config'; + let sub = null; + if (tab === 'config') sub = segs[2] ? decodeURIComponent(segs[2]) : null; + // `logs` is the legacy alias for the `tasks` main tab. + if (tab === 'logs') tab = 'tasks'; + return { app, tab, sub }; +}; + // Global navigation function for click handlers window.navigateToRoute = function(href) { if (window.spaClean) {