feat(components): extract WalletConnectionGuard shared component (#4)#98
feat(components): extract WalletConnectionGuard shared component (#4)#98devcarole wants to merge 3 commits intoFundable-Protocol:mainfrom
Conversation
📝 WalkthroughWalkthroughAdded a new WalletConnectionGuard component and integrated it into distribution, offramp, and payment-stream pages so page content renders only when a Stellar wallet is connected; offramp replaces its ProtectedRoute with the guard, distribution/payment-stream render their content inside the guard. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
|
@devcarole Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/src/components/modules/wallet/WalletConnectionGuard.tsx (1)
7-7: Remove unused import.
cnis imported but not used anywhere in this component.🧹 Proposed fix
import { Button } from "@/components/ui/button"; -import { cn } from "@/lib/utils"; import { useWallet } from "@/providers/StellarWalletProvider";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/modules/wallet/WalletConnectionGuard.tsx` at line 7, Remove the unused import 'cn' from the top of the WalletConnectionGuard component file; locate the import statement "import { cn } from '@/lib/utils';" in WalletConnectionGuard.tsx and delete it so there are no unused imports remaining.apps/web/src/app/(overview)/payment-stream/page.tsx (1)
24-33: Redundant guards and potential inconsistency in child placement.Similar to the distribution page,
ProtectedRoute(line 24) andWalletConnectionGuard(line 27) both check wallet connection status, making the inner guard redundant when disconnected.Additionally,
StreamsHistory(line 31) is placed outsideWalletConnectionGuardbut insideProtectedRoute. This creates ambiguity:
- Currently,
ProtectedRouteblocks everything when disconnected, soStreamsHistoryis hidden.- If
ProtectedRouteis removed (to useWalletConnectionGuardconsistently),StreamsHistorywould render even when the wallet is disconnected.Clarify the intended behavior:
- If
StreamsHistoryshould be hidden when disconnected, wrap it insideWalletConnectionGuardas well.- If
StreamsHistorycan be viewed without a wallet, removeProtectedRouteand keep the current structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(overview)/payment-stream/page.tsx around lines 24 - 33, ProtectedRoute and WalletConnectionGuard are redundant and StreamsHistory sits outside WalletConnectionGuard causing ambiguous visibility; decide the intended UX and update the JSX accordingly: either (A) keep ProtectedRoute but move StreamsHistory inside WalletConnectionGuard so both CreatePaymentStream and StreamsHistory are only shown when the wallet is connected (wrap StreamsHistory/Suspense/StreamsTableSkeleton with WalletConnectionGuard), or (B) remove ProtectedRoute entirely so StreamsHistory can render without a wallet while keeping CreatePaymentStream inside WalletConnectionGuard; adjust the component tree around ProtectedRoute, WalletConnectionGuard, CreatePaymentStream, StreamsHistory, and StreamsTableSkeleton to reflect the chosen behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/`(overview)/distribution/page.tsx:
- Around line 387-388: The page nests two components that both guard on
useWallet().address—ProtectedRoute and WalletConnectionGuard—making one
redundant; remove the extra wrapper so only one guard remains. Decide which to
keep: if you want the customizable title/description/actionLabel/container/card
classes, keep ProtectedRoute and remove the WalletConnectionGuard wrapper around
the page content; if you prefer WalletConnectionGuard semantics, remove
ProtectedRoute and re-add any needed metadata/props
(title/description/actionLabel/container/card classes) to WalletConnectionGuard
or the page layout. Make the change by editing the JSX that currently wraps the
page (references: ProtectedRoute, WalletConnectionGuard, and
useWallet().address) so only the chosen guard remains.
In `@apps/web/src/components/modules/wallet/WalletConnectionGuard.tsx`:
- Around line 19-21: The guard currently shows the "Connect Wallet" prompt
whenever address is falsy, causing a hydration flash because address is restored
asynchronously; update WalletConnectionGuard (which uses useWallet and currently
reads address and openModal) to also read connectionStatus from useWallet and if
connectionStatus === "idle" and address is null return null (or a loading
skeleton) instead of the connect UI, and only render the connect prompt when
connectionStatus !== "idle" && !address; this mirrors the NetworkSwitcher
pattern and ensures StellarWalletProvider's initial restore phase doesn't
trigger the connect prompt.
---
Nitpick comments:
In `@apps/web/src/app/`(overview)/payment-stream/page.tsx:
- Around line 24-33: ProtectedRoute and WalletConnectionGuard are redundant and
StreamsHistory sits outside WalletConnectionGuard causing ambiguous visibility;
decide the intended UX and update the JSX accordingly: either (A) keep
ProtectedRoute but move StreamsHistory inside WalletConnectionGuard so both
CreatePaymentStream and StreamsHistory are only shown when the wallet is
connected (wrap StreamsHistory/Suspense/StreamsTableSkeleton with
WalletConnectionGuard), or (B) remove ProtectedRoute entirely so StreamsHistory
can render without a wallet while keeping CreatePaymentStream inside
WalletConnectionGuard; adjust the component tree around ProtectedRoute,
WalletConnectionGuard, CreatePaymentStream, StreamsHistory, and
StreamsTableSkeleton to reflect the chosen behavior.
In `@apps/web/src/components/modules/wallet/WalletConnectionGuard.tsx`:
- Line 7: Remove the unused import 'cn' from the top of the
WalletConnectionGuard component file; locate the import statement "import { cn }
from '@/lib/utils';" in WalletConnectionGuard.tsx and delete it so there are no
unused imports remaining.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 76d7f729-35f7-4b86-b97c-46200b06493f
📒 Files selected for processing (4)
apps/web/src/app/(overview)/distribution/page.tsxapps/web/src/app/(overview)/offramp/page.tsxapps/web/src/app/(overview)/payment-stream/page.tsxapps/web/src/components/modules/wallet/WalletConnectionGuard.tsx
| <ProtectedRoute description="Connect your Stellar wallet to create token distributions."> | ||
| <div | ||
| ref={pageRef} | ||
| className="h-screen mt-10 bg-black text-white overflow-y-auto scroll-smooth distribution-scrollbar" | ||
| > | ||
| <WalletConnectionGuard contextLabel="Connect your Stellar wallet to create token distributions."> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check ProtectedRoute implementation for additional functionality
ast-grep --pattern $'export default function ProtectedRoute($$$) {
$$$
}'
# Also check the full file for any additional logic
fd -t f 'ProtectedRoute.tsx' --exec cat {}Repository: Fundable-Protocol/stellar_client_os
Length of output: 4286
🏁 Script executed:
fd -type f -name '*WalletConnectionGuard*' --exec cat {}Repository: Fundable-Protocol/stellar_client_os
Length of output: 249
🏁 Script executed:
fd -t f 'WalletConnectionGuard' --exec cat {}Repository: Fundable-Protocol/stellar_client_os
Length of output: 1869
Remove redundant wallet connection guard.
Both ProtectedRoute (line 387) and WalletConnectionGuard (line 388) check useWallet().address and render a wallet connection prompt when disconnected. Since ProtectedRoute wraps WalletConnectionGuard, it will always intercept first, making WalletConnectionGuard unreachable when the wallet is disconnected.
Remove one of the wrappers. If migrating to WalletConnectionGuard, remove ProtectedRoute. If keeping ProtectedRoute for its customization options (title, description, actionLabel, container/card classes), remove WalletConnectionGuard from this page.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/`(overview)/distribution/page.tsx around lines 387 - 388,
The page nests two components that both guard on
useWallet().address—ProtectedRoute and WalletConnectionGuard—making one
redundant; remove the extra wrapper so only one guard remains. Decide which to
keep: if you want the customizable title/description/actionLabel/container/card
classes, keep ProtectedRoute and remove the WalletConnectionGuard wrapper around
the page content; if you prefer WalletConnectionGuard semantics, remove
ProtectedRoute and re-add any needed metadata/props
(title/description/actionLabel/container/card classes) to WalletConnectionGuard
or the page layout. Make the change by editing the JSX that currently wraps the
page (references: ProtectedRoute, WalletConnectionGuard, and
useWallet().address) so only the chosen guard remains.
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 `@apps/web/src/components/modules/history/HistoryTable.tsx`:
- Around line 94-95: The mobile card layout was removed from HistoryTable,
causing poor small-screen UX; restore responsive rendering so small screens show
the card-based stacked view and larger screens render the Table. Update the
HistoryTable component to conditionally render the previous card-style markup
(or a Card/List component) for screens below the md breakpoint and keep <Table>
for md and up—use existing Tailwind responsive classes or a small media-query
hook (e.g., useMediaQuery) to switch views, and ensure the container classes
around the card (rounded-xl border bg-zinc-900/50) are applied to the mobile
card variant so the mobile styling/spacing is preserved.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 231d40e5-2981-4244-bff2-1129d85287b3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
apps/web/src/components/modules/history/HistoryTable.tsx
| <div className="rounded-xl border border-zinc-800 bg-zinc-900/50 overflow-hidden"> | ||
| <Table> |
There was a problem hiding this comment.
Mobile UX regression: card-based layout removed.
The mobile-specific card view has been removed, leaving only the table layout. Tables typically provide a poor experience on small screens due to horizontal scrolling and cramped content. Consider preserving mobile-responsive behavior or confirming this regression is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/modules/history/HistoryTable.tsx` around lines 94 -
95, The mobile card layout was removed from HistoryTable, causing poor
small-screen UX; restore responsive rendering so small screens show the
card-based stacked view and larger screens render the Table. Update the
HistoryTable component to conditionally render the previous card-style markup
(or a Card/List component) for screens below the md breakpoint and keep <Table>
for md and up—use existing Tailwind responsive classes or a small media-query
hook (e.g., useMediaQuery) to switch views, and ensure the container classes
around the card (rounded-xl border bg-zinc-900/50) are applied to the mobile
card variant so the mobile styling/spacing is preserved.
|
Hi @devcarole Please resolve this merge conflicts. |
2 participants
Summary
Closes #4
This PR adds wallet connection guards to the Distribution and Payment Stream pages by extracting the existing pattern from the Offramp page into a shared WalletConnectionGuard component.
Changes
Behavior
Summary by CodeRabbit
New Features
Refactor