feat(dashboard): unify mutation feedback into global flash toasts#582
Conversation
Replace the per-page notice/error banner pattern (hand-rolled state fields, inline alert markup, and page-specific gating across 19 files) with one flash store and a FlashMessages toast stack mounted in App: - flash.success()/flash.error() queue toasts top-right with timed auto-dismiss (5s/8s), manual dismiss, and identical-toast dedup - mutation feedback (saves, deletes, resets, toggles, reconnects) now flashes globally; success messages no longer vanish on next action - persistent state stays inline: load failures, in-form validation and submit errors, feature-unavailable warnings, confirm-dialog errors, and the one-time issued-API-key banner - consistent aria (role=status/alert + aria-live) in one place
|
Warning Review limit reached
Next review available in: 16 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe dashboard adds a global flash-message store and toast component, then migrates page-level mutation feedback from local notices and errors to shared success/error notifications across keys, budgets, guardrails, servers, models, settings, and workflows. ChangesGlobal flash notification migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DashboardPage
participant flash
participant App
participant FlashMessages
DashboardPage->>flash: emit mutation result
flash->>App: update reactive toast state
App->>FlashMessages: render toast stack
FlashMessages->>flash: dismiss toast
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Confidence Score: 3/5The PR needs fixes before merging because successful mutations can produce contradictory failure feedback and successful settings saves can leave obsolete error alerts visible. Success is announced before fallible refetches complete, and two settings save paths retain persistent load errors, causing the dashboard to communicate incorrect operation outcomes. web/dashboard/src/pages/budgets/budgets.svelte.js, web/dashboard/src/pages/settings/BudgetSettings.svelte, web/dashboard/src/pages/settings/TaggingSettings.svelte
What T-Rex did
Sequence DiagramsequenceDiagram
participant U as Operator
participant P as Dashboard page
participant A as Admin API
participant F as Flash store
U->>P: Submit mutation
P->>A: Mutation request
A-->>P: Success
P->>F: Queue success toast
P->>A: Refetch page data
A-->>P: Updated data or load failure
F-->>U: Render global toast
Reviews (1): Last reviewed commit: "feat(dashboard): unify mutation feedback..." | Re-trigger Greptile |
| flash.success("Budget saved."); | ||
| await this.fetchBudgets(); |
There was a problem hiding this comment.
Refetch failures misreport mutations
When a mutation succeeds but the subsequent list refetch fails, this code has already announced success and the shared catch then reports that the mutation failed. Operators consequently receive contradictory feedback even though the server-side change succeeded; separate the mutation result from refresh errors or defer success feedback until the refresh outcome is known.
Artifacts
Repro: focused executable Svelte-store harness with transport and feedback assertions
- Evidence file captured while the check ran.
Repro: successful PUT, failed follow-up GET, and contradictory feedback sequence
- The full command output behind this check.
- What the screen looked like at this point in the check.
After: Budget Stale Error Save
- What the screen looked like at this point in the check.
▶ budget-stale-error-flow.webm
- The flow T-Rex ran, recorded end to end.
| return; | ||
| } | ||
| saving = true; |
There was a problem hiding this comment.
Successful saves retain load errors
When the initial settings load fails and a later save succeeds, the save path no longer clears error, so the success toast appears while the obsolete load-failure alert remains rendered indefinitely. Clear the retained load error when beginning or completing a successful save; the same stale-state behavior exists in TaggingSettings.svelte.
Artifacts
Repro: Playwright harness controlling the real dashboard API responses and exercising the save flow.
- Evidence file captured while the check ran.
- The full command output behind this check.
- Evidence file captured while the check ran.
- What the screen looked like at this point in the check.
After: Budget Stale Error Save
- What the screen looked like at this point in the check.
- The flow T-Rex ran, recorded end to end.
| return; | ||
| } | ||
| saving = true; |
There was a problem hiding this comment.
Successful saves retain load errors
When the initial settings load fails and a later save succeeds, the save path no longer clears error, so the success toast appears while the obsolete load-failure alert remains rendered indefinitely. Clear the retained load error when beginning or completing a successful save; the same stale-state behavior exists in TaggingSettings.svelte.
Artifacts
Repro: Playwright harness controlling the real dashboard API responses and exercising the save flow.
- Evidence file captured while the check ran.
- The full command output behind this check.
- Evidence file captured while the check ran.
- What the screen looked like at this point in the check.
After: Budget Stale Error Save
- What the screen looked like at this point in the check.
- The flow T-Rex ran, recorded end to end.
Slide up from the bottom edge with a slight overshoot (backOut, 360ms) instead of the subtle 160ms side-slide, and add a fading glow ring in the toast's accent color on arrival. Glow is disabled under prefers-reduced-motion.
Anchor the toast stack over the main section (offset past the sidebar, tracking its collapsed/mobile 60px width) and drop toasts in from above. Toasts now size to their text up to 480px.
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 `@web/dashboard/src/lib/components/organisms/FlashMessages.svelte`:
- Line 18: Update the toast transitions in FlashMessages, including the fly
entry and fade exit, to honor prefers-reduced-motion by using the no-transition
parameter shape or zero duration when reduced motion is enabled, while
preserving the existing animations otherwise.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3197946d-1530-426e-843e-767ae3bf0c4a
⛔ Files ignored due to path filters (3)
internal/admin/dashboard/static/dist/assets/index-BOce0cPe.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-cOpxxzyI.cssis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (1)
web/dashboard/src/lib/components/organisms/FlashMessages.svelte
| class:flash-toast-error={toast.kind === "error"} | ||
| role={toast.kind === "error" ? "alert" : "status"} | ||
| aria-live={toast.kind === "error" ? "assertive" : "polite"} | ||
| in:fly={{ y: -24, duration: 360, easing: backOut }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="web/dashboard/src/lib/components/organisms/FlashMessages.svelte"
if [ -f "$file" ]; then
echo "== file exists =="
wc -l "$file"
echo "== relevant lines =="
sed -n '1,160p' "$file" | nl -ba
else
echo "missing $file"
echo "== flash messages candidates =="
fd -i 'FlashMessages' .
fi
echo "== reduced-motion references =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'prefers-reduced-motion|reduced motion|transition:|fade|fly|glow' web/dashboard/src || trueRepository: ENTERPILOT/GoModel
Length of output: 303
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="web/dashboard/src/lib/components/organisms/FlashMessages.svelte"
echo "== relevant file content =="
if command -v nl >/dev/null 2>&1; then
nl -ba "$file" | sed -n '1,160p'
else
awk '{printf "%6d %s\n", NR, $0}' "$file" | sed -n '1,160p'
fi
echo "== reduced-motion / transition references ==="
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'prefers-reduced-motion|reduced motion|transition:|fade|fly|glow' web/dashboard/src || true
else
grep -RIn --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build 'prefers-reduced-motion\|reduced motion\|transition:\|fade\|fly\|glow' web/dashboard/src || true
fiRepository: ENTERPILOT/GoModel
Length of output: 10118
Honor reduced-motion for the Svelte transitions.
The prefers-reduced-motion: reduce rule only disables the glow animation; the fly entry and fade exit still run for users who prefer reduced motion. Use no-transition parameter shape or zero-duration transitions under reduced-motion conditions for these toasts.
🤖 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 `@web/dashboard/src/lib/components/organisms/FlashMessages.svelte` at line 18,
Update the toast transitions in FlashMessages, including the fly entry and fade
exit, to honor prefers-reduced-motion by using the no-transition parameter shape
or zero duration when reduced motion is enabled, while preserving the existing
animations otherwise.
- Post-success list refreshes are now fire-and-forget (void) instead of awaited inside the mutation's try block, so a refresh failure can never be misreported as a failed mutation; it surfaces only as the list's own load state. (The one-shot loads in the conversation drawer and the types fetch inside fetchPage stay awaited.) - BudgetSettings/TaggingSettings clear a retained load error on a successful save, so an obsolete load-failure alert no longer sticks around next to a success toast.
|
Addressed both review findings in b1431c8:
|
What
Replaces the dashboard's per-page notice/error banner pattern (hand-rolled $state fields, inline alert markup, and page-specific gating repeated across 19 files) with a single flash-message mechanism:
flashstore (src/lib/stores/flash.svelte.js):flash.success(msg)/flash.error(msg)queue toasts with timed auto-dismiss (5s success / 8s error), manual dismiss, and dedup (an identical toast still on screen restarts instead of stacking).FlashMessagesorganism mounted once inApp.svelte: fixed bottom-right stack, rendered above modal shells so feedback fired from a dialog stays visible, with consistentrole="status"/role="alert"+aria-livein one place.noticefield is deleted.User-visible impact
Testing
make test-dashboard: 316/316.Summary by CodeRabbit