Skip to content

fix(chain-detail): run the token list under the floating bottom bar - #5985

Merged
johnnyluo merged 1 commit into
mainfrom
aminsato/5961_chain_tokens_under_bar
Sep 27, 2026
Merged

johnnyluo merged 1 commit into
mainfrom
aminsato/5961_chain_tokens_under_bar

Conversation

@aminsato

@aminsato aminsato commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #5961

Ethereum chain detail on a real vault: aEthUSDT and the row below it pass under the Wallet/DeFi pill and the camera button, and show through the fade instead of stopping at a card edge above a solid band.

Summary

  • Token list fills the screen. The chain token LazyColumn used to scroll inside a TopShineContainer card, with the whole column padded by LocalBottomNavigatorPadding, so the card ended 82dp above the bottom and clipped its last row. The list is now the outer scroller (fillMaxSize), and the navigator room is contentPadding, so rows pass under the bar and the last token can still scroll clear of the pill.
  • Card drawn per row. The first row gets the top corners and the top shine, the last row the bottom corners, and every row but the last a divider. The card always hugs its rows, so a single-token chain gets no empty band (that band is why the padding used to sit on the column). The QBTC promo banner is a list item.
  • Top scaffold padding only, the same as VaultAccountsScreen. The bottom system-bar inset otherwise stopped the list 24dp short of the screen edge.
  • Navigator fade matches iOS. BottomFadeEffect (transparent → 100% background) is replaced in BottomNavigatorOverlay by the iOS VultiTabBar.bottomGradient stops: solid from the bottom to halfway, 50% at 85%, clear at the top, all drawn at 0.7 opacity, so it is never solid. This also changes home's fade, on purpose: iOS home uses the same VultiTabBar gradient. The pill and camera button are not moved. BottomFadeEffect itself is untouched (TokenSelectionList still uses it).

Test plan

  • ./gradlew assembleDebug and :app:compileDebugAndroidTestKotlin
  • ChainTokensListUnderNavigatorTest (replaces ChainTokensListHeightTest): the list reaches the bottom of the window, and the last of 30 tokens scrolls fully above the pill
  • BottomNavigatorOverlayTest still passes
  • On device: open Ethereum with several tokens, scroll, and coins pass under the bar; at the end the last card closes above the pill with no solid band between them
  • Home: the fade behind the bar is lighter (iOS parity), with the pill and camera button where they were

Pre-existing and out of scope: once the header collapses, the Tokens tab row sits under the status bar (same on main).

Summary by CodeRabbit

  • Improvements

    • Chain token lists now extend behind the bottom navigator, with additional scroll space so the last token can clear the navigator.
    • Token rows are presented as a unified card with rounded ends and dividers; the Qbtc promo banner appears after the token list.
    • The floating claim button accounts for the bottom navigator, which now has a subtle fade behind it.
  • Tests

    • Added coverage for token-list behavior around the bottom navigator.

The token list scrolled inside a card that stopped short of the Wallet/DeFi
pill, clipping its last row, and the navigator's fade finished on a solid
band. The list now fills the screen with the navigator room as scroll padding
(the card is drawn per row so it still hugs its rows), and the fade mirrors
iOS VultiTabBar: never more than 70% opaque, so content shows through.
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: vultisig/vultisig-android/.coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: da055be0-c9be-4f3a-859e-4d59c0810ff6

📥 Commits

Reviewing files that changed from the base of the PR and between 4a590e3 and 1f915b2.

📒 Files selected for processing (5)
  • app/src/androidTest/java/com/vultisig/wallet/ui/screens/v2/chaintokens/ChainTokensListHeightTest.kt
  • app/src/androidTest/java/com/vultisig/wallet/ui/screens/v2/chaintokens/ChainTokensListUnderNavigatorTest.kt
  • app/src/debug/java/com/vultisig/wallet/debug/PreviewActivity.kt
  • app/src/main/java/com/vultisig/wallet/ui/screens/v2/chaintokens/ChainTokensScreen.kt
  • app/src/main/java/com/vultisig/wallet/ui/screens/v2/home/components/BottomNavigator.kt
💤 Files with no reviewable changes (1)
  • app/src/androidTest/java/com/vultisig/wallet/ui/screens/v2/chaintokens/ChainTokensListHeightTest.kt

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.


📝 Walkthrough

Walkthrough

The chain token list now extends behind the bottom navigator. Its bottom scroll padding lets the final token scroll above the navigator. The navigator fade and Android tests changed, and a debug preview now shows Ethereum tokens beneath the navigator.

Changes

Chain token list

Layer / File(s) Summary
List layout and token rows
app/src/main/java/com/vultisig/wallet/ui/screens/v2/chaintokens/ChainTokensScreen.kt
The list fills the available content area and uses top and bottom scroll padding, including bottom navigator padding. Token rows render as segmented card rows, and the Qbtc banner is a keyed list item.
Navigator fade
app/src/main/java/com/vultisig/wallet/ui/screens/v2/home/components/BottomNavigator.kt
The visible navigator uses a vertical gradient based on the primary background color, replacing BottomFadeEffect.
Navigator tests and debug preview
app/src/androidTest/java/com/vultisig/wallet/ui/screens/v2/chaintokens/ChainTokensListHeightTest.kt, app/src/androidTest/java/com/vultisig/wallet/ui/screens/v2/chaintokens/ChainTokensListUnderNavigatorTest.kt, app/src/debug/java/com/vultisig/wallet/debug/PreviewActivity.kt
The former card-height test was removed. New Compose tests check list bounds and final-token scrolling. A debug preview displays Ethereum token rows behind the visible navigator.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 1f915

The token list changes have no identified merge-blocking issue; the navigator fade’s appearance on other screens remains unverified.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR satisfies the chain-list objectives in [#5961]. ChainTokensScreen moves LocalBottomNavigatorPadding into LazyColumn.contentPadding, draws card segments per row, and adds tests for list re… Keep the home-screen navigator behavior unchanged while retaining the chain-detail list-under-navigator behavior. Add or complete a regression test for the home-screen fade.
Out of Scope Changes check ⚠️ Warning The BottomNavigatorOverlay diff replaces the shared BottomFadeEffect with a new gradient. The change affects the home screen, and the PR summary confirms this side effect. The linked issue scopes … Scope the transparent navigator fade to chain detail, or preserve the existing home-screen fade when the overlay is used on the home screen. Verify both paths with automated tests.
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: allowing the chain-detail token list to run beneath the floating bottom bar.
Full details: Linked Issues check

Explanation

The PR satisfies the chain-list objectives in [#5961]. ChainTokensScreen moves LocalBottomNavigatorPadding into LazyColumn.contentPadding, draws card segments per row, and adds tests for list reach and last-token clearance. The PR also changes the shared BottomNavigatorOverlay fade. The PR summary reports a resulting home-screen fade change, while [#5961] explicitly requires no home-screen navigator change. This requirement is not satisfied.

Full details: Out of Scope Changes check

Explanation

The BottomNavigatorOverlay diff replaces the shared BottomFadeEffect with a new gradient. The change affects the home screen, and the PR summary confirms this side effect. The linked issue scopes the fix to the chain detail and says not to change the home-screen navigator. This is a demonstrated out-of-scope behavior change.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@johnnyluo johnnyluo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johnnyluo
johnnyluo added this pull request to the merge queue Sep 27, 2026
Merged via the queue into main with commit d76bfb8 Sep 27, 2026
4 checks passed
@johnnyluo
johnnyluo deleted the aminsato/5961_chain_tokens_under_bar branch September 27, 2026 22:54
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.

[Fix] Chain token list should run under a transparent bottom bar [Chain detail]

2 participants