Fix: Navbar overlap#72
Conversation
📝 WalkthroughWalkthroughThe PR adjusts navigation spacing and padding for responsive design in the Navbar component, and refactors token verification logic in CreateInvoice to support chain-aware provider selection with fallback to public RPCs based on chainId, while adding wallet client to effect dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant CreateInvoice as CreateInvoice Component
participant VerifyToken as verifyToken Function
participant WalletCheck as Wallet Connected?
participant WalletProvider as Wallet Provider
participant PublicRPC as Public RPC Provider
participant TokenAPI as Token Resolution
CreateInvoice->>VerifyToken: Call verifyToken(tokenAddress, chainId)
VerifyToken->>WalletCheck: Check if walletClient exists
alt Wallet Connected
WalletCheck->>WalletProvider: Use wallet provider
WalletProvider->>TokenAPI: Resolve token details
else Wallet Disconnected
WalletCheck->>PublicRPC: Select RPC for chainId
PublicRPC->>TokenAPI: Resolve token details
end
TokenAPI->>VerifyToken: Return symbol, name, decimals, chainId
VerifyToken->>CreateInvoice: Return verified token payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/page/CreateInvoice.jsx`:
- Around line 259-274: The code can leave `provider` undefined when neither
`isConnected` nor `chainIdToUse` are available; update the logic in
CreateInvoice.jsx (the block that chooses between
`BrowserProvider(walletClient)` and `new ethers.JsonRpcProvider(rpcUrl,
Number(chainIdToUse))`) to explicitly handle the fallback case: check after
those branches if `provider` is still undefined and call
`setTokenVerificationState("error")` and return (or surface a user-friendly
error message) so downstream contract calls don't run with an undefined
`provider`; reference `provider`, `isConnected`, `chainIdToUse`, `walletClient`,
and `setTokenVerificationState` when implementing the check.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/Navbar.jsxfrontend/src/page/CreateInvoice.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/page/CreateInvoice.jsx (3)
frontend/src/page/GenerateLink.jsx (2)
walletClient(30-30)verifyToken(103-123)frontend/src/page/CreateInvoicesBatch.jsx (3)
walletClient(59-59)account(61-61)verifyToken(263-290)frontend/src/contractsABI/ERC20_ABI.js (2)
ERC20_ABI(1-10)ERC20_ABI(1-10)
🔇 Additional comments (3)
frontend/src/components/Navbar.jsx (1)
146-146: LGTM! Responsive spacing adjustments for navbar overlap fix.The changes appropriately reduce spacing at smaller viewports and restore original values at
xlbreakpoints. Therelative left-4positioning on the desktop nav wrapper is a reasonable approach to address the overlap issue.Also applies to: 159-159, 186-189, 197-200
frontend/src/page/CreateInvoice.jsx (2)
160-160: LGTM! Dependency array correctly includeswalletClient.Adding
walletClientensures URL params are re-processed when the wallet connection state changes, allowing the token verification to use the appropriate provider.
276-296: Token verification logic is sound.The Promise.all with individual
.catch()fallbacks ensures graceful degradation when token metadata methods fail. IncludingchainIdin the verified token payload correctly supports the chain-aware functionality.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if (typeof window !== "undefined" && isConnected) { | ||
| // Fallback to wallet provider if no chainId specified | ||
| provider = new BrowserProvider(walletClient); | ||
| // If chainId is available, always use public RPC (works without wallet) | ||
| } else if (chainIdToUse) { | ||
| const rpcUrl = rpcUrls[chainIdToUse]; | ||
|
|
||
| if (!rpcUrl) { | ||
| console.error(`Unsupported chain ${chainIdToUse}. Supported chains: Ethereum (1), Ethereum Classic (61), Polygon (137), BNB Smart Chain (56), Base (8453), Sepolia (11155111)`); | ||
| setTokenVerificationState("error"); | ||
| return; | ||
| } | ||
| } catch (error) { | ||
| console.error("Verification failed:", error); | ||
| setTokenVerificationState("error"); | ||
| // Use JsonRpcProvider with timeout for faster response | ||
| provider = new ethers.JsonRpcProvider(rpcUrl, Number(chainIdToUse)); | ||
| } |
There was a problem hiding this comment.
Handle the case when neither wallet nor chainId is available.
If isConnected is false and chainIdToUse is undefined (e.g., no wallet connected, no chain URL param, and account?.chainId is undefined), provider remains undefined. The subsequent contract call will fail with a generic error.
Consider adding an explicit check and user-friendly error message:
Proposed fix
if (typeof window !== "undefined" && isConnected) {
- // Fallback to wallet provider if no chainId specified
+ // Use wallet provider when connected
provider = new BrowserProvider(walletClient);
- // If chainId is available, always use public RPC (works without wallet)
} else if (chainIdToUse) {
+ // Use public RPC when chainId is available (works without wallet)
const rpcUrl = rpcUrls[chainIdToUse];
if (!rpcUrl) {
console.error(`Unsupported chain ${chainIdToUse}. Supported chains: Ethereum (1), Ethereum Classic (61), Polygon (137), BNB Smart Chain (56), Base (8453), Sepolia (11155111)`);
setTokenVerificationState("error");
return;
}
// Use JsonRpcProvider with timeout for faster response
provider = new ethers.JsonRpcProvider(rpcUrl, Number(chainIdToUse));
+ } else {
+ console.error("Cannot verify token: No wallet connected and no chain specified");
+ setTokenVerificationState("error");
+ return;
}📝 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.
| if (typeof window !== "undefined" && isConnected) { | |
| // Fallback to wallet provider if no chainId specified | |
| provider = new BrowserProvider(walletClient); | |
| // If chainId is available, always use public RPC (works without wallet) | |
| } else if (chainIdToUse) { | |
| const rpcUrl = rpcUrls[chainIdToUse]; | |
| if (!rpcUrl) { | |
| console.error(`Unsupported chain ${chainIdToUse}. Supported chains: Ethereum (1), Ethereum Classic (61), Polygon (137), BNB Smart Chain (56), Base (8453), Sepolia (11155111)`); | |
| setTokenVerificationState("error"); | |
| return; | |
| } | |
| } catch (error) { | |
| console.error("Verification failed:", error); | |
| setTokenVerificationState("error"); | |
| // Use JsonRpcProvider with timeout for faster response | |
| provider = new ethers.JsonRpcProvider(rpcUrl, Number(chainIdToUse)); | |
| } | |
| if (typeof window !== "undefined" && isConnected) { | |
| // Use wallet provider when connected | |
| provider = new BrowserProvider(walletClient); | |
| } else if (chainIdToUse) { | |
| // Use public RPC when chainId is available (works without wallet) | |
| const rpcUrl = rpcUrls[chainIdToUse]; | |
| if (!rpcUrl) { | |
| console.error(`Unsupported chain ${chainIdToUse}. Supported chains: Ethereum (1), Ethereum Classic (61), Polygon (137), BNB Smart Chain (56), Base (8453), Sepolia (11155111)`); | |
| setTokenVerificationState("error"); | |
| return; | |
| } | |
| // Use JsonRpcProvider with timeout for faster response | |
| provider = new ethers.JsonRpcProvider(rpcUrl, Number(chainIdToUse)); | |
| } else { | |
| console.error("Cannot verify token: No wallet connected and no chain specified"); | |
| setTokenVerificationState("error"); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In `@frontend/src/page/CreateInvoice.jsx` around lines 259 - 274, The code can
leave `provider` undefined when neither `isConnected` nor `chainIdToUse` are
available; update the logic in CreateInvoice.jsx (the block that chooses between
`BrowserProvider(walletClient)` and `new ethers.JsonRpcProvider(rpcUrl,
Number(chainIdToUse))`) to explicitly handle the fallback case: check after
those branches if `provider` is still undefined and call
`setTokenVerificationState("error")` and return (or surface a user-friendly
error message) so downstream contract calls don't run with an undefined
`provider`; reference `provider`, `isConnected`, `chainIdToUse`, `walletClient`,
and `setTokenVerificationState` when implementing the check.
|
Share the screen shot of the changes you made. |
|
#71 you can check here is bug image is available |
|
I meant to say that, share the image of the ui that changed after the fix. |
|
available in this top or pr and some images here also #70 present @kumawatkaran523 |
issue - #71
@Zahnentferner
@kumawatkaran523
Summary by CodeRabbit
Style
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.