refactor: Remove third-party web3 modal dependencies#92
Open
kushbosamiya wants to merge 18 commits intoStabilityNexus:mainfrom
Open
refactor: Remove third-party web3 modal dependencies#92kushbosamiya wants to merge 18 commits intoStabilityNexus:mainfrom
kushbosamiya wants to merge 18 commits intoStabilityNexus:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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:
WalkthroughThis PR removes RainbowKit and WalletConnect cloud dependencies, migrating to pure wagmi v2 with EIP-6963 for decentralized client-side wallet discovery. The connect button is rebuilt using wagmi hooks, wallet configuration now uses wagmi's Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ui/ConnectBtn.tsx`:
- Around line 177-194: The two network buttons in the ConnectBtn component
always call switchChain?.({ chainId: chains[0].id }) which hardcodes the first
chain; replace this placeholder behavior by presenting the available chains and
passing the chosen chain's id to switchChain. Implement a chooser (dropdown/menu
or modal) opened from the existing motion.button handlers (desktop and mobile)
that maps over chains and calls switchChain?.({ chainId: selected.id }) when an
item is clicked, display chain?.name as the current label, and reuse the same
selection logic for both the desktop button (span showing chain?.name) and the
mobile button (icon) so both open the same chain picker and do not always select
chains[0].id.
- Around line 65-68: The modal's exit animation on the motion.div (exit={{
opacity: 0, scale: 0.95 }}) won't run because the conditional render isn't
wrapped in AnimatePresence; update the ConnectBtn component to wrap the
conditional/modal rendering with framer-motion's AnimatePresence (imported from
'framer-motion') and ensure the motion.div has a unique key so AnimatePresence
can detect presence/exit, keeping the existing initial/animate/exit props on the
motion.div.
- Around line 71-74: Replace hardcoded user-visible strings in the ConnectBtn
component with constants in the existing constants file (you already import
CONNECT_BTN_LABEL): add new exports like CONNECT_WALLET_TITLE, CONNECTING_LABEL,
NO_WALLET_DETECTED_MSG, INSTALL_WALLET_HINT, and DISCONNECT_LABEL and use them
in the ConnectBtn JSX (the heading "Connect Wallet", the status "Connecting",
the messages "No wallet extensions detected." / "Please install a wallet like
MetaMask or Rabby.", and the "Disconnect" button). Update any references inside
the ConnectBtn component to use these constants to keep i18n consistency.
- Around line 94-102: The inline type assertions for connector and
variables?.connector reduce type safety; replace them with a proper
imported/declared type (e.g., import wagmi's Connector/InjectedConnector types
or declare an interface like ConnectorParams { ready?: boolean; uid?: string })
and use that type for connParams and varConnParams instead of inline casts;
update the declarations that set connParams and varConnParams (used by
isDisabled and isThisPending) to use the new type and adjust any null/undefined
checks accordingly so the compiler enforces the expected properties.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e326e016-424a-44a0-b17a-49810b99e701
⛔ Files ignored due to path filters (8)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/wallets/Phantom.svgis excluded by!**/*.svgpublic/wallets/backpack.pngis excluded by!**/*.pngpublic/wallets/brave.svgis excluded by!**/*.svgpublic/wallets/cbw.svgis excluded by!**/*.svgpublic/wallets/ctrl.svgis excluded by!**/*.svgpublic/wallets/metamask.svgis excluded by!**/*.svgpublic/wallets/rabby.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
.env.exampleapp/layout.tsxcomponents/dashboard/DashboardMetrics.tsxcomponents/layout/SideBar.tsxcomponents/ui/ConnectBtn.tsxlib/config.tslib/wallets.tspackage.jsonproviders/providers.tsx
💤 Files with no reviewable changes (2)
- app/layout.tsx
- .env.example
Author
|
@KanishkSogani @Zahnentferner please review it , if everything looks good to you, do merge it. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ui/ConnectBtn.tsx`:
- Line 28: The state variable isAccountMenuOpen is being used for two surfaces
(wallet connect modal and connected account dropdown), causing the account menu
to open unexpectedly after connection; introduce a separate boolean state (e.g.,
isWalletModalOpen with setter setIsWalletModalOpen) in ConnectBtn to represent
the wallet modal, update all places that open/close the connect modal to use
isWalletModalOpen, keep isAccountMenuOpen for the account dropdown, and ensure
any effect that watches isConnected resets both flags
(setIsAccountMenuOpen(false); setIsWalletModalOpen(false)) to avoid reuse of
stale true values; update handlers referenced in the file (open/close callbacks
around isAccountMenuOpen, setIsAccountMenuOpen, and any modal toggle code)
accordingly.
- Around line 72-105: Wrap the connect modal markup in real dialog semantics by
adding role="dialog" and aria-modal="true" to the motion.div
(key="connect-modal"), give the heading that renders CONNECT_WALLET_TITLE a
stable id and reference it via aria-labelledby on the dialog, and ensure
clicking the backdrop still calls setIsAccountMenuOpen(false); additionally add
keyboard focus management by creating a ref (e.g., dialogRef or closeButtonRef)
and, in ConnectBtn, use a useEffect that moves focus into the modal when it
opens (focus the close button or first focusable element) and optionally restore
focus on close to the trigger; this ensures screen readers and keyboard users
get proper dialog semantics and initial focus when the modal opens.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2645b3bc-3c2e-40c8-87a2-96fb286e085b
📒 Files selected for processing (3)
components/ui/ConnectBtn.tsxlib/config.tslib/constants.ts
Author
Successfully tested |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
components/ui/ConnectBtn.tsx (4)
218-227:⚠️ Potential issue | 🟡 MinorThe wrong-network recovery still hardcodes the first configured chain.
When this branch renders, the only action calls
switchChain?.({ chainId: chains[0].id }), which forces whatever chain happens to be first in the config instead of letting the user choose. Reuse the existing chain picker here, or switch to a named app default rather thanchains[0].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/ConnectBtn.tsx` around lines 218 - 227, The wrong-network button currently forces switchChain?.({ chainId: chains[0].id }) which hardcodes the first chain; change this to either open the existing ChainPicker UI or call switchChain with a named app default instead of chains[0]. Specifically, replace the direct call in the isUnsupported branch (where CONNECT_BTN_LABEL.wrongNetwork is rendered) to trigger the ChainPicker component/modal or use a configured defaultChainId (e.g., appDefaultChainId) and call switchChain?.({ chainId: appDefaultChainId }) so the user can choose or the app uses an explicit default rather than chains[0].
38-39:⚠️ Potential issue | 🟡 MinorSplit the wallet-modal state from the account-menu state.
isAccountMenuOpenstill backs both surfaces. Because the connect path intentionally leaves ittrue, a successful connection immediately renders the connected account dropdown on the next branch. Give the modal its own flag, or reset both menus whenisConnectedchanges.Also applies to: 103-103, 164-168, 283-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/ConnectBtn.tsx` around lines 38 - 39, The account menu and wallet modal are sharing state (isAccountMenuOpen) causing the account dropdown to show immediately after a successful connect; introduce a separate boolean (e.g., isWalletModalOpen with setter setIsWalletModalOpen) for the connect modal and update usages that open/close the wallet flow to use it, or alternatively add an effect watching isConnected that calls setIsAccountMenuOpen(false) and setIsNetworkMenuOpen(false) to reset both menus on connection state change; update all places referencing isAccountMenuOpen for the modal (and the handlers that toggle it) to use the new flag or ensure the effect runs (references: isAccountMenuOpen, isNetworkMenuOpen, isConnected, setIsAccountMenuOpen, setIsNetworkMenuOpen).
113-145:⚠️ Potential issue | 🟠 MajorTreat the wallet chooser as a real dialog.
This is still rendered as a plain overlay, so screen readers never get dialog semantics and focus stays on the background page. Add
role="dialog",aria-modal, anaria-labelledbytarget, and move initial focus into the modal when it opens.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/ConnectBtn.tsx` around lines 113 - 145, The modal is rendered without dialog semantics or focus management; add accessibility attributes and initial focus: give the heading element that renders CONNECT_WALLET_TITLE a unique id (e.g., connectWalletTitleId) and add role="dialog" aria-modal="true" aria-labelledby={connectWalletTitleId} to the motion.div that represents the modal; create a ref (e.g., modalCloseBtnRef or modalRef) for the close button or first focusable element and, inside a useEffect that runs when isAccountMenuOpen becomes true, call ref.current?.focus() to move initial focus into the modal (and cleanup if needed); keep existing setIsAccountMenuOpen(false) behavior for backdrop and close button.
135-135:⚠️ Potential issue | 🟡 MinorExternalize the remaining wallet UI copy.
Close, theNetworkfallback, the mobile switch label prefix, and(Current)are still hardcoded here, so this flow is only partially i18n-ready.As per coding guidelines, "User-visible strings should be externalized to resource files (i18n)."
Also applies to: 240-248, 272-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/ConnectBtn.tsx` at line 135, The remaining hardcoded user-visible strings in ConnectBtn.tsx (the "Close" sr-only span, the "Network" fallback label, the mobile switch label prefix, and the "(Current)" marker) must be externalized to your i18n resource and replaced with localized lookups; update the ConnectBtn component to import and use the project's localization helper (e.g., t or useTranslation) and replace those literal strings with keys like t('connect.close'), t('connect.networkFallback'), t('connect.mobileSwitchPrefix'), and t('connect.currentMarker'), and ensure the same change is applied to the other occurrences noted around the mobile switch and network display areas so all user-facing text is localized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commit_msg.txt`:
- Around line 1-15: The file commit_msg.txt contains PR/commit notes and should
not live in source root; remove or relocate it to a proper docs location (e.g.,
project docs or an archived changelogs/docs folder) and delete the artifact from
the repository. Locate the file named commit_msg.txt in the repo, move its
contents into the chosen documentation area or discard if redundant, then commit
the removal so the repository no longer contains this PR-metadata artifact.
In `@components/ui/ConnectBtn.tsx`:
- Around line 56-58: The wallet picker (`browserExtensionConnectors` in
ConnectBtn.tsx) filters connectors by connector.type === "injected" but
lib/config.ts registers safe() which reports type "safe", causing Safe users to
be excluded; fix by either updating the filter in ConnectBtn.tsx (the
connectors.filter used to build browserExtensionConnectors) to also include
connector.type === "safe" (or check connector.id === "safe") so Safe appears in
the picker, or remove the safe() registration from lib/config.ts to keep the
supported connector set consistent—pick one approach and apply it consistently
across ConnectBtn.tsx (browserExtensionConnectors) and lib/config.ts (safe()).
---
Duplicate comments:
In `@components/ui/ConnectBtn.tsx`:
- Around line 218-227: The wrong-network button currently forces switchChain?.({
chainId: chains[0].id }) which hardcodes the first chain; change this to either
open the existing ChainPicker UI or call switchChain with a named app default
instead of chains[0]. Specifically, replace the direct call in the isUnsupported
branch (where CONNECT_BTN_LABEL.wrongNetwork is rendered) to trigger the
ChainPicker component/modal or use a configured defaultChainId (e.g.,
appDefaultChainId) and call switchChain?.({ chainId: appDefaultChainId }) so the
user can choose or the app uses an explicit default rather than chains[0].
- Around line 38-39: The account menu and wallet modal are sharing state
(isAccountMenuOpen) causing the account dropdown to show immediately after a
successful connect; introduce a separate boolean (e.g., isWalletModalOpen with
setter setIsWalletModalOpen) for the connect modal and update usages that
open/close the wallet flow to use it, or alternatively add an effect watching
isConnected that calls setIsAccountMenuOpen(false) and
setIsNetworkMenuOpen(false) to reset both menus on connection state change;
update all places referencing isAccountMenuOpen for the modal (and the handlers
that toggle it) to use the new flag or ensure the effect runs (references:
isAccountMenuOpen, isNetworkMenuOpen, isConnected, setIsAccountMenuOpen,
setIsNetworkMenuOpen).
- Around line 113-145: The modal is rendered without dialog semantics or focus
management; add accessibility attributes and initial focus: give the heading
element that renders CONNECT_WALLET_TITLE a unique id (e.g.,
connectWalletTitleId) and add role="dialog" aria-modal="true"
aria-labelledby={connectWalletTitleId} to the motion.div that represents the
modal; create a ref (e.g., modalCloseBtnRef or modalRef) for the close button or
first focusable element and, inside a useEffect that runs when isAccountMenuOpen
becomes true, call ref.current?.focus() to move initial focus into the modal
(and cleanup if needed); keep existing setIsAccountMenuOpen(false) behavior for
backdrop and close button.
- Line 135: The remaining hardcoded user-visible strings in ConnectBtn.tsx (the
"Close" sr-only span, the "Network" fallback label, the mobile switch label
prefix, and the "(Current)" marker) must be externalized to your i18n resource
and replaced with localized lookups; update the ConnectBtn component to import
and use the project's localization helper (e.g., t or useTranslation) and
replace those literal strings with keys like t('connect.close'),
t('connect.networkFallback'), t('connect.mobileSwitchPrefix'), and
t('connect.currentMarker'), and ensure the same change is applied to the other
occurrences noted around the mobile switch and network display areas so all
user-facing text is localized.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ba57a1ae-4af9-498c-b58e-de0b067a3ac5
📒 Files selected for processing (3)
commit_msg.txtcomponents/ui/ConnectBtn.tsxlib/config.ts
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
Member
|
hey @kushbosamiya i am seeing this after connecting my wallet |
Author
|
@KanishkSogani Could you please explain how you encountered this error step by step? Also, did you use the backpack in while testing ? |
Refactor ConnectBtn to implement strict type checking with getConnectorParams and deduplicate connector entries based on ID/name match. Resolves double-injection connector overlap, enforcing type-safe reads and proper disabled state checks during pending connections.
Split shared isAccountMenuOpen state into dedicated isWalletModalOpen for connection flow. Introduced WAI-ARIA dialog semantics and useRef focus management to restore focus upon modal closure.
Refactored dedupedConnectors reduction into a tightly-bound Map loop to ensure Typescript accurately infers the wagmi 'Connector' return type during UI mapping. Also refactored custom z-index classes to standard variables as flagged by the IDE.
- Replaced wrong network early return with dropdown picker - Improved empty state UI and filtered safe connector conditionally - Externalized remaining hardcoded i18n strings
kushbosamiya
force-pushed
the
refactor/wallet-deps-removal
branch
from
2442893 to
b0f13fb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ui/ConnectBtn.tsx`:
- Around line 369-393: Add keyboard-dismiss support by adding a useEffect that
registers a keydown listener (handleKeyDown) which closes both dropdowns when
Escape is pressed; specifically watch the isNetworkMenuOpen and
isAccountMenuOpen state values and call setIsNetworkMenuOpen(false) and
setIsAccountMenuOpen(false) inside the handler, and ensure the listener is added
only while either menu is open and removed on cleanup to avoid leaks.
- Around line 277-286: Replace the plain <img> used for wallet icons with
Next.js' Image component: import Image from "next/image" and swap the <img
src={icon} alt={label} ... /> in ConnectBtn (the element using props icon and
label) to <Image src={icon} alt={label} width={28} height={28} className="h-7
w-7 rounded-lg object-contain" />; preserve the onError behavior by adding an
onError handler to the Image element that hides the image on load error
(matching the current e.currentTarget.style.display = "none" behavior) so icons
benefit from Next.js image optimization and remain consistent with the connector
list.
In `@lib/config.ts`:
- Around line 20-27: The transports configuration currently uses wagmi's http()
defaults (transports, http(), [mainnet.id], [sepolia.id], [polygon.id],
[optimism.id], [arbitrum.id], [base.id]); update it to accept explicit RPC URLs
from env vars so production uses dedicated endpoints (e.g.,
NEXT_PUBLIC_MAINNET_RPC_URL, NEXT_PUBLIC_SEPOLIA_RPC_URL, etc.) instead of
anonymous http() calls — replace each http() call with
http(process.env.<ENV_VAR>) and validate/throw a clear error if an expected env
var is missing.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d2576219-d8fe-471f-9d42-389c3938d9c3
⛔ Files ignored due to path filters (8)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/wallets/Phantom.svgis excluded by!**/*.svgpublic/wallets/backpack.pngis excluded by!**/*.pngpublic/wallets/brave.svgis excluded by!**/*.svgpublic/wallets/cbw.svgis excluded by!**/*.svgpublic/wallets/ctrl.svgis excluded by!**/*.svgpublic/wallets/metamask.svgis excluded by!**/*.svgpublic/wallets/rabby.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
.env.exampleapp/layout.tsxcomponents/dashboard/DashboardMetrics.tsxcomponents/layout/SideBar.tsxcomponents/ui/ConnectBtn.tsxlib/config.tslib/constants.tslib/wallets.tspackage.jsonproviders/providers.tsx
💤 Files with no reviewable changes (4)
- components/layout/SideBar.tsx
- .env.example
- app/layout.tsx
- package.json
- components/ui/ConnectBtn.tsx: Add keyboard-dismiss support (Escape key) for dropdown menus and migrate wallet icon <img> tags to Next.js <Image /> component for optimization. - lib/config.ts: Replace default anonymous transports with explicit RPC URL environment variable validation via getRpcUrl helper. - app/[username]/page.tsx: Resolve async client component warning by using React.use() to unwrap params. - package-lock.json: Align zod and abitype versions to fix dependency conflicts and restore broken node_modules state after npm ci failures.
Member
|
@kushbosamiya still showing me this and im using metamask! also it should not depend on what wallet a user is using as everyone use different wallets and it should work same for each and everyone. 
|
kushbosamiya
force-pushed
the
refactor/wallet-deps-removal
branch
from
eccdb15 to
473c90f
Compare
Author
|
All Tested
|
Author
|
Missing pre-push hook allows malformatted code to reach remote .husky/ only has a pre-commit script. Developers can bypass it with --no-verify or push unstaged changes, causing npm run check-format to fail publicly in CI. |
- Enable in WagmiProvider to persist session on refresh. - Refactor in to be purely reactive to . - Switch to async with proper EIP-1193 rejection handling. - Move dropdown closure logic from to a reactive on . - Fix bug where failed MetaMask switch attempts could disable network validation.
- use useAccount().chainId (undefined when disconnected) instead of useChainId() which returned mainnet=1 as default, causing false wrong-network flags and spurious dropdown close on mount - remove connector param from switchChainAsync — uid mismatch in config.state.connections.get() silently skipped connector.switchChain() so no MetaMask popup fired - guard external-chain-change useEffect with isNetworkMenuOpen to prevent setIsNetworkMenuOpen(false) firing on every mount/reconnect cycle - set ssr:false in config — wallet listeners deferred under ssr:true broke wallet→UI reactivity in Next.js App Router - disable current chain button only when chainId is confirmed numeric
Member
still not fixed i think you could tweak with default network to make it work but please make sure it works properly before next review.
|
Author
|
I would like to request you to kindly review the following: Wallet connection testing for various wallets: |
…ostic - Drop unused @metamask/sdk dependency; wallet discovery is handled natively via wagmi multiInjectedProviderDiscovery (EIP-6963) - Update INSTALL_WALLET_HINT in constants.ts to be wallet-agnostic
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
2 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.
Closes #90
refactor: Remove third-party web3 modal dependencies
1. Context
Replaced heavy web3 modal dependencies with a lightweight local implementation to fix UI dependencies and Framer Motion style overrides.
2. Implementation Details
package.json: Removed@rainbow-me/rainbowkitand related heavy dependencies.lib/wallets.ts&public/wallets/: Mapped connectors to customized local SVG/PNG assets.components/ui/ConnectBtn.tsx: Rebuilt UI using simplified flexbox center styling, fixedanytype warnings, and migratedimgtonext/image.providers/providers.tsx&app/layout.tsx: Removed RainbowKit configs.components/dashboard/DashboardMetrics.tsx&components/layout/SideBar.tsx: Removed unused import definitions.3. Verification
npm run lint: Passed with 0 errors.Summary by CodeRabbit
New Features
Refactor