Repository navigation
feat(toast): migrate to sonner library - #589
Conversation
Replace custom React Context-based toast implementation with sonner.
Maintains backward compatibility via useToast() hook that returns { toast }.
- Add sonner@2.0.7 dependency
- Simplify Toast.tsx from 97 lines to 35
- Preserve existing toast styling (colors, shadows, duration)
Deploying wallet-bitcoin with
|
| Latest commit: |
467f230
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5db6aaa8.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://wt-toast-sonner-20260430.wallet-bitcoin.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
467f230
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a9c8f268.arkade-wallet.pages.dev |
| Branch Preview URL: | https://wt-toast-sonner-20260430.arkade-wallet.pages.dev |
Deploying tmp-boltz-upstream-mainnet-arkade-wallet with
|
| Latest commit: |
467f230
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b127dff4.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
| Branch Preview URL: | https://wt-toast-sonner-20260430.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
✅ Approved — clean library swap, no protocol impact
Reviewed full diff + all 8 consumer call sites. This is a UI-only change (toast notifications) with zero protocol-critical surface.
Findings
CI failure — Prettier formatting (must fix before merge)
src/components/Toast.tsx fails the format:check step. Run pnpm format:write or prettier --write src/components/Toast.tsx and push.
Backward compatibility — verified ✅
All 8 consumers use const { toast } = useToast() → toast('string'). The new useToast() returns { toast } wrapping sonner's toast function, which accepts a string as its first argument. All call sites are compatible without changes:
src/components/Text.tsx:68src/components/Table.tsx:18src/components/ExpandAddresses.tsx:36src/screens/Settings/Backup.tsx:38src/screens/Settings/Logs.tsx:19src/screens/Settings/Delegates.tsx:126src/screens/Wallet/Receive/QrCode.tsx:58
Test wrappers (ToastProvider in test files) remain compatible — ToastProvider still renders {children}.
Minor nits (non-blocking):
toast-in/toast-outCSS keyframe animations were referenced in the old code but defined inline (not in a CSS file), so no dead CSS to clean up. Clean.- The fallback colors
#1a1a1a/#fafafaonvar(--toast-bg)/var(--toast-color)are a nice improvement over the old code which had no fallbacks. textAlign: 'center' as const— correct TypeScript narrowing, fine.
Not protocol-critical. No VTXO, signing, forfeit, round, or exit path code touched.
🔧 Action needed: fix Prettier formatting, then this is good to merge.
🤖 Arkana
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
✅ Approved — clean library swap, no protocol impact
Reviewed full diff + all 8 consumer call sites. This is a UI-only change (toast notifications) with zero protocol-critical surface.
Findings
CI failure — Prettier formatting (must fix before merge)
src/components/Toast.tsx fails the format:check step. Run pnpm format:write or prettier --write src/components/Toast.tsx and push.
Backward compatibility — verified ✅
All 8 consumers use const { toast } = useToast() → toast('string'). The new useToast() returns { toast } wrapping sonner's toast function, which accepts a string as its first argument. All call sites are compatible without changes:
src/components/Text.tsx:68src/components/Table.tsx:18src/components/ExpandAddresses.tsx:36src/screens/Settings/Backup.tsx:38src/screens/Settings/Logs.tsx:19src/screens/Settings/Delegates.tsx:126src/screens/Wallet/Receive/QrCode.tsx:58
Test wrappers (ToastProvider in test files) remain compatible — ToastProvider still renders {children}.
Minor nits (non-blocking):
- No orphaned CSS — old
toast-in/toast-outanimations were inline styles, not in a stylesheet. Clean. - Fallback colors
#1a1a1a/#fafafaon CSS vars are an improvement over the old code which had no fallbacks. textAlign: 'center' as const— correct TypeScript narrowing.
Not protocol-critical. No VTXO, signing, forfeit, round, or exit path code touched.
🔧 Action needed: fix Prettier formatting, then this is good to merge.
🤖 Arkana
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughReplaces the custom toast implementation with sonner (adds ChangesToast System Migration
Dev Environment Overrides
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/components/Toast.tsx`:
- Around line 12-33: Prettier flagged formatting in the Toast component JSX
(around the Toaster usage); re-run the project's formatter (e.g., Prettier) on
src/components/Toast.tsx or manually reformat the JSX block containing Toaster
and children so it matches the repo's style rules, then save and commit the
updated file; specifically ensure the Toaster prop formatting (position,
richColors, toastOptions, style) and trailing commas/spacing conform to the
configured formatter so CI style checks pass.
🪄 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: CHILL
Plan: Pro
Run ID: 09216df2-a37b-4a91-bbc5-2af15b194c89
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.jsonsrc/components/Toast.tsx
Merge orderThis PR is independent and can merge anytime. Related PRs:
This sonner migration is standalone — no dependency on Tailwind. |
Center Sonner toast boxes and their text so short copy confirmations hug their content instead of using the default fixed-width left-aligned layout. Add scoped toast CSS for the Sonner toaster and content wrapper while preserving the existing toast colors, padding, duration, and top-center placement.
Keep local dev sessions on the configured Arkade server when VITE_DEV_NSEC and VITE_ARK_SERVER are present, even if earlier localStorage pointed at the localhost ASP. Allow the Backup screen to reveal the dev nsec directly in development so the funded default wallet can exercise private-key copy toasts without an impossible password prompt.
|
@bordalix updated this based on your suggestion. Good call — the default Sonner layout keeps a fixed-width toast with left-anchored content, so I added a scoped override to center the toast and make it hug the message content. |
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
✅ Re-reviewed new commits — still approved
Three commits since my last review (April 30):
40ea642 — fix(toast): format sonner toaster props
Prettier fix I flagged. ✅
e36624e — fix(toast): center sonner content
New Toast.css with data-sonner-toaster centering overrides. Addresses bordalix's feedback. CSS-only, no logic changes. ✅
bd2e957 — chore(dev): honor funded wallet env
Extends existing VITE_DEV_NSEC pattern (already in wallet.tsx, App.tsx) to two more files:
config.tsx:shouldUseDevEnvArkServer()overridesaspUrlwhenDEV && VITE_DEV_NSEC && VITE_ARK_SERVER && aspUrl === devServer. Guard onaspUrl === devServerprevents accidentally redirecting a mainnet wallet. Correct.Backup.tsx: Skips password verification in dev mode, sets nsec directly from env.import.meta.env.DEVguard ensures dead-code elimination in prod builds. Consistent with existing pattern.Backup.tsx:91: Removedconsole.logthat was printing config (potentially including sensitive data). Good cleanup.vite-env.d.ts: AddsVITE_DEV_NSECtype declaration..env.localis gitignored. ✅
Minor nit (non-blocking): shouldUseDevEnvArkServer already checks import.meta.env.VITE_ARK_SERVER is truthy, making the && devArkServer check at the call site redundant. Harmless.
Not protocol-critical. No VTXO, signing, forfeit, round, or exit path code touched.
🤖 Arkana
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/providers/config.tsx`:
- Around line 76-77: The predicate shouldUseDevEnvArkServer currently ties the
VITE_ARK_SERVER override to VITE_DEV_NSEC; remove that unrelated gate so the
override only requires dev mode and the VITE_ARK_SERVER flag. Update the
function (shouldUseDevEnvArkServer) to return true when import.meta.env.DEV &&
import.meta.env.VITE_ARK_SERVER && aspUrl === devServer, removing the
import.meta.env.VITE_DEV_NSEC check.
In `@src/screens/Settings/Backup.tsx`:
- Around line 48-50: The code uses import.meta.env.VITE_DEV_NSEC (checked under
import.meta.env.DEV) to setNsec directly, which exposes a secret via
client-facing VITE_* env vars; replace this with a non-sensitive, clearly
test-only fixture or load the value from a local-only mechanism that never
enters the frontend build (e.g., a developer-only config file or secure IPC/read
from filesystem in dev mode). Update the branch that references
import.meta.env.VITE_DEV_NSEC and setNsec to instead use the sanitized test
value or local-only loader and add a comment documenting that this value is
strictly test/dev-only and must not be committed to production or a VITE_* env
var.
🪄 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: CHILL
Plan: Pro
Run ID: 66f5eab9-e667-452f-a64f-0710e356e77a
📒 Files selected for processing (5)
src/components/Toast.csssrc/components/Toast.tsxsrc/providers/config.tsxsrc/screens/Settings/Backup.tsxsrc/vite-env.d.ts
Remove the Backup screen shortcut that read VITE_DEV_NSEC directly from client code, keeping private key display on the normal password verification path. Keep the local Ark server override independent from the funded wallet env so stale localhost config can still recover during development.
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
✅ Re-reviewed — 4 new commits since last review, all clean
What changed
40ea642 — format fix
Addresses the Prettier issue I flagged. ✅
e36624e — toast centering CSS
New Toast.css with sonner override selectors for centered positioning. Pure cosmetic, no issues.
bd2e957 — dev server config override
shouldUseDevEnvArkServer() in config.tsx:76-77 is properly triple-gated: import.meta.env.DEV && import.meta.env.VITE_ARK_SERVER && aspUrl === devServer. Cannot fire in production. Clean.
35fb075 — remove console.log + add VITE_DEV_NSEC type
- Removes
console.log('Updating backup with new config', newConfig)fromBackup.tsx:91— good, stops leaking config objects to console. - Adds
VITE_DEV_NSECtype declaration tovite-env.d.ts— just TypeScript typing for an env var already consumed byApp.tsxandwallet.tsxon main.
Scope note
PR has expanded beyond toast migration to include dev-environment tooling. Consider splitting if this grows further, but current scope is fine.
Not protocol-critical. No VTXO, signing, forfeit, round, or exit path code touched.
🤖 Arkana
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
✅ Re-reviewed — 4 new commits since last review, all clean
What changed
40ea642 — format fix
Addresses the Prettier issue I flagged. ✅
e36624e — toast centering CSS
New Toast.css with sonner override selectors for centered positioning. Pure cosmetic, no issues.
bd2e957 — dev server config override
shouldUseDevEnvArkServer() in config.tsx:76-77 is properly triple-gated: import.meta.env.DEV && import.meta.env.VITE_ARK_SERVER && aspUrl === devServer. Cannot fire in production. Clean.
35fb075 — remove console.log + add VITE_DEV_NSEC type
- Removes
console.log('Updating backup with new config', newConfig)fromBackup.tsx:91— good, stops leaking config objects to console. - Adds
VITE_DEV_NSECtype declaration tovite-env.d.ts— just TypeScript typing for an env var already consumed byApp.tsxandwallet.tsxon main.
Scope note
PR has expanded beyond toast migration to include dev-environment tooling. Consider splitting if this grows further, but current scope is fine.
Not protocol-critical. No VTXO, signing, forfeit, round, or exit path code touched.
🤖 Arkana
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
✅ Re-reviewed new commits — still clean, approval maintained
Focused on delta since my last review (3 new commits + merge from master).
New changes reviewed
src/components/Toast.css — centering CSS for sonner toasts. Pure layout, no issues.
src/providers/config.tsx — shouldUseDevEnvArkServer() overrides localhost:7070 with VITE_ARK_SERVER in dev. Gated by import.meta.env.DEV → dead code in production builds. Safe.
src/screens/Settings/Backup.tsx — removes a stray console.log. Trivial.
src/vite-env.d.ts — adds VITE_DEV_NSEC type declaration for the already-used env var in App.tsx:103-104 and wallet.tsx:155. Was previously missing from the type file.
Security note ✅
Commit chore(dev): honor funded wallet env briefly added a Backup screen shortcut that read VITE_DEV_NSEC directly. Commit fix(dev): avoid exposing funded key in backup correctly reverted that — keeping the private key on the normal password-verification path. Good catch by the author.
Minor nit (non-blocking)
config.tsx:76: shouldUseDevEnvArkServer already checks import.meta.env.VITE_ARK_SERVER is truthy, then line 144 checks devArkServer (same value) again. Redundant but harmless.
Not protocol-critical. No VTXO, signing, forfeit, round, or exit path code touched.
🤖 Arkana
Summary
useToast()hookChanges
sonner@2.0.7dependencyToastProvidernow wraps children with sonner's<Toaster />useToast()returns{ toast }for backward compat (consumers can migrate to importingtoastdirectly)Test plan
Summary by CodeRabbit
New Features
Style
Chores