refactor(wallet): remove orphaned wallet status balance UI - #3584
Conversation
The WalletStatus balance dropdown was removed from the top nav along with the ui_top_nav flag and legacy sidebar (#3564). What remained was a self-contained chain of the old UI reachable only through YourAccount's !address branch, which HomeContainer never mounts (it gates on !!address). Delete the dead chain (WalletStatus, ConnectWallet, ManagedWalletPopup, useManagedEscrowFaqModal + specs), drop the unreachable branch from YourAccount, and clean up the stale test and doc comment. No user-facing behavior change.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (7)
📝 WalkthroughWalkthroughThe PR removes legacy wallet connection and managed-wallet UI components and tests. ChangesWallet UI removal
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/deploy-web/src/components/home/YourAccount/YourAccount.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/deploy-web/src/hooks/useOnboardingChrome.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — clean, mechanical dead-code removal.
Extended reasoning...
Overview
This PR deletes an orphaned UI chain (WalletStatus, ConnectWallet, ManagedWalletPopup, useManagedEscrowFaqModal) that became unreachable after a prior PR (#3564) removed the top nav that used to render WalletStatus. The only remaining edits to non-deleted files are in YourAccount.tsx (removing the now-dead !address branch and its ConnectWallet usage, replacing it with an early return null), its spec (dropping the corresponding test), and a stale doc-comment reference in useOnboardingChrome.ts.
Security risks
None. This is pure deletion of unreachable client-side UI plus a small conditional simplification in YourAccount.tsx. No auth, crypto, data access, or user input handling is touched.
Level of scrutiny
Low-to-moderate scrutiny is appropriate: this is a dead-code removal refactor, not new logic. I independently verified the core claim rather than trusting the PR description at face value — grepped for any remaining references to the four deleted symbols (none found) and confirmed in HomeContainer.tsx:80 that YourAccount is only rendered when isSettingsInit && !!address, meaning the deleted !address branch inside YourAccount could never have fired. The behavioral change (early return null vs. wrapping in a fragment when !address) is equivalent since the !address case rendered nothing observable via the removed branch's sibling checks in the live-render path anyway (this component is never even mounted without an address).
Other factors
No bugs were reported by the bug hunting system. The spec changes are consistent with the deleted symbols (removed the one test exercising the now-deleted ConnectWallet render path), and the remaining YourAccount.spec.tsx tests were unaffected in intent. Scope is well-contained to the described dead-code chain with no drive-by changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3584 +/- ##
==========================================
- Coverage 76.46% 75.68% -0.78%
==========================================
Files 1141 1047 -94
Lines 29722 27330 -2392
Branches 7398 6912 -486
==========================================
- Hits 22726 20684 -2042
+ Misses 6169 5849 -320
+ Partials 827 797 -30
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
2 participants
Why
The
WalletStatusbalance dropdown was removed from the top nav when theui_top_navflag and legacy sidebar shell were deleted (#3564). What was left behind is a self-contained chain of that old UI with no live entry point — it's only reachable throughYourAccount's!addressbranch, andHomeContainernever rendersYourAccountunless!!addressis true. So the branch can't fire in production, and everything hanging off it is dead code.This just clears out that leftover so the wallet UI surface reflects what's actually rendered.
What
Deleted the orphaned chain (all exclusively used by each other, no other importers):
components/layout/WalletStatus.tsx(+ spec)components/shared/ConnectWallet.tsxcomponents/wallet/ManagedWalletPopup/ManagedWalletPopup.tsx(+ spec) — thecomponents/wallet/dir is now gonehooks/useManagedEscrowFaqModal.tsx— only consumer wasManagedWalletPopupEdited:
YourAccount.tsx— dropped theConnectWalletimport/dependency and the unreachable!addressbranch; the component now early-returnsnullwhen there's no address instead of wrapping everything in a fragment.YourAccount.spec.tsx— removed the stale "renders ConnectWallet when wallet has no address" test.useOnboardingChrome.ts— removed a danglingWalletStatusmention in a doc comment.Shared hooks (
useWallet,useWalletBalance,AddFundsLink) are untouched. No user-facing behavior change — the removed branch was already unreachable.Verification: no remaining references to any of the four symbols in
deploy-web/src;tsc --noEmitshows no new errors from the changed files;YourAccountunit suite passes (12/12); lint clean on the changed files.Summary by CodeRabbit