Repository navigation
feat: Sonner toast integration - #415
Merged
Merged
Conversation
Co-authored-by: Ilya Boyandin <[email protected]>
|
Cursor Agent can help with this pull request. Just |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the legacy in-memory Radix toast system (useToast, Toast components, LegacyToaster) with a Sonner-backed Toaster; removes legacy toast modules and dependency, adds a Sonner Toaster component, updates package exports, and migrates components to import and call the new Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant ToastAPI as "toast (export)"
participant Toaster as "Sonner Toaster"
participant Browser as "UI Display"
Component->>ToastAPI: import { toast }\ntoast.success("Message", { description })
ToastAPI->>Toaster: enqueue success toast (payload)
Toaster->>Browser: render toast (icon, text, theme styles)
Browser-->>Toaster: user dismiss / timeout
Toaster->>ToastAPI: optional callback (dismissed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
ilyabo
marked this pull request as ready for review
March 3, 2026 15:18
Signed-off-by: Ilya Boyandin <[email protected]>
Signed-off-by: Ilya Boyandin <[email protected]>
Signed-off-by: Ilya Boyandin <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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)
packages/ai-settings/src/components/AiModelParameters.tsx (1)
66-83:⚠️ Potential issue | 🟡 MinorReset file input on validation failures as well.
Line 70 and Line 79 return before the input is reset, which can block re-selecting the same invalid file.
🛠️ Proposed fix
const handleFileUpload = async ( event: React.ChangeEvent<HTMLInputElement>, ) => { const file = event.target.files?.[0]; if (!file) return; + const resetFileInput = () => { + if (fileInputRef.current) { + fileInputRef.current.value = ''; + } + }; @@ if ( !allowedTypes.includes(file.type) && !allowedExtensions.includes(fileExtension) ) { toast.error('Invalid file type', { description: 'Please select a text file (.txt, .md, .json)', }); + resetFileInput(); return; } @@ if (file.size > maxSize) { toast.error('File too large', { description: 'File size must be less than 1MB', }); + resetFileInput(); return; } @@ - if (fileInputRef.current) { - fileInputRef.current.value = ''; - } + resetFileInput(); };Also applies to: 99-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-settings/src/components/AiModelParameters.tsx` around lines 66 - 83, Validation early-returns in the AiModelParameters component leave the file input value unchanged, preventing re-selecting the same invalid file; update the validation branches in the file selection handler (the block that checks allowedTypes/allowedExtensions and the maxSize check, and the later validation around lines ~99-102) to reset the file input before returning by clearing the input element (e.g., via the same input ref used elsewhere in this component) so the input’s value is set to empty string prior to each toast.error return.
🧹 Nitpick comments (1)
packages/ui/src/components/sonner.tsx (1)
28-35: Consolidate Sonner style variables in one place.
--normal-*vars are defined inline here while Sonner vars are also defined inpackages/ui/tailwind-preset.css. Keeping one source of truth will reduce styling drift.♻️ Suggested refactor
- style={ - { - '--normal-bg': 'var(--popover)', - '--normal-text': 'var(--popover-foreground)', - '--normal-border': 'var(--border)', - '--border-radius': 'var(--radius)', - } as React.CSSProperties - } + // Keep Sonner color variables in tailwind-preset.css to avoid split styling sources.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/sonner.tsx` around lines 28 - 35, The inline CSS custom properties (--normal-bg, --normal-text, --normal-border, --border-radius) set in the style prop of the Sonner component in packages/ui/src/components/sonner.tsx should be removed and consolidated to use the existing Sonner/Tailwind variables defined in packages/ui/tailwind-preset.css; update the Sonner component (where the style prop is applied) to rely on the preset CSS variables (or add any missing variables to tailwind-preset.css) instead of inline styles so there is a single source of truth for these tokens and no duplicated definitions.
🤖 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 `@packages/ai-settings/src/components/AiModelParameters.tsx`:
- Around line 66-83: Validation early-returns in the AiModelParameters component
leave the file input value unchanged, preventing re-selecting the same invalid
file; update the validation branches in the file selection handler (the block
that checks allowedTypes/allowedExtensions and the maxSize check, and the later
validation around lines ~99-102) to reset the file input before returning by
clearing the input element (e.g., via the same input ref used elsewhere in this
component) so the input’s value is set to empty string prior to each toast.error
return.
---
Nitpick comments:
In `@packages/ui/src/components/sonner.tsx`:
- Around line 28-35: The inline CSS custom properties (--normal-bg,
--normal-text, --normal-border, --border-radius) set in the style prop of the
Sonner component in packages/ui/src/components/sonner.tsx should be removed and
consolidated to use the existing Sonner/Tailwind variables defined in
packages/ui/tailwind-preset.css; update the Sonner component (where the style
prop is applied) to rely on the preset CSS variables (or add any missing
variables to tailwind-preset.css) instead of inline styles so there is a single
source of truth for these tokens and no duplicated definitions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35d36b76-aa54-4de5-a986-c9201a123e40
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
apps/sqlrooms-cli-ui/src/components/DataSourcesPanel.tsxexamples/notebook/src/DataSourcesPanel.tsxpackages/ai-settings/src/components/AiModelParameters.tsxpackages/ai-settings/src/components/AiModelsSettings.tsxpackages/ai-settings/src/components/AiProvidersSettings.tsxpackages/canvas/src/nodes/AddNodePopover.tsxpackages/ui/package.jsonpackages/ui/src/components/sonner.tsxpackages/ui/src/components/toast.tsxpackages/ui/src/components/toaster.tsxpackages/ui/src/hooks/use-toast.tspackages/ui/src/index.tspackages/ui/tailwind-preset.css
💤 Files with no reviewable changes (3)
- packages/ui/src/components/toast.tsx
- packages/ui/src/hooks/use-toast.ts
- packages/ui/src/components/toaster.tsx
Signed-off-by: Ilya Boyandin <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the codebase from the legacy Radix-based toast + useToast hook to a Sonner-based toast API, updates consumers to use toast.*, and removes the Radix toast dependency from the UI package.
Changes:
- Remove legacy toast implementation (
useToast, Radix toast components, legacy toaster wrapper). - Add a new Sonner-backed
Toastercomponent integrated with the theme provider and update UI exports. - Update call sites across apps/packages/examples to use
toast.success/error(...)instead ofuseToast().
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Removes @radix-ui/react-toast from lockfile. |
| packages/ui/package.json | Drops @radix-ui/react-toast dependency. |
| packages/ui/tailwind-preset.css | Minor formatting change. |
| packages/ui/src/index.ts | Removes legacy toast exports, re-exports Sonner toast, and exports new Toaster. |
| packages/ui/src/hooks/use-toast.ts | Deletes legacy hook-based toast state/reducer. |
| packages/ui/src/components/toaster.tsx | Deletes legacy wrapper toaster (Radix + Sonner). |
| packages/ui/src/components/toast.tsx | Deletes Radix toast component implementation. |
| packages/ui/src/components/sonner.tsx | Adds new themed Sonner Toaster component with icons and CSS variable styling. |
| packages/canvas/src/nodes/AddNodePopover.tsx | Migrates from useToast() to toast.error(...). |
| packages/ai-settings/src/components/AiProvidersSettings.tsx | Migrates to toast.success/error(...). |
| packages/ai-settings/src/components/AiModelsSettings.tsx | Migrates to toast.success/error(...). |
| packages/ai-settings/src/components/AiModelParameters.tsx | Migrates to toast.success/error(...). |
| examples/notebook/src/DataSourcesPanel.tsx | Migrates to toast.success/error(...). |
| apps/sqlrooms-cli-ui/src/components/DataSourcesPanel.tsx | Migrates to toast.success/error(...). |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
306
to
308
| // Hooks | ||
| export {useToast, toast as legacyToast, reducer} from './hooks/use-toast'; | ||
| export {toast, type ExternalToast} from 'sonner'; | ||
|
|
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/sonner.tsx`:
- Line 14: The Toaster component currently spreads {...props} and allows
caller-provided icons or toastOptions to completely replace defaults; fix this
by merging defaults with incoming props before spreading — inside the Toaster
component compute mergedProps that does icons: {...defaultIcons,
...(props.icons||{})} and toastOptions: {...defaultToastOptions,
...(props.toastOptions||{}), classNames: {...defaultToastOptions.classNames,
...(props.toastOptions?.classNames||{})}} (so classNames.toast and variant icons
are preserved) and then pass mergedProps to the underlying Toaster renderer;
reference the Toaster component, props, icons, toastOptions, classNames.toast
and variant icon keys when applying the merge.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff6e77e1-b6b4-4275-a293-5bbf269cbdeb
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/ui/package.jsonpackages/ui/src/components/sonner.tsx
💤 Files with no reviewable changes (1)
- packages/ui/package.json
| import {useTheme} from '../theme/theme-provider'; | ||
| import type {CSSProperties} from 'react'; | ||
|
|
||
| const Toaster = ({...props}: ToasterProps) => { |
Contributor
There was a problem hiding this comment.
Merge default icons/toastOptions with incoming props to avoid losing wrapper defaults.
Because ...props is applied at Line 42, any caller-provided icons or toastOptions fully replaces the defaults. That can silently drop classNames.toast = 'cn-toast' and variant icons.
🔧 Proposed fix
-const Toaster = ({...props}: ToasterProps) => {
+const Toaster = ({toastOptions, icons, ...props}: ToasterProps) => {
const {theme = 'system'} = useTheme();
return (
<Sonner
richColors
theme={theme as ToasterProps['theme']}
className="toaster group"
icons={{
success: <CircleCheckIcon className="size-4" />,
info: <InfoIcon className="size-4" />,
warning: <TriangleAlertIcon className="size-4" />,
error: <OctagonXIcon className="size-4" />,
loading: <Loader2Icon className="size-4 animate-spin" />,
+ ...icons,
}}
style={
{
'--normal-bg': 'var(--popover)',
'--normal-text': 'var(--popover-foreground)',
'--normal-border': 'var(--border)',
'--border-radius': 'var(--radius)',
} as CSSProperties
}
toastOptions={{
+ ...toastOptions,
classNames: {
toast: 'cn-toast',
+ ...toastOptions?.classNames,
},
}}
{...props}
/>
);
};Also applies to: 22-28, 37-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/sonner.tsx` at line 14, The Toaster component
currently spreads {...props} and allows caller-provided icons or toastOptions to
completely replace defaults; fix this by merging defaults with incoming props
before spreading — inside the Toaster component compute mergedProps that does
icons: {...defaultIcons, ...(props.icons||{})} and toastOptions:
{...defaultToastOptions, ...(props.toastOptions||{}), classNames:
{...defaultToastOptions.classNames, ...(props.toastOptions?.classNames||{})}}
(so classNames.toast and variant icons are preserved) and then pass mergedProps
to the underlying Toaster renderer; reference the Toaster component, props,
icons, toastOptions, classNames.toast and variant icon keys when applying the
merge.
Signed-off-by: bdjulbic <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]>
bdjulbic
approved these changes
Mar 19, 2026
dmitriy-kostianetskiy
pushed a commit
that referenced
this pull request
Mar 20, 2026
Signed-off-by: Ilya Boyandin <[email protected]> Signed-off-by: bdjulbic <[email protected]> Co-authored-by: Cursor Agent <[email protected]> Co-authored-by: bdjulbic <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: bdjulbic <[email protected]>
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
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.
Deprecate
useToastand migrate all usages to the new Sonner-based toast, while extracting legacy toaster code and aligning Sonner styles.Summary by CodeRabbit
Refactor
New Features
Style