fix: auto-dismiss toast notifications after timeout#847
fix: auto-dismiss toast notifications after timeout#847ranvier2d2 wants to merge 1 commit intopingdotgg:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3523db7fc
ℹ️ 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".
apps/web/src/components/ui/toast.tsx
Outdated
| > | ||
| <ThreadToastVisibleAutoDismiss | ||
| dismissAfterVisibleMs={toast.data?.dismissAfterVisibleMs} | ||
| dismissAfterVisibleMs={toast.data?.dismissAfterVisibleMs ?? DEFAULT_DISMISS_AFTER_VISIBLE_MS} |
There was a problem hiding this comment.
Skip default timeout for manually managed toasts
Using toast.data?.dismissAfterVisibleMs ?? DEFAULT_DISMISS_AFTER_VISIBLE_MS forces a 5s auto-dismiss even when a toast is intentionally long-lived via timeout: 0. In apps/web/src/components/GitActionsControl.tsx, runGitActionWithToast creates a loading toast and later updates that same toastId with success/error; when a git action runs longer than ~5s, this auto-dismiss closes the toast first and the later toastManager.update(...) no longer reliably surfaces the result. This regresses progress/result visibility for longer operations.
Useful? React with 👍 / 👎.
| type ToastId = ReturnType<typeof toastManager.add>; | ||
| const threadToastVisibleTimeoutRemainingMs = new Map<ToastId, number>(); | ||
|
|
||
| const DEFAULT_DISMISS_AFTER_VISIBLE_MS = 5_000; |
There was a problem hiding this comment.
🟡 Medium ui/toast.tsx:30
DEFAULT_DISMISS_AFTER_VISIBLE_MS is unconditionally applied to all toasts, including loading toasts. This causes loading toasts to auto-dismiss after 5 seconds even if the async operation is still running, leaving users without feedback and causing them to miss the eventual success/error state. Consider exempting loading toasts from the default timeout or requiring explicit durations only for non-loading types.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ui/toast.tsx around line 30:
`DEFAULT_DISMISS_AFTER_VISIBLE_MS` is unconditionally applied to all toasts, including `loading` toasts. This causes loading toasts to auto-dismiss after 5 seconds even if the async operation is still running, leaving users without feedback and causing them to miss the eventual success/error state. Consider exempting `loading` toasts from the default timeout or requiring explicit durations only for non-loading types.
Evidence trail:
- apps/web/src/components/ui/toast.tsx line 30: `DEFAULT_DISMISS_AFTER_VISIBLE_MS = 5_000`
- apps/web/src/components/ui/toast.tsx lines 267-270: `ThreadToastVisibleAutoDismiss` rendered for ALL toasts with fallback to default timeout
- apps/web/src/components/ui/toast.tsx lines 64-143: `ThreadToastVisibleAutoDismiss` effect manages timer independent of base-ui's `timeout` property
- apps/web/src/components/GitActionsControl.tsx lines 154-157: `threadToastData` only contains `{ threadId: activeThreadId }`, no `dismissAfterVisibleMs`
- apps/web/src/components/GitActionsControl.tsx lines 308-311, 316-319, 328-330: Loading toasts created with `timeout: 0` but no `dismissAfterVisibleMs` in data
Add a 5s default auto-dismiss for toasts. Loading toasts are excluded so they persist until their operation completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
a3523db to
88ed0ac
Compare
|
This is the intended behavior for “Action Failed” toasts, as stated by the maintainer in PR #672. To quote:
You may consider closing this PR. |
Summary
Changes
DEFAULT_DISMISS_AFTER_VISIBLE_MS = 5_000constant inapps/web/src/components/ui/toast.tsxdismissAfterVisibleMsis not explicitly set by the callerThreadToastVisibleAutoDismisscomponent already had visibility-aware pause/resume logic, but only 1 of ~15 call sites opted in — this makes 5s auto-dismiss the defaultTest plan
GitActionsControl.tsxstill uses its custom 10s timeoutNote
Auto-dismiss non-loading toast notifications after 5 seconds
Adds a
DEFAULT_DISMISS_AFTER_VISIBLE_MSconstant (5,000 ms) in toast.tsx and passes it as the defaultdismissAfterVisibleMstoThreadToastVisibleAutoDismissfor all non-loading toasts. Loading toasts continue to persist until explicitly dismissed.Macroscope summarized 88ed0ac.