From c6e39973821384e7769b341f6e48b38ee21dc012 Mon Sep 17 00:00:00 2001 From: librelad Date: Thu, 20 Aug 2026 23:11:59 +0100 Subject: [PATCH] feat(webui): update buttons show their task running, in place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the follow-the-task navigation from the previous commit — carrying the user to the tasks page and back was feedback by relocation. This is feedback where the click happened: the button becomes "Updating…" with a spinner and stays disabled until the task reaches a terminal state, then the data refetches and the row repaints with the result. Update, Update all / selected, the stepped Upgrade and Roll back all get it (verbs keep their own labels: Upgrading…, Rolling back…). Two paths keep the face honest. A DOM patch flips the buttons the moment the click lands — no waiting for a render pass — stashing the original face on the element so a failed dispatch can restore it. And every renderer now builds these buttons through one busy-aware helper, so a repaint landing MID-task (the auto-refresh poll, a filter change) reconstructs the spinner instead of silently re-enabling the button. Sibling actions are held while a task runs: Roll back on an app that is mid-update is disabled — without a spinner, which marks the action that is running, not the ones waiting on it. Entry points also guard on the inflight set, so a keyboard-triggered duplicate is inert. Correlation is by task id (routeAction resolves to the created task), so another task finishing cannot end this button's busy state early. Only completed/failed/cancelled end it; listeners are removed on the first terminal event, with a 30-minute timeout so a task that never reports terminal cannot pin a spinner forever. cursor is "progress", not "not-allowed" — the work is happening, the button is not refusing. Co-Authored-By: Claude Opus 5 --- .../apps/overview/js/overview-manager.js | 4 +- .../components/updater/css/updater.css | 21 +++ .../components/updater/js/updater-page.js | 125 +++++++++++++----- 3 files changed, 113 insertions(+), 37 deletions(-) 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 07b539a..2cd460d 100644 --- a/containers/libreportal/frontend/components/apps/overview/js/overview-manager.js +++ b/containers/libreportal/frontend/components/apps/overview/js/overview-manager.js @@ -791,9 +791,9 @@ class OverviewManager { // no way to act on them, which is exactly how it was reported. Secondary // styling keeps the two visibly different. const updBtn = a.update_available - ? `` + ? this.updater.actionBtn(a.name, 'update', 'Update', { cls: 'updater-btn updater-btn-primary' }) : (a.newer_version - ? `` + ? this.updater.actionBtn(a.name, 'upgrade', 'Upgrade', { cls: 'updater-btn updater-btn-success', extra: ` data-version="${esc(a.newer_version)}" title="Move to the ${esc(a.newer_version)} release line — you will be asked to confirm first"` }) : ''); // Only rows with something to apply are selectable. A checkbox on a row // that is already current would offer a choice with no outcome, and diff --git a/containers/libreportal/frontend/components/updater/css/updater.css b/containers/libreportal/frontend/components/updater/css/updater.css index 0d97150..3997a84 100644 --- a/containers/libreportal/frontend/components/updater/css/updater.css +++ b/containers/libreportal/frontend/components/updater/css/updater.css @@ -204,3 +204,24 @@ @media (max-width: 600px) { .updater-row { grid-template-columns: 1fr; gap: 6px; } } + +/* Busy face for action buttons (Update / Upgrade / Roll back) while their task + runs. The spinner borrows the global spin keyframes; currentColor keeps it + legible on primary, success and plain buttons alike without per-variant + colours. cursor: progress rather than not-allowed — the work IS happening, + the button is not refusing. */ +.updater-btn .btn-spin { + display: inline-block; + width: 11px; + height: 11px; + margin-right: 6px; + vertical-align: -1px; + border: 2px solid rgba(var(--text-rgb), 0.25); + border-top-color: currentColor; + border-radius: 50%; + animation: spin 0.8s linear infinite; +} +.updater-btn-busy { + opacity: 0.75; + cursor: progress; +} diff --git a/containers/libreportal/frontend/components/updater/js/updater-page.js b/containers/libreportal/frontend/components/updater/js/updater-page.js index b97d53f..fdb229a 100644 --- a/containers/libreportal/frontend/components/updater/js/updater-page.js +++ b/containers/libreportal/frontend/components/updater/js/updater-page.js @@ -19,6 +19,7 @@ class UpdaterPage { this._pushedAnyTab = false; this._eventBound = false; this._poll = null; // auto-refresh timer (startAutoRefresh/dispose) + this.inflight = new Map(); // app name -> { verb, taskId } while its task runs } // ---- lifecycle ----------------------------------------------------------- @@ -307,8 +308,9 @@ class UpdaterPage { this.dispatch('updater_check', {}, 'Checking apps for updates & vulnerabilities…'); } applyUpdate(app) { - if (!app) return; - this.dispatch('updater_apply', { app }, `Updating ${app} (a recovery snapshot is taken first)…`, { follow: true }); + if (!app || this.inflight.has(app)) return; // one task per app at a time + this.dispatch('updater_apply', { app }, `Updating ${app} (a recovery snapshot is taken first)…`, + { track: { apps: [app], verb: 'update' } }); } applyAll() { const list = this.apps.filter(a => a.update_available).map(a => a.name); @@ -325,17 +327,19 @@ class UpdaterPage { const want = new Set(names || []); const list = this.apps.filter(a => a.update_available && want.has(a.name)).map(a => a.name); if (!list.length) { this.toast('Everything is up to date.', 'info'); return; } - this.dispatch('updater_apply_all', { apps: list.join(',') }, `Updating ${list.length} app(s) — each is snapshotted first…`, { follow: true }); + this.dispatch('updater_apply_all', { apps: list.join(',') }, `Updating ${list.length} app(s) — each is snapshotted first…`, + { track: { apps: list, verb: 'update' } }); } rollback(app) { - if (!app) return; - this.dispatch('updater_rollback', { app }, `Rolling ${app} back to its pre-update snapshot…`); + if (!app || this.inflight.has(app)) return; + this.dispatch('updater_rollback', { app }, `Rolling ${app} back to its pre-update snapshot…`, + { track: { apps: [app], verb: 'rollback' } }); } // Cross-version upgrade. Always confirmed, and never quietly: this walks // real migrations one release at a time and can take a long while, so the // dialog states the plan and the guarantee rather than asking "are you sure?". upgrade(app, version) { - if (!app) return; + if (!app || this.inflight.has(app)) return; const a = this.apps.find((x) => x.name === app) || {}; const from = a.channel || a.current_version || 'the current version'; const to = version || a.newer_version || 'the newest release'; @@ -354,7 +358,7 @@ class UpdaterPage { migration. Watch it in Tasks.

`; const go = () => this.dispatch('updater_upgrade', { app, version: version || '' }, - `Upgrading ${app} to ${to}, one release at a time…`, { follow: true }); + `Upgrading ${app} to ${to}, one release at a time…`, { track: { apps: [app], verb: 'upgrade' } }); if (window.showConfirmation) { window.showConfirmation(`Upgrade ${label} to ${to}?`, '', go, 'Start upgrade', 'Cancel', 'warning', false, '', body, `/core/icons/apps/${app}.svg`); @@ -376,7 +380,7 @@ class UpdaterPage { if (route && typeof route.routeAction === 'function') { const started = route.routeAction(action, params || {}); this.toast(note || 'Working…', 'info'); - if (opts && opts.follow) this.followTask(started); + if (opts && opts.track) this.trackTask(opts.track.apps, opts.track.verb, started); } else if (typeof route === 'function') { route(action, params || {}); this.toast(note || 'Working…', 'info'); @@ -385,28 +389,75 @@ class UpdaterPage { } } - // Take the user to the task they just started, and bring them back when it - // lands. Pressing Update used to produce only a toast: the row did not change, - // nothing on the page moved, and the work was real but invisible. + // In-place feedback for a started task. Pressing Update used to produce only + // a toast: the row did not change, nothing on the page moved, and the work was + // real but invisible — the button read as though it had done nothing. Now the + // button itself becomes "Updating…" with a spinner and stays disabled until + // the task reaches a terminal state, at which point the data is refetched and + // the row repaints showing the result. (An earlier iteration navigated to the + // tasks page and back instead; superseded — watching the button beats being + // moved around, and the tasks page is one click away for whoever wants logs.) // // Correlated by task id rather than by app name — routeAction resolves to the - // created task — so a second update started elsewhere cannot return this one - // early, and a completion for some unrelated app cannot either. - // - // Deliberately does NOT lock navigation. The task is a background job with its - // own snapshot and rollback; it does not need to be watched, and a lock would - // strand the user on this page if it ever hung. So if they navigate away, that - // is a choice: the return only fires while they are still on a tasks page. - followTask(started) { - if (!window.spaClean || typeof window.spaClean.navigate !== 'function') return; - const back = window.location.pathname.startsWith('/apps/overview') - ? window.location.pathname - : '/apps/overview/updates'; + // created task — so a second task started elsewhere cannot end this one's + // busy state early, and an unrelated app's completion cannot either. + _busyLabel(verb) { + return { update: 'Updating…', upgrade: 'Upgrading…', rollback: 'Rolling back…' }[verb] || 'Working…'; + } + + // One action button, busy-aware. Every renderer builds these through here so + // a repaint that lands MID-task (the auto-refresh poll, a filter change) + // reconstructs the busy state instead of silently re-enabling the button. + actionBtn(app, verb, label, opts) { + const o = opts || {}; + const f = this.inflight.get(app); + const busy = !!(f && f.verb === verb); + // A SIBLING action is held while any task runs on the app — Roll back on an + // app that is mid-update is not a thing to offer. Disabled but no spinner: + // the spinner marks the action that is running, not the ones waiting on it. + const held = !!(f && f.verb !== verb); + return ``; + } + + // Patch buttons already in the DOM, both on this page and on the fleet + // Overview (they share the data-updater-action markup). This is what makes + // the click feel acknowledged INSTANTLY — no waiting for a re-render pass. + // The original face is stashed on the element so clearing restores it even + // when no repaint follows (e.g. the dispatch itself failed). + _paintInflight() { + document.querySelectorAll('button[data-updater-action][data-app]').forEach((btn) => { + const f = this.inflight.get(btn.dataset.app); + const busy = !!(f && f.verb === btn.dataset.updaterAction); + const held = !!(f && f.verb !== btn.dataset.updaterAction); + if (held && !btn.dataset.busyOrig) { btn.disabled = true; btn.dataset.held = '1'; } + else if (!held && btn.dataset.held) { btn.disabled = false; delete btn.dataset.held; } + if (busy && !btn.dataset.busyOrig) { + btn.dataset.busyOrig = btn.innerHTML; + btn.innerHTML = `${this._busyLabel(btn.dataset.updaterAction)}`; + btn.classList.add('updater-btn-busy'); + btn.disabled = true; + } else if (!busy && btn.dataset.busyOrig) { + btn.innerHTML = btn.dataset.busyOrig; + delete btn.dataset.busyOrig; + btn.classList.remove('updater-btn-busy'); + btn.disabled = false; + } + }); + } + + trackTask(apps, verb, started) { + const names = (Array.isArray(apps) ? apps : [apps]).filter(Boolean); + if (!names.length) return; + names.forEach((n) => this.inflight.set(n, { verb, taskId: null })); + this._paintInflight(); + const clear = () => names.forEach((n) => this.inflight.delete(n)); Promise.resolve(started).then((task) => { const id = task && (task.id || task.taskId); - if (!id) return; // no id, nothing to follow or match - window.spaClean.navigate(window.taskPath ? window.taskPath('all', id) : `/tasks/all/${id}`); + if (!id) { clear(); this._paintInflight(); return; } // nothing to correlate on + names.forEach((n) => this.inflight.set(n, { verb, taskId: String(id) })); let done = false; const finish = () => { @@ -420,19 +471,23 @@ class UpdaterPage { const d = e && e.detail; if (!d || String(d.taskId) !== String(id)) return; const st = (d.status || (d.task && d.task.status) || '').toLowerCase(); - // taskUpdated also fires mid-run; only a terminal state ends the wait. + // taskUpdated also fires mid-run; only a terminal state ends the busy face. if (st && !['completed', 'failed', 'cancelled'].includes(st)) return; finish(); - if (!window.location.pathname.startsWith('/tasks')) return; // they left; leave them - window.spaClean.navigate(back); + clear(); + // Refetch, then repaint: render() no-ops when this page is embedded in + // the fleet Overview, whose own refresh coordinator repaints its rows. + this.refreshAll().then(() => { this._paintInflight(); this.render(); }); }; window.addEventListener('taskCompleted', onEvent); window.addEventListener('taskUpdated', onEvent); - // A task that never reports terminal must not leak two listeners forever. - const timer = setTimeout(finish, 30 * 60 * 1000); - }).catch(() => { /* the dispatch itself already surfaced the failure */ }); + // A task that never reports terminal must not pin a spinner (or leak two + // listeners) forever. + const timer = setTimeout(() => { finish(); clear(); this._paintInflight(); }, 30 * 60 * 1000); + }).catch(() => { clear(); this._paintInflight(); }); } + toast(msg, type) { const n = this.services.notify; if (n && typeof n.show === 'function') n.show(msg, type || 'info'); @@ -523,7 +578,7 @@ class UpdaterPage { : (a.scanned ? `up to date` : `unscanned`); const sev = a.worstSeverity ? `${a.worstSeverity}` : ''; const btn = a.update_available - ? `` + ? this.actionBtn(a.name, 'update', 'Update', { cls: 'updater-btn updater-btn-primary' }) : ''; return `
${this.escape(a.displayName)} ${badge} ${sev}
@@ -611,7 +666,7 @@ class UpdaterPage {
${this.escape(a.displayName)} ${can ? 'recoverable' : 'protected'}
${snap}
-
${can ? `` : ''}
+
${can ? `${this.actionBtn(a.name, 'rollback', 'Roll back')}` : ''}
`; }).join(''); return ` @@ -666,7 +721,7 @@ class UpdaterPage { // exists and what to do about it. const newerLine = a.newer_version ? `
A newer release line is available: ${this.escape(a.newer_version)} (you track ${this.escape(a.channel || '—')}). Automatic updates keep you current within your line. Upgrading walks the releases one at a time, snapshotting and verifying each — read the release notes first. -
` + ${this.actionBtn(a.name, 'upgrade', `Upgrade to ${this.escape(a.newer_version)}`, { extra: ` data-version="${this.escape(a.newer_version)}"` })}` : ''; // Maintenance, which "up to date" cannot express. Phrased as an // observation with a date, not an accusation — plenty of small tools are @@ -719,7 +774,7 @@ class UpdaterPage { const recovery = `

Recovery

${can ? 'recoverable' : 'protected'} ${snap} - ${can ? `` : ''}
`; + ${can ? `${this.actionBtn(a.name, 'rollback', 'Roll back')}` : ''}`; const entries = ((this.history && this.history.entries) || []).filter((e) => e.app === a.name).slice(0, 8); const history = entries.length ? `

History

${entries.map((e) => `