From d5e2375f3866df06aebf44713b9af23a832eb3a4 Mon Sep 17 00:00:00 2001
From: librelad <librelad@digitalangels.vip>
Date: Mon, 1 Jun 2026 00:37:59 +0100
Subject: [PATCH] fix(webui): address review findings on the fleet Overview
 build

- HIGH: renderAppDetail gated the Roll back button on last_snapshot* fields no
  generator emits, so it never rendered. Derive recoverability from update
  history (updater_apply always snapshots first) so the affordance is reachable.
- MED: per-app Updates tab now repaints on update/rollback/check/hotfix task
  completion (mirrors the backups card) instead of going stale until re-click.
- MED: in-page tab switches now sync spaClean.currentRoute, so the sidebar
  Overview entry no longer no-ops after switching tabs.
- LOW: keyboard activation (Enter/Space) for the role=button expander heads,
  backup tiles, and sidebar Overview entry.
- LOW: preserve ALL expanded Updates rows across a background repaint, not just
  the single ?app= deep-link (split toggle into _openDetail/_closeDetail).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../apps/core/html/apps-unified-layout.html   |  3 +-
 .../apps/core/js/app-tabbed-manager.js        |  7 ++
 .../apps/overview/js/overview-manager.js      | 88 ++++++++++++++-----
 .../components/updater/js/updater-page.js     | 15 +++-
 4 files changed, 89 insertions(+), 24 deletions(-)

diff --git a/containers/libreportal/frontend/components/apps/core/html/apps-unified-layout.html b/containers/libreportal/frontend/components/apps/core/html/apps-unified-layout.html
index 61cfeaa..91832f3 100755
--- a/containers/libreportal/frontend/components/apps/core/html/apps-unified-layout.html
+++ b/containers/libreportal/frontend/components/apps/core/html/apps-unified-layout.html
@@ -9,7 +9,8 @@
       <!-- Fleet Overview entry — pinned above the search box, opens the
            Overview · Updates · Improvements · Backups tabs in the main pane. -->
       <div class="sidebar-overview-entry" id="sidebar-overview-entry" role="button" tabindex="0"
-           onclick="if(window.navigateToRoute){window.navigateToRoute('/overview');}else{window.location.href='/overview';}">
+           onclick="if(window.navigateToRoute){window.navigateToRoute('/overview');}else{window.location.href='/overview';}"
+           onkeydown="if(event.key==='Enter'||event.key===' '){event.preventDefault();this.click();}">
         <svg class="ov-entry-icon" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true">
           <rect x="3" y="3" width="7" height="9"></rect>
           <rect x="14" y="3" width="7" height="5"></rect>
diff --git a/containers/libreportal/frontend/components/apps/core/js/app-tabbed-manager.js b/containers/libreportal/frontend/components/apps/core/js/app-tabbed-manager.js
index 0248ee0..34de23e 100755
--- a/containers/libreportal/frontend/components/apps/core/js/app-tabbed-manager.js
+++ b/containers/libreportal/frontend/components/apps/core/js/app-tabbed-manager.js
@@ -869,6 +869,13 @@ class AppTabbedManager {
       if ((action === 'backup' || action === 'delete' || action === 'delete_all') && this.backupAppCard) {
         this.backupAppCard.render();
       }
+      // Repaint the per-app Updates tab after an update/rollback/check/hotfix so
+      // the version/CVE/badge don't go stale (mirrors the backups card above).
+      // Gate on the pane actually being active rather than a tracked field.
+      if (typeof action === 'string' && /updater|artifact/i.test(action)) {
+        const pane = document.getElementById('updater-tab');
+        if (pane && pane.classList.contains('active')) this.loadAppUpdater();
+      }
     });
 
     // Extra safety net: any `taskUpdated` whose status is terminal should
diff --git a/containers/libreportal/frontend/components/apps/overview/js/overview-manager.js b/containers/libreportal/frontend/components/apps/overview/js/overview-manager.js
index 560f6d1..58197de 100644
--- a/containers/libreportal/frontend/components/apps/overview/js/overview-manager.js
+++ b/containers/libreportal/frontend/components/apps/overview/js/overview-manager.js
@@ -47,6 +47,15 @@ class OverviewManager {
     if (root.dataset.ovBound !== '1') {
       root.dataset.ovBound = '1';
       root.addEventListener('click', (e) => this._handleClick(e));
+      // Keyboard activation for the role="button" elements (expander row-heads,
+      // backup tiles): Enter/Space reuse the same click dispatch.
+      root.addEventListener('keydown', (e) => {
+        if (e.key !== 'Enter' && e.key !== ' ') return;
+        const t = e.target.closest('[data-overview-action], [role="button"]');
+        if (!t || !root.contains(t)) return;
+        e.preventDefault();
+        this._handleClick(e);
+      });
     }
     // Repaint when a relevant task finishes — debounced via the shared
     // coordinator, idempotent by id (re-registering replaces), self-guarded
@@ -93,7 +102,17 @@ class OverviewManager {
     if (!id || id === this.current) return;
     this._applyTab(id);
     const url = id === 'overview' ? '/overview' : `/overview/${id}`;
-    window.history.pushState({ route: url }, '', url);
+    this._pushUrl(url, false);
+  }
+
+  // Push/replace the URL AND keep the SPA kernel's currentRoute in sync —
+  // these tab switches bypass spaClean.navigate(), and a stale currentRoute
+  // would make its de-dupe guard turn the sidebar "Overview" entry into a
+  // no-op after an in-page tab switch.
+  _pushUrl(url, replace) {
+    if (replace) window.history.replaceState({ route: url }, '', url);
+    else window.history.pushState({ route: url }, '', url);
+    try { if (window.spaClean) window.spaClean.currentRoute = url; } catch (_) {}
   }
 
   _applyTab(id) {
@@ -122,7 +141,17 @@ class OverviewManager {
     if (!pane) return;
     switch (id) {
       case 'overview': pane.innerHTML = this.renderOverview(); break;
-      case 'updates': pane.innerHTML = this.renderUpdates(); this._honorAppDeepLink(); break;
+      case 'updates': {
+        // Preserve which rows are expanded across a rebuild — a background
+        // task-refresh repaints the whole table, so restore ALL open rows, not
+        // just the single ?app= deep-link.
+        const open = Array.from(document.querySelectorAll('#overview-view .ov-row-details:not([hidden])'))
+          .map((d) => d.id.replace(/^ov-detail-/, ''));
+        pane.innerHTML = this.renderUpdates();
+        open.forEach((app) => this._openDetail(app));
+        this._honorAppDeepLink();
+        break;
+      }
       case 'improvements': pane.innerHTML = this.renderImprovements(); break;
       case 'backups': pane.innerHTML = this.renderBackups(); break;
     }
@@ -134,7 +163,7 @@ class OverviewManager {
     const app = new URLSearchParams(window.location.search).get('app');
     if (!app) return;
     const details = document.getElementById(`ov-detail-${app}`);
-    if (details && details.hidden) this.toggleAppDetails(app);
+    if (details && details.hidden) this._openDetail(app);
   }
 
   // ---- clicks --------------------------------------------------------------
@@ -272,30 +301,49 @@ class OverviewManager {
 
   toggleAppDetails(app) {
     if (!app) return;
-    const root = document.getElementById('overview-view');
     const details = document.getElementById(`ov-detail-${app}`);
-    const head = root && root.querySelector(`.ov-row[data-app="${(window.CSS && CSS.escape) ? CSS.escape(app) : app}"] .ov-row-head`);
     if (!details) return;
-    const isOpen = !details.hidden;
     const base = `/overview/${this.current || 'updates'}`;
-    if (isOpen) {
-      details.hidden = true;
-      details.classList.remove('ov-open');
-      if (head) { head.setAttribute('aria-expanded', 'false'); head.classList.remove('ov-open'); }
-      window.history.replaceState({ route: base }, '', base);
+    if (!details.hidden) {
+      this._closeDetail(app);
+      this._pushUrl(base, true);
     } else {
-      if (!details.dataset.filled && this.updater) {
-        const appObj = (this.updater.apps || []).find((x) => x.name === app);
-        details.innerHTML = appObj ? this.updater.renderAppDetail(appObj) : '';
-        details.dataset.filled = '1';
-      }
-      details.hidden = false;
-      details.classList.add('ov-open');
-      if (head) { head.setAttribute('aria-expanded', 'true'); head.classList.add('ov-open'); }
-      window.history.replaceState({ route: `${base}?app=${encodeURIComponent(app)}` }, '', `${base}?app=${encodeURIComponent(app)}`);
+      this._openDetail(app);
+      this._pushUrl(`${base}?app=${encodeURIComponent(app)}`, true);
     }
   }
 
+  _rowHead(app) {
+    const root = document.getElementById('overview-view');
+    return root && root.querySelector(`.ov-row[data-app="${(window.CSS && CSS.escape) ? CSS.escape(app) : app}"] .ov-row-head`);
+  }
+
+  // Fill (lazily) + show a row's detail body. URL/history is handled by the
+  // caller, so this is safe to call when restoring multiple rows after a
+  // rebuild without each one clobbering the ?app= deep-link.
+  _openDetail(app) {
+    const details = document.getElementById(`ov-detail-${app}`);
+    if (!details) return;
+    if (!details.dataset.filled && this.updater) {
+      const appObj = (this.updater.apps || []).find((x) => x.name === app);
+      details.innerHTML = appObj ? this.updater.renderAppDetail(appObj) : '';
+      details.dataset.filled = '1';
+    }
+    details.hidden = false;
+    details.classList.add('ov-open');
+    const head = this._rowHead(app);
+    if (head) { head.setAttribute('aria-expanded', 'true'); head.classList.add('ov-open'); }
+  }
+
+  _closeDetail(app) {
+    const details = document.getElementById(`ov-detail-${app}`);
+    if (!details) return;
+    details.hidden = true;
+    details.classList.remove('ov-open');
+    const head = this._rowHead(app);
+    if (head) { head.setAttribute('aria-expanded', 'false'); head.classList.remove('ov-open'); }
+  }
+
   // ---- Improvements tab (reuse the updater's hotfix renderer) ---------------
 
   renderImprovements() {
diff --git a/containers/libreportal/frontend/components/updater/js/updater-page.js b/containers/libreportal/frontend/components/updater/js/updater-page.js
index 4fef375..e5583c3 100644
--- a/containers/libreportal/frontend/components/updater/js/updater-page.js
+++ b/containers/libreportal/frontend/components/updater/js/updater-page.js
@@ -399,10 +399,19 @@ class UpdaterPage {
     const security = `<div class="updater-detail-section"><h4>Security</h4>${
       cves.length ? cveItems : '<p class="updater-detail-empty">No known CVEs. 🎉</p>'}</div>`;
 
-    const can = !!a.last_snapshot;
-    const snap = can
+    // A rollback target exists if a snapshot field is present (future-proofing)
+    // OR — the data the generator actually emits today — this app has a prior
+    // update/rollback in history (updater_apply always snapshots first, so that
+    // pre-update snapshot is the rollback point). Gating only on the never-
+    // written last_snapshot* fields would make the Roll back button unreachable.
+    const priorUpdate = ((this.history && this.history.entries) || [])
+      .some((e) => e.app === a.name && (e.action === 'update' || e.action === 'rollback'));
+    const can = !!a.last_snapshot || priorUpdate;
+    const snap = a.last_snapshot
       ? `${this.escape(a.last_snapshot_version || '')} · ${this.fmtRel(a.last_snapshot_at)}`
-      : 'A recovery snapshot is taken automatically before the next update.';
+      : (priorUpdate
+        ? 'Roll back to the snapshot taken before the last update.'
+        : 'A recovery snapshot is taken automatically before the next update.');
     const recovery = `<div class="updater-detail-section"><h4>Recovery</h4>
         <div class="updater-detail-row"><span class="updater-badge ${can ? 'updater-badge-ok' : 'updater-badge-unknown'}">${can ? 'recoverable' : 'protected'}</span>
         <span class="updater-detail-meta">${snap}</span>