From afb44c2f78f5af4ade2986f8f2a47bd5efe379e4 Mon Sep 17 00:00:00 2001 From: librelad Date: Sat, 30 May 2026 21:04:09 +0100 Subject: [PATCH] refactor(webui): remove dead code from core + fix two latent bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified-dead removals (zero consumers, confirmed by adversarial dependency audit): - core/lib/util/router.js โ€” legacy class Router superseded by kernel/spa.js; self-instantiated, never exposed, and added a SECOND competing popstate listener. Dropped the file + its eager index.html tag. - core/lib/task/task-global-functions.js โ€” wired window.installApp/uninstallApp/ etc. that nothing calls (real calls go through class methods / the task router). Dropped the file + its task-system scripts[] entry + the setupTaskGlobalFunctions() block in system-loader.js. - TopbarComponent.createNavigationHighlighting + clearAllNavigationHighlighting โ€” dead statics; window.topbarNavigationHighlighting was never set. - ui-helpers.js: getAppStatus/formatAppName/getAppShortName (dead), the stale setupMobileMenu/closeMobileMenu (superseded by core/ui/mobile-menu.js's #mobile-drawer impl), setupActiveNavigation + the safe* helpers (verbatim dups of dom-helpers). Only getAppIcon remains. dom-helpers loses dead setupActiveNavigation + waitForElement; it is now the sole safe* source. Bug fixes surfaced during the audit: - system-orchestrator.js called this._wireLogout() which is defined nowhere โ€” threw on the 'Continue Anyway' boot path. Removed the dangling call (logout is wired in topbar.setupLogout()). - active-nav highlighting never updated on SPA navigation (it depended on the never-set global). spa.js now calls the live window.topbar?.setActiveNav?.() after each route handler. No structural moves yet; full node --check sweep clean. Signed-off-by: librelad --- .claude/scheduled_tasks.lock | 1 + .../frontend/core/boot/system-loader.js | 6 - .../frontend/core/boot/system-orchestrator.js | 1 - .../libreportal/frontend/core/kernel/spa.js | 8 +- .../core/lib/task/task-global-functions.js | 28 -- .../frontend/core/lib/util/dom-helpers.js | 40 --- .../frontend/core/lib/util/router.js | 287 ------------------ .../frontend/core/lib/util/ui-helpers.js | 95 +----- .../libreportal/frontend/core/ui/topbar.js | 93 ------ containers/libreportal/frontend/index.html | 1 - 10 files changed, 6 insertions(+), 554 deletions(-) create mode 100644 .claude/scheduled_tasks.lock delete mode 100755 containers/libreportal/frontend/core/lib/task/task-global-functions.js delete mode 100755 containers/libreportal/frontend/core/lib/util/router.js diff --git a/.claude/scheduled_tasks.lock b/.claude/scheduled_tasks.lock new file mode 100644 index 0000000..fd5d4eb --- /dev/null +++ b/.claude/scheduled_tasks.lock @@ -0,0 +1 @@ +{"sessionId":"9cea077c-56da-4223-a765-be38c688106b","pid":1384,"procStart":"1332","acquiredAt":1780168110981} \ No newline at end of file diff --git a/containers/libreportal/frontend/core/boot/system-loader.js b/containers/libreportal/frontend/core/boot/system-loader.js index b7c4968..c54d2ab 100755 --- a/containers/libreportal/frontend/core/boot/system-loader.js +++ b/containers/libreportal/frontend/core/boot/system-loader.js @@ -149,11 +149,6 @@ class SystemLoader { window.taskEventBus.start(); } - // Initialize task global functions - if (typeof setupTaskGlobalFunctions === 'function') { - setupTaskGlobalFunctions(); - } - // Create TasksManager instance if (typeof TasksManager !== 'undefined') { window.tasksManager = new TasksManager(); @@ -171,7 +166,6 @@ class SystemLoader { '/core/lib/task/task-commands.js', '/core/lib/task/task-actions.js', '/core/lib/task/task-router.js', - '/core/lib/task/task-global-functions.js', '/core/lib/task/task-manager.js', '/core/lib/task/task-parameter-preserve.js', '/components/tasks/js/tasks-manager.js', // base: class + constructor + init + bus wiring diff --git a/containers/libreportal/frontend/core/boot/system-orchestrator.js b/containers/libreportal/frontend/core/boot/system-orchestrator.js index 023304d..3dd7240 100755 --- a/containers/libreportal/frontend/core/boot/system-orchestrator.js +++ b/containers/libreportal/frontend/core/boot/system-orchestrator.js @@ -243,7 +243,6 @@ class SystemOrchestrator { // Now initialize the SPA after all components are ready this.initializeOriginalSPA(); - this._wireLogout(); this.proceedToApplication(); } diff --git a/containers/libreportal/frontend/core/kernel/spa.js b/containers/libreportal/frontend/core/kernel/spa.js index ae9fe04..76ed334 100755 --- a/containers/libreportal/frontend/core/kernel/spa.js +++ b/containers/libreportal/frontend/core/kernel/spa.js @@ -290,11 +290,9 @@ class LibrePortalSPAClean { this.showError('Page not found'); } - // Update navigation highlighting after content loads - if (typeof window.topbarNavigationHighlighting === 'function') { - //console.log('๐Ÿ”— SPA: Updating navigation highlighting after navigation'); - window.topbarNavigationHighlighting(); - } + // Update navigation highlighting after content loads. The live topbar + // singleton recomputes the active nav from the current path. + window.topbar?.setActiveNav?.(); } catch (error) { console.error('โŒ Navigation error:', error); diff --git a/containers/libreportal/frontend/core/lib/task/task-global-functions.js b/containers/libreportal/frontend/core/lib/task/task-global-functions.js deleted file mode 100755 index 50bc28a..0000000 --- a/containers/libreportal/frontend/core/lib/task/task-global-functions.js +++ /dev/null @@ -1,28 +0,0 @@ -/** - * Global Functions for Individual Task Actions - * Extends the tasks manager with global action functions for individual tasks - */ - -// Task global functions initialization is now handled by SystemLoader -// Global task functions will be set up centrally when TasksManager is available - -function setupTaskGlobalFunctions() { - if (window.TasksManager || window.tasksManager) { - // Use whichever instance is available - const tasksManager = window.tasksManager || window.TasksManager; - - // Add modular action functions to global scope - window.installApp = (appName, config = '') => tasksManager.router.routeAction('install', { appName, config }); - window.uninstallApp = (appName) => tasksManager.router.routeAction('uninstall', { appName }); - window.restartApp = (appName) => tasksManager.router.routeAction('restart', { appName }); - window.startApp = (appName) => tasksManager.router.routeAction('start', { appName }); - window.stopApp = (appName) => tasksManager.router.routeAction('stop', { appName }); - window.backupApp = (appName) => tasksManager.router.routeAction('backup', { appName }); - window.updateConfig = (appName) => tasksManager.router.routeAction('update_config', { appName }); - window.systemUpdate = () => tasksManager.router.routeAction('system_update'); - - //console.log('โœ… Task action functions registered globally'); - } else { - console.warn('โš ๏ธ TasksManager not found, action functions not registered'); - } -} diff --git a/containers/libreportal/frontend/core/lib/util/dom-helpers.js b/containers/libreportal/frontend/core/lib/util/dom-helpers.js index 3e8ee3d..e765716 100755 --- a/containers/libreportal/frontend/core/lib/util/dom-helpers.js +++ b/containers/libreportal/frontend/core/lib/util/dom-helpers.js @@ -1,6 +1,5 @@ // DOM Helper Functions - Shared across all pages -// DOM helpers function safeGetElement(id) { const element = document.getElementById(id); if (!element) { @@ -24,42 +23,3 @@ function safeQuerySelectorAll(selector) { } return elements; } - -// Navigation helpers -function setupActiveNavigation() { - const navItems = document.querySelectorAll('.nav-item'); - navItems.forEach(item => { - if (item.textContent.includes('App Center')) { - item.classList.add('active'); - } - }); -} - -// Wait for element to appear -function waitForElement(id, timeout = 5000) { - return new Promise((resolve, reject) => { - const element = document.getElementById(id); - if (element) { - resolve(element); - return; - } - - const observer = new MutationObserver(() => { - const element = document.getElementById(id); - if (element) { - observer.disconnect(); - resolve(element); - } - }); - - observer.observe(document.body, { - childList: true, - subtree: true - }); - - setTimeout(() => { - observer.disconnect(); - reject(new Error(`Element ${id} not found within ${timeout}ms`)); - }, timeout); - }); -} \ No newline at end of file diff --git a/containers/libreportal/frontend/core/lib/util/router.js b/containers/libreportal/frontend/core/lib/util/router.js deleted file mode 100755 index 64f5e86..0000000 --- a/containers/libreportal/frontend/core/lib/util/router.js +++ /dev/null @@ -1,287 +0,0 @@ -// SPA Router for LibrePortal - Handles navigation without page reloads -class Router { - constructor() { - this.routes = new Map(); - this.currentRoute = null; - this.loadingBar = null; - this.contentContainer = null; - this.init(); - } - - init() { - // Create loading bar - this.createLoadingBar(); - - // Get content container - this.contentContainer = document.getElementById('main-content') || document.querySelector('.main'); - //console.log('Router: Content container found:', !!this.contentContainer, this.contentContainer); - - // Handle browser back/forward - window.addEventListener('popstate', (e) => { - if (e.state && e.state.route) { - this.navigate(e.state.route, false); - } - }); - - // Don't handle initial route here - let SPA handle it after routes are registered - //console.log('Router: Initialized, waiting for SPA to handle initial navigation'); - } - - createLoadingBar() { - // Create neon loading bar - this.loadingBar = document.createElement('div'); - this.loadingBar.className = 'neon-loading-bar'; - this.loadingBar.innerHTML = ` -
-
-
-
- `; - - // Insert after header - const header = document.getElementById('topbar-container'); - if (header) { - header.insertAdjacentElement('afterend', this.loadingBar); - } - - // Add CSS - this.addLoadingBarStyles(); - } - - addLoadingBarStyles() { - const style = document.createElement('style'); - style.textContent = ` - .neon-loading-bar { - position: fixed; - top: 60px; - left: 0; - right: 0; - height: 3px; - background: rgba(0, 0, 0, 0.3); - z-index: 1000; - opacity: 0; - transition: opacity 0.3s ease; - } - - .neon-loading-bar.active { - opacity: 1; - } - - .neon-loading-bar-inner { - position: relative; - width: 100%; - height: 100%; - overflow: hidden; - } - - .neon-loading-bar-progress { - height: 100%; - width: 0%; - background: linear-gradient(90deg, #00d4ff, #0099ff, #00d4ff); - transition: width 0.3s ease; - box-shadow: 0 0 10px #00d4ff; - animation: neonPulse 1.5s ease-in-out infinite; - } - - .neon-loading-bar-glow { - position: absolute; - top: 0; - left: 0; - right: 0; - height: 100%; - background: linear-gradient(90deg, transparent, #00d4ff, transparent); - opacity: 0.6; - animation: neonGlow 2s ease-in-out infinite; - } - - @keyframes neonPulse { - 0%, 100% { opacity: 1; } - 50% { opacity: 0.7; } - } - - @keyframes neonGlow { - 0%, 100% { - opacity: 0.3; - transform: scaleX(1); - } - 50% { - opacity: 0.8; - transform: scaleX(1.1); - } - } - - .neon-loading-bar.complete .neon-loading-bar-progress { - width: 100%; - animation: none; - } - - .neon-loading-bar.complete .neon-loading-bar-glow { - animation: none; - opacity: 0; - } - `; - document.head.appendChild(style); - } - - // Register a route - register(path, handler) { - this.routes.set(path, handler); - //console.log('Router: Registered route:', path); - } - - // Navigate to a route - async navigate(path, addToHistory = true) { - if (this.currentRoute === path) return; - - // Show loading bar - this.showLoadingBar(); - - // Update browser history - if (addToHistory) { - history.pushState({ route: path }, '', path); - } - - this.currentRoute = path; - - try { - //console.log('Router: Navigating to:', path); - //console.log('Router: Available routes:', Array.from(this.routes.keys())); - - // Find matching route - simple wildcard matching - let handler = null; - - if (this.routes.has(path)) { - // Exact match - handler = this.routes.get(path); - } else { - // Wildcard matching - for (const [route, routeHandler] of this.routes) { - if (route.includes('*')) { - const routePattern = route.replace('*', ''); - if (path.startsWith(routePattern)) { - handler = routeHandler; - break; - } - } - } - } - - if (handler) { - //console.log('Router: Found handler for:', path); - await handler(); - } else { - console.warn('Router: No handler found for:', path); - console.warn('Router: Available routes were:', Array.from(this.routes.keys())); - this.hideLoadingBar(); - } - } catch (error) { - console.error('Navigation error:', error); - this.hideLoadingBar(); - } - } - - showLoadingBar() { - if (this.loadingBar) { - this.loadingBar.classList.add('active'); - // Reset progress - const progress = this.loadingBar.querySelector('.neon-loading-bar-progress'); - if (progress) { - progress.style.width = '0%'; - } - } - } - - updateProgress(percent) { - if (this.loadingBar) { - const progress = this.loadingBar.querySelector('.neon-loading-bar-progress'); - if (progress) { - progress.style.width = `${percent}%`; - } - } - } - - hideLoadingBar() { - if (this.loadingBar) { - // Complete animation - this.updateProgress(100); - - setTimeout(() => { - this.loadingBar.classList.add('complete'); - - setTimeout(() => { - this.loadingBar.classList.remove('active', 'complete'); - const progress = this.loadingBar.querySelector('.neon-loading-bar-progress'); - if (progress) { - progress.style.width = '0%'; - } - }, 300); - }, 200); - } - } - - // Load content dynamically - async loadContent(content, title = null) { - //console.log('Router: Loading content with title:', title); - if (!this.contentContainer) { - console.error('Router: No content container found'); - return; - } - - // Clear current content - this.contentContainer.innerHTML = ''; - - // Update page title - if (title) { - document.title = `${title} - LibrePortal`; - } - - // Add new content - this.contentContainer.innerHTML = content; - //console.log('Router: Content loaded successfully, container innerHTML length:', this.contentContainer.innerHTML.length); - - // Update active nav state - this.updateActiveNav(); - } - - updateActiveNav() { - // Always clear all navigation first to prevent sticky highlighting - if (typeof window.topbarNavigationHighlighting === 'function') { - window.topbarNavigationHighlighting(); - return; // Exit early - let topbar handle it - } - - // Fallback to original logic if helper not available - // Remove active class from all nav items - document.querySelectorAll('.nav-item').forEach(item => { - item.classList.remove('active'); - }); - - // Use path-based detection only for consistency - const pathname = window.location.pathname; - - let activeNavId; - - // PRIMARY: Use path-based detection only (most reliable) - if (pathname.startsWith('/app') || pathname.startsWith('/apps')) { - activeNavId = 'nav-app-center'; - } else if (pathname.startsWith('/admin') || pathname.startsWith('/config') || pathname.startsWith('/ssh')) { - activeNavId = 'nav-config'; - } else if (pathname.startsWith('/tasks')) { - activeNavId = 'nav-tasks'; - } else if (pathname === '/' || pathname === '/dashboard') { - activeNavId = 'nav-dashboard'; - } else { - activeNavId = 'nav-dashboard'; // default - } - - if (activeNavId) { - const activeNav = document.getElementById(activeNavId); - if (activeNav) { - activeNav.classList.add('nav-active'); - } - } - } -} - -// Global router instance -const router = new Router(); diff --git a/containers/libreportal/frontend/core/lib/util/ui-helpers.js b/containers/libreportal/frontend/core/lib/util/ui-helpers.js index 0f19d68..a0bbf53 100755 --- a/containers/libreportal/frontend/core/lib/util/ui-helpers.js +++ b/containers/libreportal/frontend/core/lib/util/ui-helpers.js @@ -1,99 +1,8 @@ -// UI Helper Functions - Shared across all pages -// Simple functions without import/export syntax for compatibility +// App metadata helper โ€” resolves an app's icon path, falling back to the +// bundled SVG when the catalog entry doesn't carry an explicit icon URL. -// App icon and name helpers function getAppIcon(appName, appIconUrl) { const cleanAppName = appName.replace('install_', '').replace(' ', ''); // Use icon URL from JSON if available, otherwise fall back to default return appIconUrl || `/core/icons/apps/${cleanAppName}.svg`; } - -function getAppStatus(installed) { - return installed ? 'Installed' : 'Not Installed'; -} - -function formatAppName(name) { - return name.split(' - ')[0].split(' -')[0].trim(); -} - -function getAppShortName(appData) { - return appData.name.split(' - ')[0].split(' -')[0].trim(); -} - -// Mobile menu helpers -function setupMobileMenu() { - const mobileMenuToggle = document.getElementById('mobile-menu-toggle'); - const sidebar = document.getElementById('sidebar'); - const mobileOverlay = document.getElementById('mobile-overlay'); - - if (!mobileMenuToggle || !sidebar || !mobileOverlay) { - //console.log('Mobile menu elements not found'); - return; - } - - function toggleMobileMenu() { - sidebar.classList.toggle('mobile-open'); - mobileOverlay.classList.toggle('active'); - - if (sidebar.classList.contains('mobile-open')) { - document.body.style.overflow = 'hidden'; - } else { - document.body.style.overflow = ''; - } - } - - function closeMobileMenu() { - sidebar.classList.remove('mobile-open'); - mobileOverlay.classList.remove('active'); - document.body.style.overflow = ''; - } - - // Mobile menu event listeners - mobileMenuToggle.addEventListener('click', toggleMobileMenu); - mobileOverlay.addEventListener('click', closeMobileMenu); - - // Close mobile menu when window is resized to desktop size - window.addEventListener('resize', () => { - if (window.innerWidth > 768) { - closeMobileMenu(); - } - }); - - // Expose closeMobileMenu globally for other scripts - window.closeMobileMenu = closeMobileMenu; -} - -// Navigation helpers -function setupActiveNavigation() { - const navItems = document.querySelectorAll('.nav-item'); - navItems.forEach(item => { - if (item.textContent.includes('App Center')) { - item.classList.add('active'); - } - }); -} - -// DOM helpers -function safeGetElement(id) { - const element = document.getElementById(id); - if (!element) { - //console.log(`${id} element not found`); - } - return element; -} - -function safeQuerySelector(selector) { - const element = document.querySelector(selector); - if (!element) { - //console.log(`${selector} element not found`); - } - return element; -} - -function safeQuerySelectorAll(selector) { - const elements = document.querySelectorAll(selector); - if (!elements || elements.length === 0) { - //console.log(`${selector} elements not found`); - } - return elements; -} diff --git a/containers/libreportal/frontend/core/ui/topbar.js b/containers/libreportal/frontend/core/ui/topbar.js index 0b47736..91150e0 100755 --- a/containers/libreportal/frontend/core/ui/topbar.js +++ b/containers/libreportal/frontend/core/ui/topbar.js @@ -481,97 +481,4 @@ class TopbarComponent { document.body.setAttribute('data-theme', theme); } - static clearAllNavigationHighlighting() { - //// // console.log('๐Ÿงน Aggressively clearing all navigation highlighting'); - - // Remove ALL active classes from ALL navigation items - document.querySelectorAll('.nav-item').forEach(item => { - //// // console.log('๐Ÿงน Clearing nav item:', item.id, item.textContent.trim()); - item.classList.remove('active'); - item.classList.remove('nav-active'); - // Also remove any other possible active states - item.removeAttribute('aria-current'); - item.blur(); // Remove focus - }); - - // Also clear other active classes that might interfere - document.querySelectorAll('.category.active').forEach(item => { - item.classList.remove('active'); - }); - document.querySelectorAll('.tab-button.active').forEach(item => { - item.classList.remove('active'); - }); - - //// // console.log('๐Ÿงน Cleared all navigation highlighting'); - } - - static createNavigationHighlighting() { - // Define the single function that handles navigation highlighting - window.topbarNavigationHighlighting = function() { - // Always clear all existing navigation first to prevent sticky highlighting - TopbarComponent.clearAllNavigationHighlighting(); - - // PRIMARY: Use path-based detection only (most reliable) - // This avoids confusion from query parameters like tab=, app=, etc. - const path = window.location.pathname; - let activeNavId; - - if (path.startsWith('/app') || path.startsWith('/apps')) { - activeNavId = 'nav-app-center'; - } else if (path.startsWith('/admin') || path.startsWith('/config') || path.startsWith('/ssh')) { - activeNavId = 'nav-config'; - } else if (path.startsWith('/tasks')) { - activeNavId = 'nav-tasks'; - } else if (path.startsWith('/backup')) { - activeNavId = 'nav-backup'; - } else if (path.startsWith('/updater')) { - activeNavId = 'nav-updater'; - } else if (path === '/' || path === '/dashboard') { - activeNavId = 'nav-dashboard'; - } else { - // Fallback: use currentPage detection - const currentPage = new TopbarComponent().getCurrentPage(); - switch (currentPage) { - case 'index.html': - case '': - case 'index': - case 'app': - case 'apps': - activeNavId = 'nav-app-center'; - break; - case 'config': - activeNavId = 'nav-config'; - break; - case 'tasks': - activeNavId = 'nav-tasks'; - break; - case 'backup': - activeNavId = 'nav-backup'; - break; - case 'updater': - activeNavId = 'nav-updater'; - break; - case 'dashboard': - activeNavId = 'nav-dashboard'; - break; - default: - activeNavId = 'nav-dashboard'; - } - } - - // Add active class to current page nav - if (activeNavId) { - const activeNav = document.getElementById(activeNavId); - if (activeNav) { - activeNav.classList.add('nav-active'); - } else { - console.warn(`โŒ Topbar: Nav element not found: ${activeNavId}`); - } - } else { - console.warn(`โŒ Topbar: Could not determine active nav for path: ${path}`); - } - }; - - //// // console.log('๐ŸŒ Topbar navigation highlighting function created'); - } } diff --git a/containers/libreportal/frontend/index.html b/containers/libreportal/frontend/index.html index c9d8418..f8ffc01 100755 --- a/containers/libreportal/frontend/index.html +++ b/containers/libreportal/frontend/index.html @@ -92,7 +92,6 @@ -