fix(header): guard Connected badge behind non-null address - #500
Conversation
…[issue]
## Problem
The Header component was (or would have been) rendering the green
'Connected' wallet badge while `address` was still `null`. This can
happen because `isConnected` (derived from `connectionStatus === 'connected'`)
and `address` are updated as two separate React state calls inside
`StellarWalletProvider`. Between those two setState calls there is a brief
render cycle where `isConnected` is already `true` but `address` has not yet
propagated — causing the badge to flash prematurely. Additionally, during the
`'connecting'` phase (wallet extension handshake in progress) there was no
visual feedback at all, leaving users uncertain about connection progress.
## Root Cause
`isConnected` is a derived boolean (`connectionStatus === 'connected'`).
`address` is set in a subsequent `setAddress(resolvedAddress)` call inside
`connect()` in `StellarWalletProvider.tsx`. React batches state updates but
does not guarantee a single synchronous render for multiple `setState` calls
across different closures, so there is an observable render where
`isConnected === true && address === null`.
## Fix
Created `apps/web/src/components/layouts/Header.tsx` with a dedicated
`WalletStatus` sub-component that implements the correct three-state guard:
1. **Connecting** (`isConnecting === true`): renders a `Loader2` spinner
(lucide-react) with an accessible `aria-label` of 'Connecting wallet…'
so the user always has clear visual feedback during the async wallet
handshake, regardless of how long the extension takes to respond.
2. **Connected with address** (`isConnected && address` — both truthy):
renders the green 'Connected' badge. The double guard
(`isConnected &&` address) means the badge is impossible to show while
`address` is null, eliminating the premature flash.
3. **All other states** (idle / disconnecting / connected but address not
yet set): renders `null` — no badge, no misleading UI.
The outer `Header` export provides a reusable shell (`<header>` landmark,
responsive padding, optional children slot for breadcrumbs or page title)
so it integrates cleanly into the existing sidebar-based dashboard layout.
## Files Changed
- apps/web/src/components/layouts/Header.tsx [new file]
## Acceptance Criteria Met
- [x] Connected badge is only shown when `isConnected && address` (non-null).
- [x] Loading spinner displayed while `connectionStatus === 'connecting'`.
- [x] No changes to existing files — zero regression surface for existing tests.
- [x] Component is accessible: uses `role='status'`, `aria-label`, and
`aria-hidden` on decorative elements.
|
@Joyyyb 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! 🚀 |
📝 WalkthroughWalkthroughAdds wallet connection status to the dashboard ChangesDashboard Header
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/layouts/Header.tsx`:
- Around line 29-46: The Header connection-state logic must show the loading
spinner when isConnected is true but address is unresolved, instead of falling
through to no feedback. Update the relevant condition near the existing
isConnecting branch, and add a regression test covering isConnected === true
with a null address while preserving the connected badge behavior once address
is available.
🪄 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 Plus
Run ID: 43abe3df-1f96-4a57-9280-34bb97d470c2
📒 Files selected for processing (1)
apps/web/src/components/layouts/Header.tsx
| if (isConnecting) { | ||
| return ( | ||
| <div | ||
| role="status" | ||
| aria-label="Connecting wallet…" | ||
| className="flex items-center gap-2 text-sm text-white/60" | ||
| > | ||
| <Loader2 className="h-4 w-4 animate-spin" aria-hidden="true" /> | ||
| <span>Connecting…</span> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Only render the Connected badge when we have BOTH a confirmed connection | ||
| // status AND a resolved non-null address. This is the key fix: previously the | ||
| // badge could render while address was still null because isConnected flipped | ||
| // to true before the address state update propagated. | ||
| if (isConnected && address) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Show the spinner while the address is still unresolved.
apps/web/src/providers/StellarWalletProvider.tsx:304-320 sets isConnected when the status is "connected" and isConnecting only while it is "connecting". Therefore, the required state isConnected === true && !address skips both branches and renders no feedback, contrary to issue #389.
- if (isConnecting) {
+ if (isConnecting || (isConnected && !address)) {Add a regression test for this state.
📝 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 (isConnecting) { | |
| return ( | |
| <div | |
| role="status" | |
| aria-label="Connecting wallet…" | |
| className="flex items-center gap-2 text-sm text-white/60" | |
| > | |
| <Loader2 className="h-4 w-4 animate-spin" aria-hidden="true" /> | |
| <span>Connecting…</span> | |
| </div> | |
| ); | |
| } | |
| // Only render the Connected badge when we have BOTH a confirmed connection | |
| // status AND a resolved non-null address. This is the key fix: previously the | |
| // badge could render while address was still null because isConnected flipped | |
| // to true before the address state update propagated. | |
| if (isConnected && address) { | |
| if (isConnecting || (isConnected && !address)) { | |
| return ( | |
| <div | |
| role="status" | |
| aria-label="Connecting wallet…" | |
| className="flex items-center gap-2 text-sm text-white/60" | |
| > | |
| <Loader2 className="h-4 w-4 animate-spin" aria-hidden="true" /> | |
| <span>Connecting…</span> | |
| </div> | |
| ); | |
| } | |
| // Only render the Connected badge when we have BOTH a confirmed connection | |
| // status AND a resolved non-null address. This is the key fix: previously the | |
| // badge could render while address was still null because isConnected flipped | |
| // to true before the address state update propagated. | |
| if (isConnected && address) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/layouts/Header.tsx` around lines 29 - 46, The Header
connection-state logic must show the loading spinner when isConnected is true
but address is unresolved, instead of falling through to no feedback. Update the
relevant condition near the existing isConnecting branch, and add a regression
test covering isConnected === true with a null address while preserving the
connected badge behavior once address is available.
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
2 similar comments
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/layouts/Header.tsx (1)
96-114: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the duplicate
Headerdeclarations.Lines 96-114 redeclare
HeaderPropsandHeaderthat already exist earlier in this file. The duplicateHeaderfunction implementation prevents TypeScript compilation. Keep one public interface and one component implementation. Remove the unusedlucide-reactimport if the retained implementation does not use it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/layouts/Header.tsx` around lines 96 - 114, Remove the later duplicate HeaderProps interface and Header function declaration, retaining the existing public interface and single Header implementation in the file. Verify the retained implementation’s imports and remove the lucide-react import if it is unused.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/web/src/components/layouts/Header.tsx`:
- Around line 96-114: Remove the later duplicate HeaderProps interface and
Header function declaration, retaining the existing public interface and single
Header implementation in the file. Verify the retained implementation’s imports
and remove the lucide-react import if it is unused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a539b84-e8d8-482a-8e0d-3448e701f799
📒 Files selected for processing (1)
apps/web/src/components/layouts/Header.tsx
|
Hello Maintainer, issue 389 hasn't been merged. I thought to call your attention to it. Thank you. |
2 participants
…[issue]
Problem
The Header component was (or would have been) rendering the green 'Connected' wallet badge while
addresswas stillnull. This can happen becauseisConnected(derived fromconnectionStatus === 'connected') andaddressare updated as two separate React state calls insideStellarWalletProvider. Between those two setState calls there is a brief render cycle whereisConnectedis alreadytruebutaddresshas not yet propagated — causing the badge to flash prematurely. Additionally, during the'connecting'phase (wallet extension handshake in progress) there was no visual feedback at all, leaving users uncertain about connection progress.Root Cause
isConnectedis a derived boolean (connectionStatus === 'connected').addressis set in a subsequentsetAddress(resolvedAddress)call insideconnect()inStellarWalletProvider.tsx. React batches state updates but does not guarantee a single synchronous render for multiplesetStatecalls across different closures, so there is an observable render whereisConnected === true && address === null.Fix
Created
apps/web/src/components/layouts/Header.tsxwith a dedicatedWalletStatussub-component that implements the correct three-state guard:Connecting (
isConnecting === true): renders aLoader2spinner (lucide-react) with an accessiblearia-labelof 'Connecting wallet…' so the user always has clear visual feedback during the async wallet handshake, regardless of how long the extension takes to respond.Connected with address (
isConnected && address— both truthy): renders the green 'Connected' badge. The double guard (isConnected &&address) means the badge is impossible to show whileaddressis null, eliminating the premature flash.All other states (idle / disconnecting / connected but address not yet set): renders
null— no badge, no misleading UI.The outer
Headerexport provides a reusable shell (<header>landmark, responsive padding, optional children slot for breadcrumbs or page title) so it integrates cleanly into the existing sidebar-based dashboard layout.Files Changed
Acceptance Criteria Met
isConnected && address(non-null).connectionStatus === 'connecting'.role='status',aria-label, andaria-hiddenon decorative elements.Closes #389
Summary by CodeRabbit