feat(gui): fixed action toasts for dashboard sync + model-apply feedback - #1050
Conversation
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).
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe GUI replaces inline Models and dashboard synchronization notices with timed fixed-position toasts. It adds success and error icons, animated synchronization feedback, shared toast styling, updated quota colors, muted-text token mapping, dark-theme source-mark filters, localization, and React tests. ChangesGUI feedback toast updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardOverviewSections
participant useDashboardData
participant ActionToast
User->>DashboardOverviewSections: Start synchronization
DashboardOverviewSections->>useDashboardData: Receive synchronization result or error
DashboardOverviewSections->>ActionToast: Render feedback toast
ActionToast-->>User: Display live feedback
User->>DashboardOverviewSections: Dismiss toast
DashboardOverviewSections->>useDashboardData: Clear synchronization feedback
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/src/pages/dashboard-overview-sections.tsx`:
- Line 220: Move the hardcoded version transition text in the update overview
rendering into the i18n system: add the corresponding
dash.updateVersionTransition key to locale files with currentVersion and
latestVersion parameters, then use t(...) at the updateJob.latestVersion
conditional while preserving the existing conditional display behavior.
In `@gui/src/pages/Models.tsx`:
- Around line 84-93: The feedback timer in the status effect must re-arm for
every custom-model action, including repeated identical successes and validation
errors. Update addCustomModel, updateCustomModel, and deleteCustomModel to clear
status before each feedback-producing validation or mutation result, ensuring
the existing useEffect observes a status change and preserves the current
dismissal durations.
In `@gui/tests/models-status-toast.test.tsx`:
- Around line 9-12: Add "fetch" to the globals list used by the test cleanup so
afterEach restores globalThis.fetch after the Models API mock replaces it. Keep
the existing global restoration behavior unchanged.
- Around line 85-116: Extend gui/tests/models-status-toast.test.tsx lines 85-116
to use controlled timers, verify the success toast remains visible before 6
seconds and is dismissed at 6 seconds, then perform another model action and
confirm the toast reappears; extend gui/tests/dashboard-sync-feedback.test.tsx
lines 75-108 to use controlled timers, verify success dismissal at 6 seconds and
error dismissal at 8 seconds, and confirm a new sync re-arms the dismissed
toast. Anchor the changes to the existing model-action and dashboard-sync
feedback tests without changing production behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3083fd83-068b-43d5-8493-04d5f453e5df
📒 Files selected for processing (7)
gui/src/pages/Models.tsxgui/src/pages/dashboard-overview-sections.tsxgui/src/styles.cssgui/src/styles/provider-overview-dashboard.cssgui/src/styles/provider-quota.cssgui/tests/dashboard-sync-feedback.test.tsxgui/tests/models-status-toast.test.tsx
| 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( | ||
| <LanguageProvider> | ||
| <Models apiBase="http://localhost" /> | ||
| </LanguageProvider>, | ||
| ); | ||
| }); | ||
| 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<HTMLButtonElement>("button")].find(b => b.textContent === "All off")!; | ||
| off.click(); | ||
| await new Promise(resolve => testWindow.setTimeout(resolve, 0)); | ||
| await Promise.resolve(); | ||
| }); | ||
|
|
||
| const toast = container.querySelector<HTMLElement>(".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<HTMLElement>(".models-workspace-root"); | ||
| expect(workspace?.previousElementSibling?.classList.contains("action-toast")).toBe(true); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Test toast dismissal and re-arm behavior.
Both tests verify only the initial toast DOM. They do not verify the required 6-second success dismissal, 8-second error dismissal, or a new action after dismissal. A timer cleanup or re-arm regression can therefore pass this suite.
gui/tests/models-status-toast.test.tsx#L85-L116: use controlled timers to verify a successful model-action toast expires after 6 seconds and reappears for a subsequent action.gui/tests/dashboard-sync-feedback.test.tsx#L75-L108: use controlled timers to verify success and error hold times and that a new sync re-arms a dismissed toast.
📍 Affects 2 files
gui/tests/models-status-toast.test.tsx#L85-L116(this comment)gui/tests/dashboard-sync-feedback.test.tsx#L75-L108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/tests/models-status-toast.test.tsx` around lines 85 - 116, Extend
gui/tests/models-status-toast.test.tsx lines 85-116 to use controlled timers,
verify the success toast remains visible before 6 seconds and is dismissed at 6
seconds, then perform another model action and confirm the toast reappears;
extend gui/tests/dashboard-sync-feedback.test.tsx lines 75-108 to use controlled
timers, verify success dismissal at 6 seconds and error dismissal at 8 seconds,
and confirm a new sync re-arms the dismissed toast. Anchor the changes to the
existing model-action and dashboard-sync feedback tests without changing
production behavior.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d887cd3b02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| contextCaps: {}, | ||
| contextCapValue: 350_000, | ||
| })); | ||
| globalThis.fetch = (async (input, init) => { |
There was a problem hiding this comment.
This test replaces globalThis.fetch, but fetch is not in the saved/restored globals list, so after this file runs the shared Bun test process keeps the 404-only mock. Later test files can then capture or use this stub instead of the real fetch, making the required cd gui; bun test tests suite order-dependent; include fetch in the restoration set or restore it explicitly in afterEach.
AGENTS.md reference: gui/AGENTS.md:L42-L50
Useful? React with 👍 / 👎.
| // 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); |
There was a problem hiding this comment.
Because syncResult and syncError live in useDashboardData above the dashboard tabs, this component-local dismissed state resets whenever the Overview panel unmounts. If a user runs Sync, lets the toast auto-dismiss, switches to another dashboard tab, and returns to Overview, the old result still satisfies the render condition and the stale toast appears again without a new sync; persist the dismissed/result token in dashboard data or clear the sync result when the timer fires.
Useful? React with 👍 / 👎.
| syncToastTimerRef.current = null; | ||
| } | ||
| if (syncResult || syncError) { | ||
| const holdMs = syncError ? 8000 : 6000; |
There was a problem hiding this comment.
When /api/sync succeeds but returns nativeSubagentDefaultsWarning or staleAppServerHint, the toast contains the only visible warning/restart instruction, yet this timer treats it like a plain success and hides it after 6 seconds. Users who miss that brief toast have no persistent way on the dashboard to recover the warning or the ocx sync --restart-codex hint; keep warning-bearing results visible until the next sync or provide a dismiss control.
Useful? React with 👍 / 👎.
- 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.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/src/pages/dashboard-overview-sections.tsx`:
- Around line 151-154: Update syncHoldsWarning in the dashboard overview section
to also treat syncResult.warning as a persistent warning condition, preserving
visibility until the next sync or explicit dismissal. Add a test covering a sync
result with warning set while nativeSubagentDefaultsWarning and
staleAppServerHint are absent, and verify it does not auto-dismiss.
In `@gui/src/styles.css`:
- Around line 715-727: Update the .action-toast-dismiss rule to add min-width
and min-height of 24px, preserving the existing icon size and other styling.
In `@gui/tests/dashboard-sync-feedback.test.tsx`:
- Around line 78-94: Update the fake-timer helpers installFakeTimers and
fireTimers to track timer IDs, remove canceled callbacks in clearTimeout, and
maintain a monotonic virtual clock so callbacks fire when their deadlines elapse
rather than matching raw durations. Add coverage for starting a second success
toast before the first six-second hold expires, verifying the stale timer cannot
dismiss the newer toast.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 34d834cc-5df1-470c-92dc-23872625e558
📒 Files selected for processing (4)
gui/src/pages/dashboard-overview-sections.tsxgui/src/pages/use-dashboard-data.tsgui/src/styles.cssgui/tests/dashboard-sync-feedback.test.tsx
| .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; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
fd -i 'DashboardMaintenancePanel|styles\.css' gui || true
printf '%s\n' '--- CSS context ---'
cat -n gui/src/styles.css | sed -n '700,740p'
printf '%s\n' '--- relevant component references ---'
rg -n -C 8 'action-toast-dismiss|IconX|DashboardMaintenancePanel' guiRepository: lidge-jun/opencodex
Length of output: 48698
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- CSS context ---'
cat -n gui/src/styles.css | sed -n '700,740p'
printf '%s\n' '--- relevant component references ---'
rg -n -C 8 'action-toast-dismiss|IconX|DashboardMaintenancePanel' guiRepository: lidge-jun/opencodex
Length of output: 48655
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- icon defaults and button sizing ---'
cat -n gui/src/icons.tsx | sed -n '1,28p'
rg -n -C 6 '(^|[,{ ])button|\.btn-icon|\.btn-sm|\.action-toast|box-sizing|svg[[:space:]]*\{' gui/src/styles.css
printf '%s\n' '--- toast layout and dismissal behavior ---'
cat -n gui/src/pages/dashboard-overview-sections.tsx | sed -n '145,265p'Repository: lidge-jun/opencodex
Length of output: 39780
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
css = Path("gui/src/styles.css").read_text()
tsx = Path("gui/src/pages/dashboard-overview-sections.tsx").read_text()
rule = re.search(r"\.action-toast-dismiss\s*\{(?P<body>[^}]*)\}", css, re.S)
assert rule, "dismiss rule not found"
body = rule.group("body")
padding = re.search(r"\bpadding\s*:\s*(\d+)px\s*;", body)
assert padding, "single-value padding not found"
padding_px = int(padding.group(1))
icons = re.findall(
r'className="action-toast-dismiss"[\s\S]{0,300}?'
r'<IconX\s+width=\{(\d+)\}\s+height=\{(\d+)\}',
tsx,
)
assert len(icons) == 2, icons
assert all((w, h) == ("13", "13") for w, h in icons), icons
intrinsic = 13 + 2 * padding_px
print({
"dismiss_buttons": len(icons),
"icon_sizes": icons,
"padding_px": padding_px,
"intrinsic_hit_area_px": (intrinsic, intrinsic),
"has_min_width": bool(re.search(r"\bmin-width\s*:", body)),
"has_min_height": bool(re.search(r"\bmin-height\s*:", body)),
})
assert intrinsic == 21
assert not re.search(r"\bmin-width\s*:", body)
assert not re.search(r"\bmin-height\s*:", body)
PYRepository: lidge-jun/opencodex
Length of output: 325
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
css = Path("gui/src/styles.css").read_text()
tsx = Path("gui/src/pages/dashboard-overview-sections.tsx").read_text()
rule = re.search(r"\.action-toast-dismiss\s*\{(?P<body>[^}]*)\}", css, re.S)
assert rule
body = rule.group("body")
padding = int(re.search(r"\bpadding\s*:\s*(\d+)px\s*;", body).group(1))
icons = re.findall(
r'className="action-toast-dismiss"[\s\S]{0,300}?'
r'<IconX\s+width=\{(\d+)\}\s+height=\{(\d+)\}',
tsx,
)
assert len(icons) == 2
assert all((w, h) == ("13", "13") for w, h in icons)
intrinsic = 13 + 2 * padding
print({
"dismiss_buttons": len(icons),
"icon_sizes": icons,
"intrinsic_hit_area_px": (intrinsic, intrinsic),
"has_min_width": bool(re.search(r"\bmin-width\s*:", body)),
"has_min_height": bool(re.search(r"\bmin-height\s*:", body)),
})
assert intrinsic == 21
assert not re.search(r"\bmin-width\s*:", body)
assert not re.search(r"\bmin-height\s*:", body)
PYRepository: lidge-jun/opencodex
Length of output: 308
Increase the dismissal button hit area.
At gui/src/styles.css:715-727, the 13px icon and 4px padding produce a 21px square hit area. Add min-width: 24px and min-height: 24px without changing the icon size.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/styles.css` around lines 715 - 727, Update the .action-toast-dismiss
rule to add min-width and min-height of 24px, preserving the existing icon size
and other styling.
- 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.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/tests/models-status-toast.test.tsx`:
- Around line 86-103: Update installFakeTimers and fireTimers to model
cancellable timers with elapsed-time advancement so cleared callbacks do not
execute. In the success-path test around the repeated action, trigger the
identical action before the initial 6-second deadline, advance beyond that
original deadline, and assert the toast remains visible before advancing to the
re-armed deadline and asserting dismissal. Add the equivalent re-arming
assertion for the 8-second error path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a8cdbff7-b27a-4b9e-964c-68caacf1949d
📒 Files selected for processing (8)
gui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/pages/Models.tsxgui/tests/models-status-toast.test.tsx
| // 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(); }); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test timer re-arming before the original timeout expires.
Lines 163-176 dismiss the first toast before the repeated action. This passes even if feedbackGen is removed because status changes from "" to "Applied".
clearTimeout is a no-op, and fireTimers(6000) invokes every matching callback. The helper cannot verify that React cleanup cancels the first 6-second timer.
Use cancellation-aware timers with elapsed-time advancement. Trigger the second identical action before 6 seconds, advance past the original deadline, and assert that the toast remains visible. Then advance to the new deadline and assert dismissal. Add the equivalent 8-second error-path assertion.
Also applies to: 163-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/tests/models-status-toast.test.tsx` around lines 86 - 103, Update
installFakeTimers and fireTimers to model cancellable timers with elapsed-time
advancement so cleared callbacks do not execute. In the success-path test around
the repeated action, trigger the identical action before the initial 6-second
deadline, advance beyond that original deadline, and assert the toast remains
visible before advancing to the re-armed deadline and asserting dismissal. Add
the equivalent re-arming assertion for the 8-second error path.
- 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.
1 participant
What
Two transient-action feedback surfaces in the GUI now render as fixed toasts instead of inline notices:
DashboardMaintenancePanel): the sync result/error used to render as an inline notice that pushed every panel below the card down by its full height on each click. It is now a fixed toast (bottom-right,position: fixed, out of the layout flow) with ok/err/warn tones, an icon,role="status", and auto-dismiss (6s ok / 8s err). A new click re-arms the toast even after auto-dismiss.Models.tsx): the old inlineNoticeunder the page subtitle is replaced by the same fixed toast; appearing or clearing never shifts the model grid below.Supporting CSS:
.action-toast+ entry animation +.spin-iconfor the in-flight sync button.Palette / a11y cleanup
.bar-warn,.bar-amber): thelinear-gradient(90deg, var(--green), var(--amber))reads as a cyan/mint tell on dark (the green stop is#4ecb9d). Both are now flatvar(--amber)to match the flat-surface grammar..pws-dashboard … --fg-muted): the#888fallback only reached ~3.5:1 on white; the variable is now aliased to the design--mutedtoken (#6e6e6elight /#a6a6a6dark) → 5.1:1 light, 6.6:1 dark.Tests
gui/tests/dashboard-sync-feedback.test.tsx— sync ok/err render as.action-toast(not.maintenance-notice).gui/tests/models-status-toast.test.tsx— apply feedback renders as the fixed toast and no inline notice sits before the workspace.Both pass on current
dev(3/3, happy-dom).bun run lintclean.Summary by CodeRabbit
New Features
Style
Tests