feat[toasts]: instead of queueing, remove oldest toasts#2899
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Toast component now enforces a maximum of 3 visible non-persistent toasts by maintaining an Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
js/app/packages/core/component/Toast/Toast.tsx (1)
502-530:⚠️ Potential issue | 🟠 MajorCap enforcement is bypassed for other non-persistent
toast-regionAPIs.Line 502–Line 530 applies eviction/tracking only inside
createToast, butembed(Line 560) andcustom(Line 588) can still create non-persistent toasts intoast-regionwithout this path. That means queueing can still happen, so the “remove oldest instead of queueing” behavior is not consistently enforced.Proposed consolidation (single tracked show path)
+function showToastInRegion( + render: Parameters<typeof toaster.show>[0], + options?: { region?: string; persistent?: boolean; onDismiss?: (id: number) => void } +) { + const region = options?.region ?? 'toast-region'; + const shouldTrack = region === 'toast-region' && !options?.persistent; + + if (shouldTrack && activeToastIds.length >= MAX_VISIBLE_TOASTS) { + const oldestId = activeToastIds.shift(); + if (oldestId !== undefined) toaster.dismiss(oldestId); + } + + const toastId = toaster.show(render, { region }); + if (shouldTrack) activeToastIds.push(toastId); + return toastId; +}Then route
createToast,embed, andcustomthrough this helper (withpersistent/regionpassed through) so cap behavior is consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/core/component/Toast/Toast.tsx` around lines 502 - 530, The eviction/tracking logic (activeToastIds, MAX_VISIBLE_TOASTS, toaster.show) is only used in createToast so embed and custom can still create non-persistent toasts in 'toast-region' and bypass the cap; refactor by extracting a single helper (e.g., showTrackedToast or showToastWithCap) that accepts the toast payload plus region and persistent flags, performs the oldest-id eviction when region === 'toast-region' and persistent is false, calls toaster.show with the provided renderer/opts, pushes the returned toastId into activeToastIds, and wires the onDismiss removal; then replace direct calls in createToast, embed, and custom to use this helper so cap behavior is consistently enforced across all APIs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@js/app/packages/core/component/Toast/Toast.tsx`:
- Around line 502-530: The eviction/tracking logic (activeToastIds,
MAX_VISIBLE_TOASTS, toaster.show) is only used in createToast so embed and
custom can still create non-persistent toasts in 'toast-region' and bypass the
cap; refactor by extracting a single helper (e.g., showTrackedToast or
showToastWithCap) that accepts the toast payload plus region and persistent
flags, performs the oldest-id eviction when region === 'toast-region' and
persistent is false, calls toaster.show with the provided renderer/opts, pushes
the returned toastId into activeToastIds, and wires the onDismiss removal; then
replace direct calls in createToast, embed, and custom to use this helper so cap
behavior is consistently enforced across all APIs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a656fadd-5a3f-45b6-a1e5-f4d579ce703e
📒 Files selected for processing (1)
js/app/packages/core/component/Toast/Toast.tsx
No description provided.