Skip to content

fix(wallet): make every wallet card reachable on home - #2682

Merged
ethicnology merged 1 commit into
mainfrom
fix/home-hidden-wallet-card
Aug 17, 2026
Merged

fix(wallet): make every wallet card reachable on home#2682
ethicnology merged 1 commit into
mainfrom
fix/home-hidden-wallet-card

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

The last wallet card was hidden behind the Receive/Send buttons and couldn't be scrolled into view. The list now reserves space for them.

Screen.Recording.2026-08-17.at.12.35.51.PM.mov

@wired-pasteque wired-pasteque self-assigned this Aug 17, 2026
@ethicnology
ethicnology changed the base branch from develop to main August 17, 2026 17:08
@claude

claude Bot commented Aug 17, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 6m 46s —— View job


Code review — PR #2682

  • Gather context (diff, surrounding code, no root CLAUDE.md found)
  • Review changes for correctness bugs, security issues, regressions
  • Post inline review comments — not possible: the workflow token denies gh api .../pulls/reviews, so findings are posted here with file/line references instead (see FAQ)
  • Post summary

The core fix is sound: bottomInset in BBPullableBody correctly adds scroll extent equal to the pinned bar (52 + 16×2 matches BBButton.big's default height of 52 and the bar's vertical padding, plus safe-area bottom), short content stays unscrollable because SliverFillRemaining absorbs the inset, and the new widget test covers the regression well. No security issues. But the PR also changes two things beyond the described fix, and one of them looks like a real regression.

Likely regression

1. The price chart loses ~78dp of drawing heightlib/features/wallet/ui/widgets/wallet_home_top_section.dart:24

The dark area shrank from 264 + 78 = 342 to 264. When the chart is toggled on (from the wallet home app bar), PriceChartWidget fills that area, and its fixed parts — Gap(72) + price display (~110dp) + Gap(16) + EdgeInsets.only(bottom: 40) — consume ~235dp. The Expanded chart canvas absorbs the entire 78dp loss, dropping from roughly ~105dp to ~30dp tall; with a modest system text-scale bump it can reach 0 and the chart line disappears. The demo video only shows the balance view — please check the chart toggle on this branch. If the shorter area is intended, PriceChartWidget's internal spacing (72dp top gap / 40dp bottom padding in lib/features/bitcoin_price/ui/price_chart_widget.dart:123-128) needs to shrink with it. Fix this →

Worth confirming

2. The top section is now permanently pinnedlib/features/wallet/ui/screens/wallet_home_screen.dart:130

Switching SliverToBoxAdapterPinnedHeaderSliver means the 340dp header never scrolls away. On small devices (568dp-tall iPhone SE class) that leaves only ~150dp of visible scroll window between the pinned header and the bottom bar for announcements + warnings + banners + all wallet cards. Everything stays reachable thanks to bottomInset, so this isn't a bug — but it's a substantial UX change not mentioned in the PR description; worth confirming it's intended and acceptable on small screens.

3. Balance layout has less slack under large text scalelib/features/wallet/ui/widgets/wallet_home_top_section.dart:121

Replacing MainAxisAlignment.center with a fixed Gap(123) in a container that also shrank by 78dp: with an unconfirmed incoming balance visible (~+50dp of extra rows) the column sits at ~255dp of the 264dp available, so a small text-scale increase overflows the SizedBox (striped overflow in debug, clipped in release). The old centered layout redistributed this automatically.

Nits

  • lib/features/wallet/ui/screens/wallet_home_screen.dart:24_bottomBarHeight = 52.0 + 16.0 * 2 silently duplicates BBButton.big's default height and the padding around WalletBottomButtons. If either changes, the inset under-reserves and the hidden-last-card bug quietly returns; a shared constant would prevent drift.
  • The bottom 76dp of the pinned section is transparent in the 13px strips beside the ActionCard, so scrolled content can show through there and taps in those strips pass through to content hidden under the header. Wallet cards share the 13px padding so this is likely invisible in practice — just flagging it.
  • The WalletCards gap change (trailing → between-items) also removes the 8dp trailing gap on the pay/sell wallet-selection screens — cosmetic and probably fine, but it does touch those screens.

@ethicnology
ethicnology merged commit 02d341a into main Aug 17, 2026
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants