From d887cd3b02704011e64e0a9c4e0d9483a0a772f2 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Wed, 5 Aug 2026 09:20:24 +0200 Subject: [PATCH 1/4] feat(gui): fixed action toasts for dashboard sync + model-apply feedback Sync and model-apply feedback now render as a transient fixed toast (bottom-right, out of the layout flow) instead of an inline notice that pushed the grid/panels down on every action. The toast auto-dismisses and re-arms on each new action. Quota warn/amber data bars drop the green-to-amber gradient (the mint stop read as an AI-cyan tell on dark) for a flat amber tone, and the providers overview mutes --fg-muted to the design token so muted labels clear WCAG 4.5:1 (3.5:1 before). --- gui/src/pages/Models.tsx | 19 ++- gui/src/pages/dashboard-overview-sections.tsx | 129 +++++++++++------- gui/src/styles.css | 27 +++- .../styles/provider-overview-dashboard.css | 3 + gui/src/styles/provider-quota.css | 4 +- gui/tests/dashboard-sync-feedback.test.tsx | 108 +++++++++++++++ gui/tests/models-status-toast.test.tsx | 116 ++++++++++++++++ 7 files changed, 354 insertions(+), 52 deletions(-) create mode 100644 gui/tests/dashboard-sync-feedback.test.tsx create mode 100644 gui/tests/models-status-toast.test.tsx diff --git a/gui/src/pages/Models.tsx b/gui/src/pages/Models.tsx index e6ddb3726..9fc281220 100644 --- a/gui/src/pages/Models.tsx +++ b/gui/src/pages/Models.tsx @@ -1,6 +1,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Switch, Notice, EmptyState, Select, Tooltip } from "../ui"; -import { IconChevron, IconBoxes, IconInfo, IconShuffle } from "../icons"; +import { IconChevron, IconBoxes, IconInfo, IconShuffle, IconCheck, IconAlert } from "../icons"; import { useT } from "../i18n/shared"; import type { TFn, TKey } from "../i18n/shared"; import { modelLabel } from "../model-display"; @@ -81,6 +81,16 @@ export default function Models({ apiBase }: { apiBase: string }) { const needsDefaultCollapseRef = useRef(initialCollapsed === null); const [status, setStatus] = useState(""); const [ok, setOk] = useState(false); + // Transient action feedback as a fixed toast: appearing or auto-clearing it never shifts + // the workspace below (the old inline Notice pushed the whole model grid down by its + // height on every apply). Every mutation already clears `status` first, so a new action + // re-arms the hold timer; the timer itself just clears the status again. + useEffect(() => { + if (!status) return; + const holdMs = ok ? 6000 : 8000; + const timer = setTimeout(() => setStatus(""), holdMs); + return () => clearTimeout(timer); + }, [status, ok]); const [busy, setBusy] = useState(false); const busyRef = useRef(false); const loadGenerationRef = useRef(0); @@ -1343,7 +1353,12 @@ export default function Models({ apiBase }: { apiBase: string }) {

{t("models.subtitle")}

- {status && {status}} + {status && ( +
+ {ok ? : } + {status} +
+ )} {/* Keep the last-good catalog interactive but make a failed revalidation explicit. */} {catalogState.showError && {t("models.loadFail")}}
diff --git a/gui/src/pages/dashboard-overview-sections.tsx b/gui/src/pages/dashboard-overview-sections.tsx index d51ef4a07..098f6c20c 100644 --- a/gui/src/pages/dashboard-overview-sections.tsx +++ b/gui/src/pages/dashboard-overview-sections.tsx @@ -1,4 +1,5 @@ -import { IconAlert, IconInfo, IconRefresh } from "../icons"; +import { useEffect, useRef, useState } from "react"; +import { IconAlert, IconCheck, IconInfo, IconRefresh } from "../icons"; import { Trans } from "../i18n/provider"; import { Select } from "../ui"; import { navigateHash } from "../hash-routing"; @@ -147,42 +148,85 @@ export function DashboardMaintenancePanel({ d }: { d: Dash }) { syncResult, syncError, updateJob, reconnecting, } = d; + // Sync feedback is a transient fixed toast instead of an inline notice: the toast sits + // outside the layout flow, so the result can appear without pushing the panels below + // this card down by a full box height (the old notice shifted the whole dashboard on + // every sync click). It auto-dismisses; a new sync clears and re-arms it. + const [syncToastDismissed, setSyncToastDismissed] = useState(false); + const syncToastTimerRef = useRef | null>(null); + + useEffect(() => { + if (syncToastTimerRef.current) { + clearTimeout(syncToastTimerRef.current); + syncToastTimerRef.current = null; + } + if (syncResult || syncError) { + const holdMs = syncError ? 8000 : 6000; + syncToastTimerRef.current = setTimeout(() => { + syncToastTimerRef.current = null; + setSyncToastDismissed(true); + }, holdMs); + } + return () => { + if (syncToastTimerRef.current) clearTimeout(syncToastTimerRef.current); + }; + }, [syncResult, syncError]); + + // A fresh click re-arms the toast even if the previous one was already auto-dismissed. + const handleRunSync = () => { + setSyncToastDismissed(false); + void runSync(); + }; + return ( -
- {/* Same one-row chrome as Sub-agent delegation: copy left, action right. */} -
-
-
{t("dash.syncModels")}
-
{t("dash.syncModelsHint")}
-
-
- - {/* - The update flow lives in the sidebar footer, which reports whether one is waiting - and is reachable from every page. A second button here duplicated it without - adding that signal. The trigger stays as a zero-size anchor so the deep link - (`#dashboard/update`) still has something to open against and the dialog has a - focus target to return to on close. - */} - + {/* + The update flow lives in the sidebar footer, which reports whether one is waiting + and is reachable from every page. A second button here duplicated it without + adding that signal. The trigger stays as a zero-size anchor so the deep link + (`#dashboard/update`) still has something to open against and the dialog has a + focus target to return to on close. + */} +
+ {updateJob && ( +
+ {updateJob.status === "failed" ? : } + + {updateJobLabel(updateJob.status, t)} + {updateJob.latestVersion ? ` ${updateJob.currentVersion} -> ${updateJob.latestVersion}.` : ""} + {reconnecting ? ` ${t("dash.updateReconnecting")}` : ""} + {updateJob.error ? ` ${updateJob.error}` : ""} + +
+ )}
- {syncResult && ( -
- {syncResult.nativeSubagentDefaultsWarning ? : } + {!syncToastDismissed && syncResult && ( +
+ {syncResult.nativeSubagentDefaultsWarning ? : } {t("dash.syncOk", { count: syncResult.added })} {syncResult.warning ? ` ${syncResult.warning}` : ""} @@ -191,23 +235,12 @@ export function DashboardMaintenancePanel({ d }: { d: Dash }) {
)} - {syncError && ( -
+ {!syncToastDismissed && syncError && ( +
{t("dash.syncFailed", { error: syncError })}
)} - {updateJob && ( -
- {updateJob.status === "failed" ? : } - - {updateJobLabel(updateJob.status, t)} - {updateJob.latestVersion ? ` ${updateJob.currentVersion} -> ${updateJob.latestVersion}.` : ""} - {reconnecting ? ` ${t("dash.updateReconnecting")}` : ""} - {updateJob.error ? ` ${updateJob.error}` : ""} - -
- )} -
+ ); } diff --git a/gui/src/styles.css b/gui/src/styles.css index 31ab807b4..4d8230111 100644 --- a/gui/src/styles.css +++ b/gui/src/styles.css @@ -687,6 +687,30 @@ a.btn, a.btn:hover { text-decoration: none; } overflow: hidden; } .maintenance-notice { margin: 14px 0 0; align-items: flex-start; } +/* + Transient action feedback (sync result, model-apply confirmation, …) as a fixed toast. + It is out of the layout flow, so appearing or dismissing it can never shift the page the + way an inline notice did (that box pushed every panel below it down by its full height on + each action). It sits bottom-right of the viewport, clear of the content column, and + reuses the notice tones. +*/ +.action-toast { + position: fixed; + right: var(--space-6); + bottom: var(--space-6); + z-index: var(--z-modal); + margin: 0; + max-width: min(480px, calc(100vw - 48px)); + box-shadow: var(--shadow); + align-items: flex-start; + animation: sync-toast-in var(--motion-normal) ease-out; +} +@keyframes sync-toast-in { + from { opacity: 0; transform: translateY(8px); } + to { opacity: 1; transform: translateY(0); } +} +/* Refresh icon spins while a sync is in flight (the `.spin` keyframes already exist). */ +.spin-icon { animation: spin 0.9s linear infinite; } .update-row { display: flex; align-items: center; justify-content: space-between; gap: 12px; margin-bottom: 14px; } .update-row .field-label { margin: 0; } .update-empty { padding: 18px; margin-bottom: 14px; } @@ -1325,7 +1349,8 @@ dialog.modal-overlay::backdrop { transition: transform var(--motion-normal); } .bar-green { background: var(--green); } -.bar-amber { background: linear-gradient(90deg, var(--green), var(--amber)); } +/* Flat, matching .bar-warn: no gradients on data bars (FE-GRADIENT-02 / AI-tell audit). */ +.bar-amber { background: var(--amber); } .quota-row--skeleton { min-height: 18px; } .quota-skel { display: inline-block; diff --git a/gui/src/styles/provider-overview-dashboard.css b/gui/src/styles/provider-overview-dashboard.css index e25831fff..be875ef1d 100644 --- a/gui/src/styles/provider-overview-dashboard.css +++ b/gui/src/styles/provider-overview-dashboard.css @@ -1,6 +1,9 @@ /* ProviderOverviewDashboard — aggregate overview (Phase 010) */ .pws-dashboard { + /* Muted labels here must clear WCAG 4.5:1 in both themes; the old `var(--fg-muted, #888)` + fallback only reached ~3.5:1 on white. Alias the design-system muted token instead. */ + --fg-muted: var(--muted); display: flex; flex-direction: column; gap: 14px; diff --git a/gui/src/styles/provider-quota.css b/gui/src/styles/provider-quota.css index bc990d682..05f616b20 100644 --- a/gui/src/styles/provider-quota.css +++ b/gui/src/styles/provider-quota.css @@ -1,7 +1,9 @@ /* Quota rows: stacked overview layout + warn/exhausted states (WP070). */ +/* Flat warn tone, not a gradient: gradients on data bars read as an AI tell and break the + flat-surface grammar (FE-GRADIENT-02). Warn matches the amber label/value tones. */ .bar-warn { - background: linear-gradient(90deg, var(--green), var(--amber)); + background: var(--amber); } .quota-row--warn .quota-label { diff --git a/gui/tests/dashboard-sync-feedback.test.tsx b/gui/tests/dashboard-sync-feedback.test.tsx new file mode 100644 index 000000000..f3f41790a --- /dev/null +++ b/gui/tests/dashboard-sync-feedback.test.tsx @@ -0,0 +1,108 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import type { Root } from "react-dom/client"; +import { en } from "../src/i18n/en"; +import { DashboardMaintenancePanel } from "../src/pages/dashboard-overview-sections"; +import type { useDashboardData } from "../src/pages/use-dashboard-data"; + +const globals = ["document", "window", "navigator", "IS_REACT_ACT_ENVIRONMENT"] as const; +let previousGlobals: Record<(typeof globals)[number], PropertyDescriptor | undefined>; +let testWindow: Window; +let host: HTMLElement; +let root: Root | null = null; + +type Dash = ReturnType; + +beforeEach(() => { + previousGlobals = Object.fromEntries( + globals.map((key) => [key, Object.getOwnPropertyDescriptor(globalThis, key)]), + ) as typeof previousGlobals; + root = null; + testWindow = new Window({ url: "http://localhost/" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow }, + navigator: { configurable: true, value: testWindow.navigator }, + }); + (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + host = testWindow.document.createElement("div") as unknown as HTMLElement; + testWindow.document.body.appendChild(host as never); +}); + +afterEach(async () => { + try { + if (root) { + const current = root; + await act(async () => { current.unmount(); }); + } + } finally { + root = null; + testWindow.close(); + for (const key of globals) { + const descriptor = previousGlobals[key]; + if (descriptor) Object.defineProperty(globalThis, key, descriptor); + else Reflect.deleteProperty(globalThis, key); + } + } +}); + +function dash(overrides: Partial = {}): Dash { + return { + t: (key: keyof typeof en) => en[key], + runSync: async () => {}, + syncing: false, + updateTriggerRef: { current: null }, + openUpdateDialog: () => {}, + updateLoading: false, + updateOpen: false, + syncResult: null, + syncError: null, + updateJob: null, + reconnecting: false, + ...overrides, + } as unknown as Dash; +} + +async function mount(d: Dash) { + const { createRoot } = await import("react-dom/client"); + await act(async () => { + root = createRoot(host); + root.render(); + }); +} + +test("renders sync feedback as a fixed toast, not an inline notice under the row", async () => { + await mount(dash({ + syncResult: { + ok: true, + added: 3, + catalogPath: null, + catalogExists: false, + cacheSynced: true, + message: "ok", + }, + })); + + const toast = host.querySelector(".action-toast"); + expect(toast).not.toBeNull(); + expect(toast!.className).toContain("notice-ok"); + expect(toast!.getAttribute("role")).toBe("status"); + expect(toast!.textContent).toContain("Sync complete"); + // The toast lives outside the panel so it cannot push the panel's content around. + expect(host.querySelector(".maintenance-panel .sync-toast")).toBeNull(); + // The old inline notice below the row is gone. + expect(host.querySelector(".maintenance-notice")).toBeNull(); + // The row itself is still there. + expect(host.querySelector(".dash-sync-summary")).not.toBeNull(); +}); + +test("renders sync errors as an error-toned toast", async () => { + await mount(dash({ syncError: "boom" })); + + const toast = host.querySelector(".action-toast"); + expect(toast).not.toBeNull(); + expect(toast!.className).toContain("notice-err"); + expect(toast!.textContent).toContain("Sync failed"); + expect(host.querySelector(".maintenance-notice")).toBeNull(); +}); diff --git a/gui/tests/models-status-toast.test.tsx b/gui/tests/models-status-toast.test.tsx new file mode 100644 index 000000000..d3953c5c0 --- /dev/null +++ b/gui/tests/models-status-toast.test.tsx @@ -0,0 +1,116 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import type { Root } from "react-dom/client"; +import { clearClientResourceStoresForTests } from "../src/client-resource"; +import { LanguageProvider } from "../src/i18n/provider"; +import Models from "../src/pages/Models"; + +const globals = [ + "document", "window", "navigator", "localStorage", "sessionStorage", + "IS_REACT_ACT_ENVIRONMENT", "setInterval", "clearInterval", +] as const; +let previousGlobals: Record<(typeof globals)[number], PropertyDescriptor | undefined>; +let testWindow: Window; +let container: HTMLElement; +let root: Root | null = null; + +beforeEach(() => { + previousGlobals = Object.fromEntries( + globals.map((key) => [key, Object.getOwnPropertyDescriptor(globalThis, key)]), + ) as typeof previousGlobals; + root = null; + testWindow = new Window({ url: "http://localhost/" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow }, + navigator: { configurable: true, value: testWindow.navigator }, + localStorage: { configurable: true, value: testWindow.localStorage }, + sessionStorage: { configurable: true, value: testWindow.sessionStorage }, + IS_REACT_ACT_ENVIRONMENT: { configurable: true, value: true }, + setInterval: { configurable: true, value: () => 1 }, + clearInterval: { configurable: true, value: () => {} }, + }); + testWindow.localStorage.setItem("ocx-models-collapsed:v2", JSON.stringify([])); + testWindow.sessionStorage.setItem("ocx.models.catalog.v1:http://localhost", JSON.stringify({ + models: [ + { provider: "anthropic", id: "claude-sonnet-5", namespaced: "anthropic/claude-sonnet-5", disabled: false }, + { provider: "anthropic", id: "claude-opus-4-5", namespaced: "anthropic/claude-opus-4-5", disabled: false }, + ], + providers: [{ name: "anthropic", liveModels: true, models: ["claude-sonnet-5", "claude-opus-4-5"] }], + selectedModels: {}, + disabled: [], + contextCaps: {}, + contextCapValue: 350_000, + })); + globalThis.fetch = (async (input, init) => { + const url = String(input); + if (url.endsWith("/api/models")) { + return Response.json([ + { provider: "anthropic", id: "claude-sonnet-5", namespaced: "anthropic/claude-sonnet-5", disabled: false }, + { provider: "anthropic", id: "claude-opus-4-5", namespaced: "anthropic/claude-opus-4-5", disabled: false }, + ]); + } + if (url.endsWith("/api/providers")) return Response.json([{ name: "anthropic", liveModels: true, models: ["claude-sonnet-5", "claude-opus-4-5"] }]); + if (url.endsWith("/api/selected-models")) return Response.json({ selected: {} }); + if (url.endsWith("/api/provider-context-caps")) return Response.json({ caps: {} }); + if (url.endsWith("/api/combos")) return Response.json({ combos: [] }); + if (url.endsWith("/api/shadow-call-settings")) return Response.json({ enabled: false, model: "" }); + if (url.endsWith("/api/v2")) return Response.json({ enabled: false, agentsMaxThreadsConflict: false, multiAgentMode: "default" }); + if (url.endsWith("/api/model-visibility") && init?.method === "PUT") return Response.json({ ok: true }); + return new Response(null, { status: 404 }); + }) as typeof fetch; + container = testWindow.document.createElement("div"); + testWindow.document.body.appendChild(container as never); +}); + +afterEach(async () => { + clearClientResourceStoresForTests(); + try { + if (root) { + const current = root; + await act(async () => { current.unmount(); }); + } + } finally { + root = null; + testWindow.close(); + for (const key of globals) { + const descriptor = previousGlobals[key]; + if (descriptor) Object.defineProperty(globalThis, key, descriptor); + else Reflect.deleteProperty(globalThis, key); + } + } +}); + +test("apply feedback renders as a fixed toast, not an inline notice before the workspace", async () => { + const { createRoot } = await import("react-dom/client"); + await act(async () => { + root = createRoot(container); + root.render( + + + , + ); + }); + await act(async () => { + await new Promise(resolve => testWindow.setTimeout(resolve, 0)); + await Promise.resolve(); + }); + + // Hide the whole provider group, the same action that used to pop the inline notice. + await act(async () => { + const off = [...container.querySelectorAll("button")].find(b => b.textContent === "All off")!; + off.click(); + await new Promise(resolve => testWindow.setTimeout(resolve, 0)); + await Promise.resolve(); + }); + + const toast = container.querySelector(".action-toast"); + expect(toast).not.toBeNull(); + expect(toast!.className).toContain("notice-ok"); + expect(toast!.getAttribute("role")).toBe("status"); + expect(toast!.textContent).toContain("Applied"); + // No inline notice sits in the flow before the workspace anymore. + const workspace = container.querySelector(".models-workspace-root"); + expect(workspace?.previousElementSibling?.classList.contains("action-toast")).toBe(true); +}); From 8a66a10e10e85376f696337528035be6b4aead56 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Wed, 5 Aug 2026 09:55:13 +0200 Subject: [PATCH 2/4] fix(gui): address Codex review on sync feedback toasts - Publish toast dismissal to the dashboard data hook (clearSyncFeedback) so a dismissed sync result cannot remount as a fresh toast when the Overview panel unmounts and is revisited. - Keep warning-bearing sync results (native subagent defaults override, stale app-server hint) visible until the next sync or an explicit dismiss, with a dismiss affordance on the toast. - Regression tests for both behaviors plus the remount scenario. --- gui/src/pages/dashboard-overview-sections.tsx | 37 +++- gui/src/pages/use-dashboard-data.ts | 13 +- gui/src/styles.css | 23 +++ gui/tests/dashboard-sync-feedback.test.tsx | 189 +++++++++++++++++- 4 files changed, 249 insertions(+), 13 deletions(-) diff --git a/gui/src/pages/dashboard-overview-sections.tsx b/gui/src/pages/dashboard-overview-sections.tsx index 098f6c20c..998b42a36 100644 --- a/gui/src/pages/dashboard-overview-sections.tsx +++ b/gui/src/pages/dashboard-overview-sections.tsx @@ -1,5 +1,5 @@ import { useEffect, useRef, useState } from "react"; -import { IconAlert, IconCheck, IconInfo, IconRefresh } from "../icons"; +import { IconAlert, IconCheck, IconInfo, IconRefresh, IconX } from "../icons"; import { Trans } from "../i18n/provider"; import { Select } from "../ui"; import { navigateHash } from "../hash-routing"; @@ -145,13 +145,20 @@ export function DashboardInjectionPanel({ d }: { apiBase: string; d: Dash }) { export function DashboardMaintenancePanel({ d }: { d: Dash }) { const { t, runSync, syncing, updateTriggerRef, openUpdateDialog, updateLoading, updateOpen, - syncResult, syncError, updateJob, reconnecting, + syncResult, syncError, updateJob, reconnecting, clearSyncFeedback, } = d; + // A sync result that carries actionable guidance (native subagent defaults override or + // the stale app-server hint) is the ONLY place that warning is visible, so it must not + // vanish on a timer: it stays until the next sync or an explicit dismiss. + const syncHoldsWarning = !!syncResult && (!!syncResult.nativeSubagentDefaultsWarning || !!syncResult.staleAppServerHint); + // Sync feedback is a transient fixed toast instead of an inline notice: the toast sits // outside the layout flow, so the result can appear without pushing the panels below // this card down by a full box height (the old notice shifted the whole dashboard on - // every sync click). It auto-dismisses; a new sync clears and re-arms it. + // every sync click). Plain results auto-dismiss; a new sync clears and re-arms it. + // Dismissal is published to the dashboard data (clearSyncFeedback), not just a local + // flag, so switching tabs and back cannot resurrect a stale result as a fresh toast. const [syncToastDismissed, setSyncToastDismissed] = useState(false); const syncToastTimerRef = useRef | null>(null); @@ -160,17 +167,18 @@ export function DashboardMaintenancePanel({ d }: { d: Dash }) { clearTimeout(syncToastTimerRef.current); syncToastTimerRef.current = null; } - if (syncResult || syncError) { + if ((syncResult || syncError) && !syncHoldsWarning) { const holdMs = syncError ? 8000 : 6000; syncToastTimerRef.current = setTimeout(() => { syncToastTimerRef.current = null; setSyncToastDismissed(true); + clearSyncFeedback(); }, holdMs); } return () => { if (syncToastTimerRef.current) clearTimeout(syncToastTimerRef.current); }; - }, [syncResult, syncError]); + }, [syncResult, syncError, syncHoldsWarning, clearSyncFeedback]); // A fresh click re-arms the toast even if the previous one was already auto-dismissed. const handleRunSync = () => { @@ -178,6 +186,13 @@ export function DashboardMaintenancePanel({ d }: { d: Dash }) { void runSync(); }; + // Shared dismiss affordance for the sync toast: closes it locally AND clears the + // dashboard-level result so it cannot remount as fresh on the next Overview visit. + const dismissSyncToast = () => { + setSyncToastDismissed(true); + clearSyncFeedback(); + }; + return ( <>
@@ -217,7 +232,7 @@ export function DashboardMaintenancePanel({ d }: { d: Dash }) { {updateJob.status === "failed" ? : } {updateJobLabel(updateJob.status, t)} - {updateJob.latestVersion ? ` ${updateJob.currentVersion} -> ${updateJob.latestVersion}.` : ""} + {updateJob.latestVersion ? ` ${t("dash.updateVersionTransition", { currentVersion: updateJob.currentVersion, latestVersion: updateJob.latestVersion })}` : ""} {reconnecting ? ` ${t("dash.updateReconnecting")}` : ""} {updateJob.error ? ` ${updateJob.error}` : ""} @@ -225,19 +240,25 @@ export function DashboardMaintenancePanel({ d }: { d: Dash }) { )}
{!syncToastDismissed && syncResult && ( -
- {syncResult.nativeSubagentDefaultsWarning ? : } +
+ {syncHoldsWarning ? : } {t("dash.syncOk", { count: syncResult.added })} {syncResult.warning ? ` ${syncResult.warning}` : ""} {syncResult.nativeSubagentDefaultsWarning ? ` ${syncResult.nativeSubagentDefaultsWarning}` : ""} {syncResult.staleAppServerHint ? <>{" "} : null} +
)} {!syncToastDismissed && syncError && (
{t("dash.syncFailed", { error: syncError })} +
)} diff --git a/gui/src/pages/use-dashboard-data.ts b/gui/src/pages/use-dashboard-data.ts index 2f2924f02..0264781fa 100644 --- a/gui/src/pages/use-dashboard-data.ts +++ b/gui/src/pages/use-dashboard-data.ts @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useKeyedClientResource } from "../client-resource"; import { replaceHash } from "../hash-routing"; import { useI18n } from "../i18n/shared"; @@ -589,6 +589,15 @@ export function useDashboardData(apiBase: string) { } }; + // Clears the sync result/error in this hook. The dashboard toast owns its own dismissal + // timer but must publish the dismissal here: syncResult/syncError live above the dashboard + // tabs, so a component-local flag alone would let a stale result remount as a fresh toast + // after the Overview panel unmounts and comes back. + const clearSyncFeedback = useCallback(() => { + setSyncResult(null); + setSyncError(null); + }, []); + const runSync = async () => { if (syncing) return; setSyncing(true); @@ -737,7 +746,7 @@ export function useDashboardData(apiBase: string) { effortCapHelpTriggerRef, updateTriggerRef, maHelpTriggerRef, shadowCallHelpTriggerRef, effortCapHelpDialogRef, updateDialogRef, maHelpDialogRef, shadowCallHelpDialogRef, filteredGroups, sidecarModels, - saveSidecar, saveShadowCall, switchMaMode, toggleCodexAutoStart, runSync, + saveSidecar, saveShadowCall, switchMaMode, toggleCodexAutoStart, runSync, clearSyncFeedback, fetchUpdateCheck, closeUpdateDialog, openUpdateDialog, changeUpdateChannel, runUpdate, }; } diff --git a/gui/src/styles.css b/gui/src/styles.css index 4d8230111..5543f3646 100644 --- a/gui/src/styles.css +++ b/gui/src/styles.css @@ -711,6 +711,24 @@ a.btn, a.btn:hover { text-decoration: none; } } /* Refresh icon spins while a sync is in flight (the `.spin` keyframes already exist). */ .spin-icon { animation: spin 0.9s linear infinite; } +/* Toast dismiss affordance: quiet ghost button inside the notice row. */ +.action-toast-dismiss { + flex: none; + display: inline-flex; + align-items: center; + justify-content: center; + align-self: flex-start; + margin: -4px -6px 0 2px; + padding: 4px; + border: none; + background: none; + border-radius: var(--radius-sm); + color: var(--muted); + cursor: pointer; + transition: color var(--motion-fast), background var(--motion-fast); +} +.action-toast-dismiss:hover { color: var(--text); background: color-mix(in srgb, var(--muted) 14%, transparent); } +.action-toast-dismiss:focus-visible { outline: 2px solid var(--accent); outline-offset: 1px; } .update-row { display: flex; align-items: center; justify-content: space-between; gap: 12px; margin-bottom: 14px; } .update-row .field-label { margin: 0; } .update-empty { padding: 18px; margin-bottom: 14px; } @@ -2019,6 +2037,11 @@ button.prov-account-row.active { cursor: default; } .usage-segmented-btn { display: inline-flex; align-items: center; justify-content: center; gap: 6px; border: none; background: transparent; color: var(--muted); padding: 4px 12px; border-radius: var(--radius-pill); cursor: pointer; font: inherit; white-space: nowrap; } .usage-segmented-btn.active { background: var(--raised); color: var(--text); font-weight: var(--weight-semibold); } .usage-source-mark { width: var(--icon-sm); height: var(--icon-sm); flex: 0 0 auto; object-fit: contain; } +:root[data-theme="dark"] .usage-source-mark { filter: invert(1); } + +@media (prefers-color-scheme: dark) { + :root:not([data-theme="light"]) .usage-source-mark { filter: invert(1); } +} @media (max-width: 760px) { .usage-segmented-btn { min-height: var(--control-touch); } diff --git a/gui/tests/dashboard-sync-feedback.test.tsx b/gui/tests/dashboard-sync-feedback.test.tsx index f3f41790a..2aa474ea5 100644 --- a/gui/tests/dashboard-sync-feedback.test.tsx +++ b/gui/tests/dashboard-sync-feedback.test.tsx @@ -3,10 +3,11 @@ import { Window } from "happy-dom"; import { act } from "react"; import type { Root } from "react-dom/client"; import { en } from "../src/i18n/en"; +import { LanguageProvider } from "../src/i18n/provider"; import { DashboardMaintenancePanel } from "../src/pages/dashboard-overview-sections"; import type { useDashboardData } from "../src/pages/use-dashboard-data"; -const globals = ["document", "window", "navigator", "IS_REACT_ACT_ENVIRONMENT"] as const; +const globals = ["document", "window", "navigator", "IS_REACT_ACT_ENVIRONMENT", "setTimeout", "clearTimeout"] as const; let previousGlobals: Record<(typeof globals)[number], PropertyDescriptor | undefined>; let testWindow: Window; let host: HTMLElement; @@ -60,6 +61,7 @@ function dash(overrides: Partial = {}): Dash { syncError: null, updateJob: null, reconnecting: false, + clearSyncFeedback: () => {}, ...overrides, } as unknown as Dash; } @@ -67,11 +69,31 @@ function dash(overrides: Partial = {}): Dash { async function mount(d: Dash) { const { createRoot } = await import("react-dom/client"); await act(async () => { - root = createRoot(host); - root.render(); + if (!root) root = createRoot(host); + // The stale app-server hint renders , which needs the provider context. + root.render(); }); } +// Controlled global timers: the toast hold timer runs through the real global setTimeout +// (the component calls it bare), so tests can advance time deterministically instead of +// waiting 6-8 real seconds. clearTimeout is a no-op here — stale timers are filtered out +// by the hold duration when fired. +let scheduledTimers: Array<{ fn: () => void; ms: number }> = []; +function installFakeTimers() { + scheduledTimers = []; + globalThis.setTimeout = ((fn: () => void, ms?: number) => { + scheduledTimers.push({ fn, ms: ms ?? 0 }); + return scheduledTimers.length; + }) as typeof setTimeout; + globalThis.clearTimeout = (() => {}) as typeof clearTimeout; +} +async function fireTimers(ms: number) { + const due = scheduledTimers.filter(t => t.ms === ms); + scheduledTimers = scheduledTimers.filter(t => t.ms !== ms); + await act(async () => { for (const t of due) t.fn(); }); +} + test("renders sync feedback as a fixed toast, not an inline notice under the row", async () => { await mount(dash({ syncResult: { @@ -106,3 +128,164 @@ test("renders sync errors as an error-toned toast", async () => { expect(toast!.textContent).toContain("Sync failed"); expect(host.querySelector(".maintenance-notice")).toBeNull(); }); + +test("success toast auto-dismisses after 6s; error toast holds for 8s", async () => { + installFakeTimers(); + + await mount(dash({ + syncResult: { + ok: true, + added: 3, + catalogPath: null, + catalogExists: false, + cacheSynced: true, + message: "ok", + }, + })); + expect(host.querySelector(".action-toast")).not.toBeNull(); + await fireTimers(6000); + expect(host.querySelector(".action-toast")).toBeNull(); + + // Fresh mount so the success dismissal state cannot suppress the error toast. + await act(async () => { root?.unmount(); root = null; }); + // Error tone: still visible at the 6s success boundary, dismissed only at 8s. + await mount(dash({ syncError: "boom" })); + expect(host.querySelector(".action-toast")).not.toBeNull(); + await fireTimers(6000); + expect(host.querySelector(".action-toast")).not.toBeNull(); + await fireTimers(8000); + expect(host.querySelector(".action-toast")).toBeNull(); +}); + +test("warning-bearing results never auto-dismiss and offer an explicit dismiss", async () => { + installFakeTimers(); + + await mount(dash({ + syncResult: { + ok: true, + added: 3, + catalogPath: null, + catalogExists: false, + cacheSynced: true, + message: "ok", + nativeSubagentDefaultsWarning: "native defaults were not applied", + staleAppServerHint: "restart codex", + }, + })); + + const toast = host.querySelector(".action-toast"); + expect(toast).not.toBeNull(); + expect(toast!.className).toContain("notice-warn"); + // The stale-app-server hint and warning stay in the message. + expect(toast!.textContent).toContain("ocx sync --restart-codex"); + // Still visible well past the plain success 6s / error 8s hold times. + await fireTimers(6000); + await fireTimers(8000); + expect(host.querySelector(".action-toast")).not.toBeNull(); + + // The dismiss button closes it and calls clearSyncFeedback (the parent-level clear). + const dismiss = toast!.querySelector(".action-toast-dismiss"); + expect(dismiss).not.toBeNull(); + await act(async () => { dismiss!.click(); }); + expect(host.querySelector(".action-toast")).toBeNull(); +}); + +test("dismissing the sync toast clears the dashboard-level result, not just a local flag", async () => { + installFakeTimers(); + + const cleared: string[] = []; + await mount(dash({ + syncError: "boom", + clearSyncFeedback: () => { cleared.push("cleared"); }, + })); + const dismiss = host.querySelector(".action-toast-dismiss"); + expect(dismiss).not.toBeNull(); + await act(async () => { dismiss!.click(); }); + expect(cleared).toEqual(["cleared"]); + expect(host.querySelector(".action-toast")).toBeNull(); +}); + +test("a timer-dismissed result does not remount as a fresh toast after the panel unmounts", async () => { + installFakeTimers(); + + // Models the real data flow: syncResult lives in useDashboardData above the dashboard + // tabs, and clearSyncFeedback clears it there. A component-local dismissed flag alone + // would reset on unmount and resurrect the stale toast when the tab is revisited. + const state: { result: NonNullable | null; error: string | null } = { + result: { + ok: true, + added: 3, + catalogPath: null, + catalogExists: false, + cacheSynced: true, + message: "ok", + }, + error: null, + }; + const makeDash = () => dash({ + syncResult: state.result, + syncError: state.error, + clearSyncFeedback: () => { + state.result = null; + state.error = null; + }, + }); + + await mount(makeDash()); + expect(host.querySelector(".action-toast")).not.toBeNull(); + + // Auto-dismiss fires the timer, which clears the dashboard-level result. + await fireTimers(6000); + expect(host.querySelector(".action-toast")).toBeNull(); + expect(state.result).toBeNull(); + + // Tab switch unmounts the panel; remounting reads the (now cleared) parent state, so + // the stale result cannot reappear as a fresh toast. + await act(async () => { root?.unmount(); root = null; }); + await mount(makeDash()); + expect(host.querySelector(".action-toast")).toBeNull(); +}); + +test("a new sync re-arms a dismissed toast", async () => { + installFakeTimers(); + + let d: Dash = dash({ syncResult: null }); + await mount(d); + expect(host.querySelector(".action-toast")).toBeNull(); + + // First sync result: toast appears, then auto-dismisses. + d = dash({ + syncResult: { + ok: true, + added: 3, + catalogPath: null, + catalogExists: false, + cacheSynced: true, + message: "ok", + }, + }); + await mount(d); + expect(host.querySelector(".action-toast")).not.toBeNull(); + await fireTimers(6000); + expect(host.querySelector(".action-toast")).toBeNull(); + + // A fresh sync click (re-arms dismissed=false) with a new result re-shows the + // toast on a fresh 6s timer instead of staying hidden. + const syncButton = [...host.querySelectorAll(".maintenance-actions button")] + .find(b => !b.classList.contains("maintenance-update-anchor"))!; + await act(async () => { syncButton.click(); }); + d = dash({ + syncResult: { + ok: true, + added: 5, + catalogPath: null, + catalogExists: false, + cacheSynced: true, + message: "ok", + }, + }); + await mount(d); + expect(host.querySelector(".action-toast")).not.toBeNull(); + await fireTimers(6000); + expect(host.querySelector(".action-toast")).toBeNull(); +}); From 339b8a05fbf4c1ae608147ceec27ad505da7320d Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Wed, 5 Aug 2026 10:00:43 +0200 Subject: [PATCH 3/4] fix(gui): address CodeRabbit review findings - Localize the update version transition (dash.updateVersionTransition) in all six locale files. - Re-arm the Models action toast for repeated custom-model mutations by clearing status before each feedback-producing mutation, so identical results restart the dismissal timer. - Restore globalThis.fetch after the Models toast test and add controlled timer coverage for success/error dismissal and re-arm. --- gui/src/i18n/de.ts | 2 + gui/src/i18n/en.ts | 2 + gui/src/i18n/ja.ts | 2 + gui/src/i18n/ko.ts | 2 + gui/src/i18n/ru.ts | 2 + gui/src/i18n/zh.ts | 2 + gui/src/pages/Models.tsx | 34 +++++++------- gui/tests/models-status-toast.test.tsx | 61 ++++++++++++++++++++++++++ 8 files changed, 92 insertions(+), 15 deletions(-) diff --git a/gui/src/i18n/de.ts b/gui/src/i18n/de.ts index 639f00c7a..45d4b6c31 100644 --- a/gui/src/i18n/de.ts +++ b/gui/src/i18n/de.ts @@ -258,6 +258,8 @@ export const de: Record = { "dash.updateStatus.running": "opencodex wird aktualisiert.", "dash.updateStatus.restarting": "Update installiert. Proxy wird neu gestartet.", "dash.updateStatus.succeeded": "Update abgeschlossen.", + "dash.updateVersionTransition": "{currentVersion} -> {latestVersion}.", + "dash.updateStatus.failed": "Update fehlgeschlagen.", "prov.subtitle": "Konfiguriere die Upstream-Anbieter, die opencodex in Codex routet. Melde dich mit einem Konto an, füge einen Anbieter hinzu oder bearbeite die Rohkonfiguration.", "prov.add": "Anbieter hinzufügen", diff --git a/gui/src/i18n/en.ts b/gui/src/i18n/en.ts index cbfd40892..8680cbc15 100644 --- a/gui/src/i18n/en.ts +++ b/gui/src/i18n/en.ts @@ -273,6 +273,8 @@ export const en = { "dash.updateStatus.running": "Updating opencodex.", "dash.updateStatus.restarting": "Update installed. Restarting proxy.", "dash.updateStatus.succeeded": "Update finished.", + "dash.updateVersionTransition": "{currentVersion} -> {latestVersion}.", + "dash.updateStatus.failed": "Update failed.", // providers diff --git a/gui/src/i18n/ja.ts b/gui/src/i18n/ja.ts index 23df35de4..daa8e9d0c 100644 --- a/gui/src/i18n/ja.ts +++ b/gui/src/i18n/ja.ts @@ -267,6 +267,8 @@ export const ja: Record = { "dash.updateStatus.running": "opencodex を更新しています。", "dash.updateStatus.restarting": "更新をインストールしました。プロキシを再起動中。", "dash.updateStatus.succeeded": "更新が完了しました。", + "dash.updateVersionTransition": "{currentVersion} -> {latestVersion}.", + "dash.updateStatus.failed": "更新に失敗しました。", // providers diff --git a/gui/src/i18n/ko.ts b/gui/src/i18n/ko.ts index b08030411..cddda3f7a 100644 --- a/gui/src/i18n/ko.ts +++ b/gui/src/i18n/ko.ts @@ -265,6 +265,8 @@ export const ko: Record = { "dash.updateStatus.running": "opencodex 업데이트 중입니다.", "dash.updateStatus.restarting": "업데이트 설치 완료. 프록시를 재시작하는 중입니다.", "dash.updateStatus.succeeded": "업데이트가 완료됐습니다.", + "dash.updateVersionTransition": "{currentVersion} -> {latestVersion}.", + "dash.updateStatus.failed": "업데이트에 실패했습니다.", // providers diff --git a/gui/src/i18n/ru.ts b/gui/src/i18n/ru.ts index 98b7c6243..3aa6fb8bd 100644 --- a/gui/src/i18n/ru.ts +++ b/gui/src/i18n/ru.ts @@ -267,6 +267,8 @@ export const ru: Record = { "dash.updateStatus.running": "Обновление opencodex.", "dash.updateStatus.restarting": "Обновление установлено. Перезапуск прокси.", "dash.updateStatus.succeeded": "Обновление завершено.", + "dash.updateVersionTransition": "{currentVersion} -> {latestVersion}.", + "dash.updateStatus.failed": "Обновление не удалось.", // providers diff --git a/gui/src/i18n/zh.ts b/gui/src/i18n/zh.ts index c77a6fb6d..8ac70dd5a 100644 --- a/gui/src/i18n/zh.ts +++ b/gui/src/i18n/zh.ts @@ -262,6 +262,8 @@ export const zh: Record = { "dash.updateStatus.running": "正在更新 opencodex。", "dash.updateStatus.restarting": "更新已安装。正在重启代理。", "dash.updateStatus.succeeded": "更新完成。", + "dash.updateVersionTransition": "{currentVersion} -> {latestVersion}.", + "dash.updateStatus.failed": "更新失败。", // providers diff --git a/gui/src/pages/Models.tsx b/gui/src/pages/Models.tsx index 9fc281220..f2ff909c3 100644 --- a/gui/src/pages/Models.tsx +++ b/gui/src/pages/Models.tsx @@ -81,16 +81,25 @@ export default function Models({ apiBase }: { apiBase: string }) { const needsDefaultCollapseRef = useRef(initialCollapsed === null); const [status, setStatus] = useState(""); const [ok, setOk] = useState(false); + // Feedback generation: a repeated identical message (same success string, same validation + // error) must still re-arm the toast timer. Clearing `status` alone is not enough — a + // second identical value bails out of React's state diff, so the old timer would dismiss + // the new toast early. Every publish bumps the generation. + const [feedbackGen, setFeedbackGen] = useState(0); + const publishFeedback = (nextOk: boolean, message: string) => { + setOk(nextOk); + setStatus(message); + setFeedbackGen(g => g + 1); + }; // Transient action feedback as a fixed toast: appearing or auto-clearing it never shifts // the workspace below (the old inline Notice pushed the whole model grid down by its - // height on every apply). Every mutation already clears `status` first, so a new action - // re-arms the hold timer; the timer itself just clears the status again. + // height on every apply). The timer itself just clears the status again. useEffect(() => { if (!status) return; const holdMs = ok ? 6000 : 8000; const timer = setTimeout(() => setStatus(""), holdMs); return () => clearTimeout(timer); - }, [status, ok]); + }, [status, ok, feedbackGen]); const [busy, setBusy] = useState(false); const busyRef = useRef(false); const loadGenerationRef = useRef(0); @@ -440,7 +449,7 @@ export default function Models({ apiBase }: { apiBase: string }) { const applyCustomCap = () => { const value = Number(customCap.replace(/[_,\s]/g, "")); - if (!Number.isFinite(value) || value <= 0) { setOk(false); setStatus(t("models.capSaveFailed")); return; } + if (!Number.isFinite(value) || value <= 0) { publishFeedback(false, t("models.capSaveFailed")); return; } setShowCustom(false); setGlobalCap(value); }; @@ -506,7 +515,7 @@ export default function Models({ apiBase }: { apiBase: string }) { // (setMaxConcurrentThreads no-ops on equal value), so a re-selected current // value or a double click can never double-write config.toml. if (!v2 || v2BusyRef.current) return; - if (!Number.isInteger(value) || value < 1) { setOk(false); setStatus(t("models.v2ThreadsInvalid")); return; } + if (!Number.isInteger(value) || value < 1) { publishFeedback(false, t("models.v2ThreadsInvalid")); return; } if (v2.maxConcurrentThreadsPerSession === value) return; setV2Busy(true); v2BusyRef.current = true; @@ -591,8 +600,7 @@ export default function Models({ apiBase }: { apiBase: string }) { try { await readJsonOrThrow(r, t("models.customSaveFailed")); setCustomModalOpen(false); - setOk(true); - setStatus(t("models.customAdded")); + publishFeedback(true, t("models.customAdded")); await load(true); } catch (e) { setCustomError(e instanceof Error ? e.message : t("models.customSaveFailed")); @@ -616,8 +624,7 @@ export default function Models({ apiBase }: { apiBase: string }) { try { await readJsonOrThrow(r, t("models.customSaveFailed")); setCustomModalOpen(false); - setOk(true); - setStatus(t("models.customUpdated")); + publishFeedback(true, t("models.customUpdated")); await load(true); } catch (e) { setCustomError(e instanceof Error ? e.message : t("models.customSaveFailed")); @@ -633,16 +640,13 @@ export default function Models({ apiBase }: { apiBase: string }) { try { const r = await fetch(`${apiBase}/api/custom-models/${encodeURIComponent(id)}`, { method: "DELETE" }); if (r.ok) { - setOk(true); - setStatus(t("models.customDeleted")); + publishFeedback(true, t("models.customDeleted")); await load(true); } else { - setOk(false); - setStatus(t("models.customSaveFailed")); + publishFeedback(false, t("models.customSaveFailed")); } } catch { - setOk(false); - setStatus(t("models.networkError")); + publishFeedback(false, t("models.networkError")); } }; diff --git a/gui/tests/models-status-toast.test.tsx b/gui/tests/models-status-toast.test.tsx index d3953c5c0..29c902c2c 100644 --- a/gui/tests/models-status-toast.test.tsx +++ b/gui/tests/models-status-toast.test.tsx @@ -9,6 +9,7 @@ import Models from "../src/pages/Models"; const globals = [ "document", "window", "navigator", "localStorage", "sessionStorage", "IS_REACT_ACT_ENVIRONMENT", "setInterval", "clearInterval", + "setTimeout", "clearTimeout", "fetch", ] as const; let previousGlobals: Record<(typeof globals)[number], PropertyDescriptor | undefined>; let testWindow: Window; @@ -82,6 +83,25 @@ afterEach(async () => { } }); +// Controlled global timers: the toast hold timer runs through the real global setTimeout +// (the component calls it bare), so tests can advance time deterministically instead of +// waiting 6-8 real seconds. clearTimeout is a no-op here — stale timers are filtered out +// by the hold duration when fired. +let scheduledTimers: Array<{ fn: () => void; ms: number }> = []; +function installFakeTimers() { + scheduledTimers = []; + globalThis.setTimeout = ((fn: () => void, ms?: number) => { + scheduledTimers.push({ fn, ms: ms ?? 0 }); + return scheduledTimers.length; + }) as typeof setTimeout; + globalThis.clearTimeout = (() => {}) as typeof clearTimeout; +} +async function fireTimers(ms: number) { + const due = scheduledTimers.filter(t => t.ms === ms); + scheduledTimers = scheduledTimers.filter(t => t.ms !== ms); + await act(async () => { for (const t of due) t.fn(); }); +} + test("apply feedback renders as a fixed toast, not an inline notice before the workspace", async () => { const { createRoot } = await import("react-dom/client"); await act(async () => { @@ -114,3 +134,44 @@ test("apply feedback renders as a fixed toast, not an inline notice before the w const workspace = container.querySelector(".models-workspace-root"); expect(workspace?.previousElementSibling?.classList.contains("action-toast")).toBe(true); }); + +test("success toast expires after 6s and a repeated action re-arms it", async () => { + installFakeTimers(); + const { createRoot } = await import("react-dom/client"); + await act(async () => { + root = createRoot(container); + root.render( + + + , + ); + }); + await act(async () => { + await new Promise(resolve => testWindow.setTimeout(resolve, 0)); + await Promise.resolve(); + }); + + const offButton = () => [...container.querySelectorAll("button")].find(b => b.textContent === "All off")!; + const clickOff = async () => { + await act(async () => { + offButton().click(); + await new Promise(resolve => testWindow.setTimeout(resolve, 0)); + await Promise.resolve(); + }); + }; + + // First action: toast appears with a 6s hold timer. + await clickOff(); + expect(container.querySelector(".action-toast")).not.toBeNull(); + + // 6s elapse: auto-dismissed even though no new action happened. + await fireTimers(6000); + expect(container.querySelector(".action-toast")).toBeNull(); + + // The exact same action again: the toast re-arms (fresh 6s hold) instead of + // staying dismissed because the message value did not change. + await clickOff(); + expect(container.querySelector(".action-toast")).not.toBeNull(); + await fireTimers(6000); + expect(container.querySelector(".action-toast")).toBeNull(); +}); From f9fdb19a864c8f261bc1207a51685361ccba52ae Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Wed, 5 Aug 2026 10:12:42 +0200 Subject: [PATCH 4/4] fix(gui): address second CodeRabbit pass on sync toasts - Hold generic syncResult.warning results like other warnings: they get the warn tone and stay visible instead of auto-dismissing after 6s. - Floor the toast dismiss button hit area at 24px (13px icon + 4px padding was only 21px). - Model timer cancellation in the test fake-clock (clearTimeout now really cancels) and cover the re-arm case where a second result lands before the first hold expires. --- gui/src/pages/dashboard-overview-sections.tsx | 13 ++- gui/src/styles.css | 4 + gui/tests/dashboard-sync-feedback.test.tsx | 105 +++++++++++++++--- 3 files changed, 100 insertions(+), 22 deletions(-) diff --git a/gui/src/pages/dashboard-overview-sections.tsx b/gui/src/pages/dashboard-overview-sections.tsx index 998b42a36..b293884e6 100644 --- a/gui/src/pages/dashboard-overview-sections.tsx +++ b/gui/src/pages/dashboard-overview-sections.tsx @@ -148,10 +148,15 @@ export function DashboardMaintenancePanel({ d }: { d: Dash }) { syncResult, syncError, updateJob, reconnecting, clearSyncFeedback, } = d; - // A sync result that carries actionable guidance (native subagent defaults override or - // the stale app-server hint) is the ONLY place that warning is visible, so it must not - // vanish on a timer: it stays until the next sync or an explicit dismiss. - const syncHoldsWarning = !!syncResult && (!!syncResult.nativeSubagentDefaultsWarning || !!syncResult.staleAppServerHint); + // A sync result that carries actionable guidance (generic warning, native subagent + // defaults override, or the stale app-server hint) is the ONLY place that warning is + // visible, so it must not vanish on a timer: it stays until the next sync or an + // explicit dismiss. + const syncHoldsWarning = !!syncResult && ( + !!syncResult.warning + || !!syncResult.nativeSubagentDefaultsWarning + || !!syncResult.staleAppServerHint + ); // Sync feedback is a transient fixed toast instead of an inline notice: the toast sits // outside the layout flow, so the result can appear without pushing the panels below diff --git a/gui/src/styles.css b/gui/src/styles.css index 5543f3646..c133bef9e 100644 --- a/gui/src/styles.css +++ b/gui/src/styles.css @@ -718,6 +718,10 @@ a.btn, a.btn:hover { text-decoration: none; } align-items: center; justify-content: center; align-self: flex-start; + /* 13px icon + 4px padding = 21px: too small to hit comfortably. Floor the hit + target at 24px while keeping the icon size and visual padding. */ + min-width: 24px; + min-height: 24px; margin: -4px -6px 0 2px; padding: 4px; border: none; diff --git a/gui/tests/dashboard-sync-feedback.test.tsx b/gui/tests/dashboard-sync-feedback.test.tsx index 2aa474ea5..d8712b8b6 100644 --- a/gui/tests/dashboard-sync-feedback.test.tsx +++ b/gui/tests/dashboard-sync-feedback.test.tsx @@ -76,21 +76,33 @@ async function mount(d: Dash) { } // Controlled global timers: the toast hold timer runs through the real global setTimeout -// (the component calls it bare), so tests can advance time deterministically instead of -// waiting 6-8 real seconds. clearTimeout is a no-op here — stale timers are filtered out -// by the hold duration when fired. -let scheduledTimers: Array<{ fn: () => void; ms: number }> = []; +// (the component calls it bare), so tests advance a monotonic virtual clock instead of +// waiting 6-8 real seconds. Unlike a duration-matching stub, this models real timer +// semantics: clearTimeout cancels a scheduled callback, so a stale hold timer from an +// earlier result can never dismiss a newer toast. +let virtualClockMs = 0; +let nextTimerId = 1; +let scheduledTimers: Array<{ id: number; fn: () => void; at: number }> = []; function installFakeTimers() { + virtualClockMs = 0; + nextTimerId = 1; scheduledTimers = []; globalThis.setTimeout = ((fn: () => void, ms?: number) => { - scheduledTimers.push({ fn, ms: ms ?? 0 }); - return scheduledTimers.length; + const id = nextTimerId++; + scheduledTimers.push({ id, fn, at: virtualClockMs + (ms ?? 0) }); + return id; }) as typeof setTimeout; - globalThis.clearTimeout = (() => {}) as typeof clearTimeout; + globalThis.clearTimeout = ((id?: number) => { + if (id === undefined) return; + scheduledTimers = scheduledTimers.filter(t => t.id !== id); + }) as typeof clearTimeout; } -async function fireTimers(ms: number) { - const due = scheduledTimers.filter(t => t.ms === ms); - scheduledTimers = scheduledTimers.filter(t => t.ms !== ms); +async function advanceTime(ms: number) { + virtualClockMs += ms; + const due = scheduledTimers + .filter(t => t.at <= virtualClockMs) + .sort((a, b) => a.at - b.at); + scheduledTimers = scheduledTimers.filter(t => t.at > virtualClockMs); await act(async () => { for (const t of due) t.fn(); }); } @@ -143,7 +155,7 @@ test("success toast auto-dismisses after 6s; error toast holds for 8s", async () }, })); expect(host.querySelector(".action-toast")).not.toBeNull(); - await fireTimers(6000); + await advanceTime(6000); expect(host.querySelector(".action-toast")).toBeNull(); // Fresh mount so the success dismissal state cannot suppress the error toast. @@ -151,12 +163,36 @@ test("success toast auto-dismisses after 6s; error toast holds for 8s", async () // Error tone: still visible at the 6s success boundary, dismissed only at 8s. await mount(dash({ syncError: "boom" })); expect(host.querySelector(".action-toast")).not.toBeNull(); - await fireTimers(6000); + await advanceTime(6000); expect(host.querySelector(".action-toast")).not.toBeNull(); - await fireTimers(8000); + await advanceTime(8000); expect(host.querySelector(".action-toast")).toBeNull(); }); +test("a result with only a generic warning holds like other warnings", async () => { + installFakeTimers(); + + await mount(dash({ + syncResult: { + ok: true, + added: 3, + catalogPath: null, + catalogExists: false, + cacheSynced: true, + message: "ok", + warning: "some generic warning", + }, + })); + + const toast = host.querySelector(".action-toast"); + expect(toast).not.toBeNull(); + expect(toast!.className).toContain("notice-warn"); + // Does not vanish on the plain 6s hold. + await advanceTime(6000); + await advanceTime(8000); + expect(host.querySelector(".action-toast")).not.toBeNull(); +}); + test("warning-bearing results never auto-dismiss and offer an explicit dismiss", async () => { installFakeTimers(); @@ -179,8 +215,8 @@ test("warning-bearing results never auto-dismiss and offer an explicit dismiss", // The stale-app-server hint and warning stay in the message. expect(toast!.textContent).toContain("ocx sync --restart-codex"); // Still visible well past the plain success 6s / error 8s hold times. - await fireTimers(6000); - await fireTimers(8000); + await advanceTime(6000); + await advanceTime(8000); expect(host.querySelector(".action-toast")).not.toBeNull(); // The dismiss button closes it and calls clearSyncFeedback (the parent-level clear). @@ -235,7 +271,7 @@ test("a timer-dismissed result does not remount as a fresh toast after the panel expect(host.querySelector(".action-toast")).not.toBeNull(); // Auto-dismiss fires the timer, which clears the dashboard-level result. - await fireTimers(6000); + await advanceTime(6000); expect(host.querySelector(".action-toast")).toBeNull(); expect(state.result).toBeNull(); @@ -266,7 +302,7 @@ test("a new sync re-arms a dismissed toast", async () => { }); await mount(d); expect(host.querySelector(".action-toast")).not.toBeNull(); - await fireTimers(6000); + await advanceTime(6000); expect(host.querySelector(".action-toast")).toBeNull(); // A fresh sync click (re-arms dismissed=false) with a new result re-shows the @@ -286,6 +322,39 @@ test("a new sync re-arms a dismissed toast", async () => { }); await mount(d); expect(host.querySelector(".action-toast")).not.toBeNull(); - await fireTimers(6000); + await advanceTime(6000); + expect(host.querySelector(".action-toast")).toBeNull(); +}); + +test("a newer success toast re-arms: the first result's stale timer cannot dismiss it", async () => { + installFakeTimers(); + + const result = (added: number) => dash({ + syncResult: { + ok: true, + added, + catalogPath: null, + catalogExists: false, + cacheSynced: true, + message: "ok", + }, + }); + + await mount(result(3)); + expect(host.querySelector(".action-toast")).not.toBeNull(); + + // A second sync lands 1s in: the effect clears the first hold timer and starts a + // fresh 6s hold from now (deadline at t=7s, not the first result's t=6s). + await advanceTime(1000); + await mount(result(5)); + expect(host.querySelector(".action-toast")).not.toBeNull(); + + // t=6s: the stale first timer WOULD have fired here had clearTimeout not cancelled + // it. The newer toast must still be up. + await advanceTime(5000); + expect(host.querySelector(".action-toast")).not.toBeNull(); + + // t=7s: the fresh 6s hold expires and dismisses the newer toast. + await advanceTime(1000); expect(host.querySelector(".action-toast")).toBeNull(); });