Toasts - #15
Conversation
📝 WalkthroughWalkthroughThis pull request implements a lightweight toast notification system and integrates wallet connectivity features. The toast system includes a context-based provider with deduplication and auto-dismiss behavior, accompanied by CSS styling. Dashboard and pool components are updated to use wagmi hooks for wallet interactions and to dispatch toast notifications on transaction success or error. The main entry point is configured with the ToastProvider and Wagmi infrastructure. Unit tests validate the toast reducer logic and configuration. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
src/ui/toast/ToastProvider.tsx (1)
26-31: Dedup checks only the single last toast, not the full visible set.If toasts A → B → A fire within the dedup window, the second A won't be deduplicated because
lastToastis B. This may be intentional for simplicity, but worth noting.src/components/pool-display.tsx (1)
82-99: Consider:onSuccesscan run after component unmount.
waitForTransactionReceiptis async and may resolve after the component unmounts (e.g., user navigates away). Callingtoast()/setSuccessMessage()on an unmounted component will either no-op (toast via context) or trigger a state update warning (setErrorMessage). The status message setState calls are the concern here.This is pre-existing behavior, not introduced by this PR, so just flagging for awareness.
src/components/dashboard-page.tsx (1)
52-54: Dual error display: toast + inline banner.The user sees the same error in both a persistent toast (
duration: 0) and the inline#error-message-wrapperbanner. This is intentional for backward compatibility, but consider whether showing both simultaneously is the desired UX long-term — it could feel redundant.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
src/components/dashboard-page.tsx (2)
54-57: Errors are shown in both a toast and the inline status banner — intentional?
toast(message, "error", 0)fires a persistent toast, whilesetErrorMessage(message)also populates the inline error section (lines 72–79). If dual display is intentional, no action needed; otherwise pick one.
21-21:window.ethereummay not be injected yet at render time.Some wallets inject
ethereumasynchronously. This check can returnfalseon first render, thentrueafter. Consider using wagmi'suseConnectors()to detect available connectors instead of a rawwindow.ethereumcheck.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/ui/toast/ToastContainer.tsx (1)
16-23:ToastItemProps.toastduplicates theToastinterface.You already import from
./types. ReusingToastdirectly is cleaner.Proposed fix
+import type { Toast, ToastVariant } from "./types"; -import type { ToastVariant } from "./types"; -interface ToastItemProps { - toast: { - id: string; - message: string; - variant: ToastVariant; - }; - onDismiss: (id: string) => void; -} +interface ToastItemProps { + toast: Toast; + onDismiss: (id: string) => void; +}src/ui/toast/ToastProvider.tsx (2)
52-73: Duplicated dedup logic betweentoast()and the reducer.Lines 54–60 replicate the same dedup check that exists in
toastReducer(lines 22–27). If the dedup criteria change, both must be updated in lockstep. Additionally,state.lastToastin the callback closure can be stale relative to what the reducer sees (TOCTOU).Consider removing the callback-side check and having the reducer return a signal (e.g., via a ref or separate state) indicating whether the toast was actually added. Alternatively, accept the duplication but add a comment linking the two.
18-27: Reducer callsDate.now()— impure.Line 23 makes the reducer non-deterministic. This is pragmatic for dedup but complicates testing and violates the React expectation that reducers are pure functions. If you want strict purity, pass the current timestamp as part of the action payload (you already have
action.payload.timestamp).Proposed fix
case "ADD": { if (state.lastToast && state.lastToast.message === action.payload.message) { - const timeSinceLast = Date.now() - state.lastToast.timestamp; + const timeSinceLast = action.payload.timestamp - state.lastToast.timestamp; if (timeSinceLast < TOAST_CONFIG.dedupWindow) { return state; } }
| import { BaseError } from "viem"; | ||
| import { useStatusMessageState, useStatusMessageDispatch } from "../context/status-message.tsx"; | ||
| import { useToast } from "../ui/toast"; | ||
| import { ConnectWalletButton } from "./connect-wallet.tsx"; |
There was a problem hiding this comment.
ConnectWalletButton is imported but never used.
Remove the unused import.
-import { ConnectWalletButton } from "./connect-wallet.tsx";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ConnectWalletButton } from "./connect-wallet.tsx"; |
🧰 Tools
🪛 ESLint
[error] 9-9: 'ConnectWalletButton' is defined but never used.
(@typescript-eslint/no-unused-vars)
| const { isConnected } = useAppKitAccount(); | ||
| const { chainId } = useAppKitNetwork(); | ||
| const { address, chain, status } = useAccount(); |
There was a problem hiding this comment.
Mixed wallet state sources: isConnected from AppKit vs address from wagmi.
Line 15 reads isConnected from useAppKitAccount() and line 17 reads address/chain/status from wagmi's useAccount(). Line 42 then checks isConnected && address — these can momentarily disagree during connection/disconnection, causing flicker or a brief broken state where one is truthy and the other isn't.
Pick one source of truth. Since the inline buttons already use wagmi's connect/disconnect, consider deriving isConnected from useAccount() as well (e.g., status === "connected").
Also applies to: 42-42
| import { useReadContract, useWriteContract, usePublicClient } from "wagmi"; | ||
| import { parseUnits, BaseError } from "viem"; | ||
| import { stakingContract } from "../constants/contracts"; | ||
| import erc20Abi from "../abis/erc20"; | ||
| import { useErc20Token } from "../hooks/erc20Token"; | ||
| import { useState } from "react"; | ||
| import { waitForTransactionReceipt } from "viem/actions"; | ||
| import { useStatusMessageState, useStatusMessageDispatch } from "../context/status-message.tsx"; | ||
| import { useToast } from "../ui/toast"; |
There was a problem hiding this comment.
Remove unused imports.
ESLint confirms useWriteContract, parseUnits, erc20Abi, and useStatusMessageState are all unused. These appear to be leftovers from removed code.
Proposed fix
-import { useReadContract, useWriteContract, usePublicClient } from "wagmi";
-import { parseUnits, BaseError } from "viem";
+import { useReadContract, usePublicClient } from "wagmi";
+import { BaseError } from "viem";
import { stakingContract } from "../constants/contracts";
-import erc20Abi from "../abis/erc20";
import { useErc20Token } from "../hooks/erc20Token";
import { useState } from "react";
import { waitForTransactionReceipt } from "viem/actions";
-import { useStatusMessageState, useStatusMessageDispatch } from "../context/status-message.tsx";
+import { useStatusMessageDispatch } from "../context/status-message.tsx";🧰 Tools
🪛 ESLint
[error] 2-2: 'useWriteContract' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 3-3: 'parseUnits' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 5-5: 'erc20Abi' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 9-9: 'useStatusMessageState' is defined but never used.
(@typescript-eslint/no-unused-vars)
|
I think there is still a build-blocking issue in This PR adds a second wagmi config block before the existing It also exports that new |
3 participants
PR Description
Summary
Implemented a minimal toast system for consistent error/success feedback across key flows (RPC read failures, approve/stake actions, reward claim) without introducing large UI libraries.
Fixes #8
Features
Validation
Notes