From 25e35706164636e717a43e1fb524313dea6cbefd Mon Sep 17 00:00:00 2001 From: librelad Date: Sat, 30 May 2026 15:27:36 +0100 Subject: [PATCH] refactor(webui): fold app-detail into the apps feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit app-detail was a separate sibling component but is really the apps feature's detail view (shares the apps controllers + apps-unified-layout). Merge it in: the apps feature now owns /apps*, /app, /app* and its mount() dispatches grid vs detail by path (checks /apps first for wildcard precedence). Removed the components/app-detail/ folder + its manifest entry. (The 1-level feature scan means a feature must be a direct components// child — folding the routes into apps is the correct way to express the relationship.) Co-Authored-By: Claude Opus 4.8 Signed-off-by: librelad --- .../components/app-detail/feature.json | 8 -- .../frontend/components/app-detail/index.js | 58 ------------- .../frontend/components/apps/feature.json | 12 ++- .../frontend/components/apps/index.js | 76 ++++++++++++----- .../frontend/components/manifest.dev.json | 84 ++++++++++++++----- 5 files changed, 128 insertions(+), 110 deletions(-) delete mode 100644 containers/libreportal/frontend/components/app-detail/feature.json delete mode 100644 containers/libreportal/frontend/components/app-detail/index.js diff --git a/containers/libreportal/frontend/components/app-detail/feature.json b/containers/libreportal/frontend/components/app-detail/feature.json deleted file mode 100644 index ffdf149..0000000 --- a/containers/libreportal/frontend/components/app-detail/feature.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "id": "app-detail", - "routes": ["/app", "/app*"], - "module": "/components/app-detail/index.js", - "handler": "handleAppDetail", - "navId": "nav-app-center", - "order": 25 -} diff --git a/containers/libreportal/frontend/components/app-detail/index.js b/containers/libreportal/frontend/components/app-detail/index.js deleted file mode 100644 index bd40070..0000000 --- a/containers/libreportal/frontend/components/app-detail/index.js +++ /dev/null @@ -1,58 +0,0 @@ -// features/app-detail/index.js — the per-app detail page (/app/[/]). -// -// Shares the apps-unified-layout fragment with the App Center grid but renders -// via window.appTabbedManager (a system-loader singleton; call .initialize(), -// never `new`). Registered AFTER the apps feature so '/apps*' wins the wildcard -// match and only true /app* paths land here. mount() mirrors handleAppDetail -// exactly, including the legacy ?app= / ?=name parsing and the ?tab=/?config= -// → path rewrite. unmount() is a no-op for now (the singleton is shared); the -// known unguarded popstate/task-listener re-bind in app-tabbed-manager is -// addressed in a follow-up (it predates this migration). -LP.features.register({ - id: 'app-detail', - routes: ['/app', '/app*'], - - async mount(ctx) { - const url = new URL(window.location); - let appName = url.pathname.replace(/^\/app\/?/, '').split('/')[0]; - appName = appName ? decodeURIComponent(appName) : ''; - if (!appName) appName = url.searchParams.get('app'); - - // Old format ?=appname&tab=tabname - if (!appName && url.search.includes('?=')) { - const queryPart = url.search.replace('?', ''); - for (const part of queryPart.split('&')) { - if (part.startsWith('=')) { appName = part.substring(1); break; } - } - } - - if (!appName) { return ctx.nav('/apps', false); } - - // Back-compat: rewrite legacy ?tab=/?config= to the canonical path shape - // before the page reads URL state (replaceState — no extra history entry). - 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 = ctx.services.router.appPath(appName, tab, sub, taskId); - if (canonical !== url.pathname + url.search) { - window.history.replaceState({ route: canonical }, '', canonical); - } - } - - const html = await ctx.loadFragment('/components/apps/core/html/apps-unified-layout.html'); - ctx.setContent(html, appName); - - if (!window.appTabbedManager) { - throw new Error('AppTabbedManager not initialized by SystemLoader'); - } - await window.appTabbedManager.initialize(); - }, - - async unmount() { - // No-op: AppTabbedManager is a shared system-loader singleton. Its - // listener-rebind leak is pre-existing and fixed separately. - }, -}); diff --git a/containers/libreportal/frontend/components/apps/feature.json b/containers/libreportal/frontend/components/apps/feature.json index 660b2cc..7805010 100644 --- a/containers/libreportal/frontend/components/apps/feature.json +++ b/containers/libreportal/frontend/components/apps/feature.json @@ -1,9 +1,17 @@ { "id": "apps", - "routes": ["/apps", "/apps*"], + "routes": [ + "/apps", + "/apps*", + "/app", + "/app*" + ], "module": "/components/apps/index.js", "handler": "handleApps", "navId": "nav-app-center", - "nav": { "label": "App Center", "order": 20 }, + "nav": { + "label": "App Center", + "order": 20 + }, "order": 20 } diff --git a/containers/libreportal/frontend/components/apps/index.js b/containers/libreportal/frontend/components/apps/index.js index 31efeb5..9c276a8 100644 --- a/containers/libreportal/frontend/components/apps/index.js +++ b/containers/libreportal/frontend/components/apps/index.js @@ -1,18 +1,24 @@ -// features/apps/index.js — the App Center grid (/apps, /apps/). -// -// window.appsManager is a system-loader singleton (created by -// initializeComponents); mount() calls .initialize(), never `new`. Routes -// '/apps' and '/apps*' are registered BEFORE the app-detail feature's '/app*' -// so findRouteHandler's longest-prefix wildcard match resolves /apps* here -// (the manifest order guarantees this). unmount() is a no-op: the singleton and -// its constructor-time taskRefresh registrations are shared with app-detail and -// the dashboard and must outlive this view. +// components/apps/index.js — the App Center: the grid (/apps, /apps/) +// AND the per-app detail page (/app/[/]). One feature owns both route +// sets; mount() dispatches by path. Both render the shared apps-unified-layout +// and drive system-loader singletons — appsManager for the grid, appTabbedManager +// for detail — via .initialize(), never `new`. (app-detail used to be a separate +// sibling component; it's the same feature, so it lives here.) LP.features.register({ id: 'apps', - routes: ['/apps', '/apps*'], + routes: ['/apps', '/apps*', '/app', '/app*'], async mount(ctx) { - // Category from the path (/apps/), else legacy ?= / ?apps=. + // /apps* -> grid; everything else (/app*) -> detail. Check '/apps' FIRST so + // it wins over '/app' (since '/apps'.startsWith('/app')). + if (window.location.pathname.startsWith('/apps')) { + return this._mountGrid(ctx); + } + return this._mountDetail(ctx); + }, + + // ---- grid (/apps, /apps/) ---- + async _mountGrid(ctx) { const seg = window.location.pathname.replace(/^\/apps\/?/, '').split('/')[0]; if (seg) { window.appsCategory = decodeURIComponent(seg); @@ -24,23 +30,53 @@ LP.features.register({ window.appsCategory = new URLSearchParams(search).get('apps') || 'all'; } } - // Load the unified layout only if it isn't already present — preserves grid - // state when moving between categories / back from app-detail (legacy behaviour). + // state when moving between categories / back from detail (legacy behaviour). if (!document.querySelector('.apps-layout')) { const html = await ctx.loadFragment('/components/apps/core/html/apps-unified-layout.html'); ctx.setContent(html, 'Applications'); } - - if (!window.appsManager) { - throw new Error('AppsManager not initialized by SystemLoader'); - } + if (!window.appsManager) throw new Error('AppsManager not initialized by SystemLoader'); await window.appsManager.initialize(); }, + // ---- per-app detail (/app/[/]) ---- + async _mountDetail(ctx) { + const url = new URL(window.location); + let appName = url.pathname.replace(/^\/app\/?/, '').split('/')[0]; + appName = appName ? decodeURIComponent(appName) : ''; + if (!appName) appName = url.searchParams.get('app'); + // Old format ?=appname&tab=tabname + if (!appName && url.search.includes('?=')) { + const queryPart = url.search.replace('?', ''); + for (const part of queryPart.split('&')) { + if (part.startsWith('=')) { appName = part.substring(1); break; } + } + } + if (!appName) { return ctx.nav('/apps', false); } + + // Back-compat: rewrite legacy ?tab=/?config= to the canonical path shape + // before the page reads URL state (replaceState — no extra history entry). + 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 = ctx.services.router.appPath(appName, tab, sub, taskId); + if (canonical !== url.pathname + url.search) { + window.history.replaceState({ route: canonical }, '', canonical); + } + } + + const html = await ctx.loadFragment('/components/apps/core/html/apps-unified-layout.html'); + ctx.setContent(html, appName); + if (!window.appTabbedManager) throw new Error('AppTabbedManager not initialized by SystemLoader'); + await window.appTabbedManager.initialize(); + }, + async unmount() { - // Nothing to release — AppsManager is a shared system-loader singleton. - // The dirty-config nav guard (window.__appConfigNavGuard) fires in - // navigate() BEFORE unmount and is owned by AppsManager, so we leave it. + // No-op: appsManager / appTabbedManager are shared system-loader singletons. + // The dirty-config nav guard fires in navigate() before unmount. }, }); diff --git a/containers/libreportal/frontend/components/manifest.dev.json b/containers/libreportal/frontend/components/manifest.dev.json index 8ae620d..f264f1d 100644 --- a/containers/libreportal/frontend/components/manifest.dev.json +++ b/containers/libreportal/frontend/components/manifest.dev.json @@ -4,73 +4,113 @@ "features": [ { "id": "dashboard", - "routes": ["/", "/dashboard"], + "routes": [ + "/", + "/dashboard" + ], "module": "/components/dashboard/index.js", "handler": "handleDashboard", "navId": "nav-dashboard", - "nav": { "label": "Dashboard", "order": 10 } + "nav": { + "label": "Dashboard", + "order": 10 + } }, { "id": "apps", - "routes": ["/apps", "/apps*"], + "routes": [ + "/apps", + "/apps*", + "/app", + "/app*" + ], "module": "/components/apps/index.js", "handler": "handleApps", "navId": "nav-app-center", - "nav": { "label": "App Center", "order": 20 } - }, - { - "id": "app-detail", - "routes": ["/app", "/app*"], - "module": "/components/app-detail/index.js", - "handler": "handleAppDetail", - "navId": "nav-app-center" + "nav": { + "label": "App Center", + "order": 20 + } }, { "id": "admin", - "routes": ["/admin", "/admin*"], + "routes": [ + "/admin", + "/admin*" + ], "module": "/components/admin/index.js", "handler": "handleAdmin", "navId": "nav-config", - "nav": { "label": "Admin", "order": 40 } + "nav": { + "label": "Admin", + "order": 40 + } }, { "id": "config-redirect", - "routes": ["/config", "/config*"], + "routes": [ + "/config", + "/config*" + ], "handler": "handleConfigRedirect", "navId": "nav-config" }, { "id": "tasks", - "routes": ["/tasks", "/tasks*"], + "routes": [ + "/tasks", + "/tasks*" + ], "module": "/components/tasks/index.js", "handler": "handleTasks", "navId": "nav-tasks", - "nav": { "label": "Tasks", "order": 60 } + "nav": { + "label": "Tasks", + "order": 60 + } }, { "id": "updater", - "routes": ["/updater", "/updater*"], + "routes": [ + "/updater", + "/updater*" + ], "module": "/components/updater/index.js", "navId": "nav-updater", - "nav": { "label": "Updates", "order": 30 } + "nav": { + "label": "Updates", + "order": 30 + } }, { "id": "backup", - "routes": ["/backup", "/backup*"], + "routes": [ + "/backup", + "/backup*" + ], "module": "/components/backup/index.js", "handler": "handleBackup", "navId": "nav-backup", - "nav": { "label": "Backups", "order": 50 } + "nav": { + "label": "Backups", + "order": 50 + } }, { "id": "peers", - "routes": ["/peers", "/peers*"], + "routes": [ + "/peers", + "/peers*" + ], "handler": "handlePeers", "navId": "nav-config" }, { "id": "ssh", - "routes": ["/ssh", "/ssh*"], + "routes": [ + "/ssh", + "/ssh*" + ], "handler": "handleSsh", "navId": "nav-config" }