feat(blade-svelte): Toast component#3365
Open
rohankokane-dev wants to merge 5 commits intomasterfrom
Open
Conversation
Made-with: Cursor
🦋 Changeset detectedLatest commit: d54fb41 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
✅ PR title follows Conventional Commits specification. |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit d54fb41:
|
Collaborator
🛡️ Coverage ReportSummaryFull Coverage Details |
rohankokane-dev
commented
May 5, 2026
Contributor
Author
rohankokane-dev
left a comment
rohankokane-dev
left a comment
There was a problem hiding this comment.
Code Review: Toast Component Migration
Solid migration with good architecture choices — clean separation into blade-core styles and blade-svelte components, idiomatic Svelte store pattern replacing react-hot-toast, and comprehensive stories. A few items to address below.
What's Done Well
- CVA usage consistent with the rest of the design system
- Thorough TypeScript types with good JSDoc
- CSS custom properties for runtime positioning keeps transforms in CSS (clean and performant)
- Comprehensive Storybook coverage (basic, variants, offset, z-index)
Issues & Suggestions
1. Bug risk: double-pause safety (toastStore.ts ~L113)
Casting 0 to a timeout ID in pauseToast() is fragile. If pauseToast() is called twice without resumeToast() in between, the second call hits clearTimeout(0) (no-op) and recomputes remaining from a stale startedAt (never updated on pause), producing an incorrect remaining duration.
Consider guarding against double-pause (check if pausedAt is already set) or deleting the timer entry after clearing and storing remaining separately.
2. SSR caveat: module-scoped store (toastStore.ts ~L30)
The toastStore is created at module scope, so in an SSR context (SvelteKit) it becomes a singleton shared across all concurrent requests — toast state from one user could leak to another. Probably fine for now since Blade is primarily client-side, but worth a doc comment noting the limitation.
3. Consider narrowing public API surface (index.ts)
updateToastHeight, pauseToast, and resumeToast are internal implementation details consumed only by ToastContainer. Exporting them from the public barrel invites consumers to call them directly and break toast stack invariants (e.g., calling pauseToast without a matching resumeToast). Consider keeping only useToast, Toast, ToastContainer, and types in the public exports.
4. Typo in story content (Toast.stories.svelte ~L64)
positive: 'Customer details failed successfully',"Failed successfully" is contradictory — should this be 'Customer details saved successfully'?
5. MAX_TOASTS = 1 naming is misleading (toast.ts ~L173)
This controls the number of "front" (non-peeking, full-scale) toasts, not the total allowed. MAX_FRONT_TOASTS would better communicate intent.
6. Fragile anti-tree-shake pattern (Toast.svelte ~L38, ToastContainer.svelte ~L33)
void getToastTemplateClasses();Works today but fragile — a future bundler update could optimize away void expressions with no side effects. Worth verifying this actually preserves CSS module imports in production builds.
7. bind:clientHeight callback pattern (ToastContainer.svelte ~L216)
bind:clientHeight={null, (h) => syncHeight(toast.id, h)}The null as first argument with a callback as second is an unusual pattern. Is this a stable Svelte 5 API? If undocumented/experimental, it could break in future Svelte releases.
8. IconButton TODO
Once IconButton is migrated to Svelte, the native <button> dismiss workaround in Toast.svelte should be swapped for consistency.
Checklist Reminder
The PR's own checklist has several unchecked items (Component Status Page, cross-browser testing, KitchenSink Story, Interaction Tests) — address or track as follow-ups before merge.
saurabhdaware
approved these changes
May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Migrates the React Toast component to Svelte 5.
Changes
packages/blade-svelte/src/components/Toast/(Toast + ToastContainer + useToast + toastStore)packages/blade-core/src/styles/Toast/packages/blade-svelte/src/components/index.tspackages/blade-core/src/styles/index.tsNotes
react-hot-toastwith a Svelte-native writable store +useToast()hook (no external runtime dependency added).--toast-offset,--toast-scale, etc.) on astyleattribute; actualtransform/opacitydeclarations stay inside the CSS module.IconButtonis substituted with a styled native<button>(IconButton not yet migrated).AnnouncementIconis substituted withInfoIconin the Toast Variants story.Additional Information
Artifacts (in worktree):
.cursor/artifacts/Toast/discovery-report.md.cursor/artifacts/Toast/migration-plan.md.cursor/artifacts/Toast/verification-report.md.cursor/artifacts/Toast/screenshots/See the verification report for the full validation summary.
Component Checklist
Made with Cursor