Skip to content

fix: avoid animated switch to auto tab - #1308

Open
jvsena42 wants to merge 10 commits into
masterfrom
fix/876-receive-auto-tab-switch
Open

jvsena42 wants to merge 10 commits into
masterfrom
fix/876-receive-auto-tab-switch

Conversation

@jvsena42

@jvsena42 jvsena42 commented Sep 17, 2026

Copy link
Copy Markdown
Member

Fixes #876
Related to #1275

This PR:

  1. Stops the Receive sheet from overriding a tab the user picked when the Auto tab becomes available
  2. Replaces the animated slide to Auto with an instant jump when the user has not touched the tabs
  3. Clears a leftover Edit Invoice amount before the Receive sheet opens, so it no longer starts on Savings and then jumps to Auto

Description

  • Tracks whether the user has picked a tab by tap or by dragging the QR pager, and skips the switch to Auto once they have, because the switch used to fire whenever Lightning became ready and took the user off the tab they chose.
  • Skips the switch when an initial tab was requested or Auto is already selected, and moves the decision into a small pure function so it can be unit tested.
  • Uses an instant scroll instead of an animated one when the sheet still moves to Auto after opening, so it no longer visibly slides across tabs.
  • Clears the Receive amount when the Receive sheet is requested, before it is shown, because an amount above inbound capacity left over from an earlier Edit Invoice session made the next open start on Savings and then jump to Auto once the receive state refreshed.

On a warm open with the node running, master already starts on Auto from the first frame on the emulator; the slide on open did not reproduce in that case. On a cold start, master opens on Savings while the node starts, then moves to Auto and overrides a tab the user picked in the meantime. That override is the part reproduced and fixed here.

Out of Scope

  • ReceiveQrScreen.kt: when the sheet is opened while the Lightning node is still starting, it still opens on Savings (no Auto tab yet) and then jumps to Auto once Lightning is ready. The jump is now instant, but the Auto QR needs a bolt11 from the running node, so it cannot be shown from the first frame.
  • ReceiveQrScreen.kt: the user-selection flag records that the tabs were touched, not which tab was picked. If the user stays on Auto, Lightning drops (node restart) and comes back, the sheet stays on Savings instead of returning to Auto.
  • AppViewModel.kt: only the leftover amount is cleared on open; the old bip21/bolt11 strings are replaced when the receive state refresh finishes, as before.
  • The CJIT switch to Spending and the other tab effects are unchanged.
  • iOS parity: not checked in this PR; bitkit-ios may need the same guard if its Receive sheet auto-switches to Auto.
  • feat: add blockstream jade hardware wallet support #1231 renames the Trezor tab in ReceiveQrScreen.kt and fix: polish wallet ui #1270 edits the tab row there; expect a small text conflict with whichever lands first.

Design

Preview

Recorded on a Pixel 9 emulator with a paired hardware wallet (hence the extra Trezor tab). Differences from Figma that already exist on master: the Trezor tab, no contacts icon in the top bar, and no "Scan or tap to pay" hint.

Figma Before (master) After
Figma Receive Auto Before: user picked Savings, moved to Auto After: user picked Savings, stays on Savings

Before (master): cold start, tap Trezor then Savings before the Auto tab exists; when Auto appears the sheet moves to it.

876-master-coldstart-userpick.mp4

After: same flow; Savings stays selected after the Auto tab appears.

876-after-coldstart-userpick.mp4

Before (master): cold start, Receive opened without touching the tabs; opens on Savings, then moves to Auto once Lightning is ready.

876-master-coldstart.mp4

After: same flow; the move to Auto is an instant jump with no slide frames.

876-after-coldstart.mp4

After: five warm opens, each on Auto from the first frame.

876-after-opens.mp4

After: warm open on Auto, swipe to Savings; Savings stays selected.

876-after-swipe.mp4

After: reopening Receive after a $3,000 Edit Invoice amount (above inbound capacity) opens directly on Auto.

876-after-stale-amount.mp4

QA Notes

Manual Tests

Needs a wallet with a usable spending channel.

  • 1. Node running → Home → Receive: Auto tab selected on the first frame, no slide.
  • 2. Receive → swipe QR to Savings → wait a few seconds: Savings stays selected.
  • 3a. Force-stop app → relaunch → Receive right away (no Auto tab yet) → tap Savings → wait until the Auto tab appears: Savings stays selected.
    • 3b. Force-stop app → relaunch → Receive right away without touching tabs → wait until the Auto tab appears: sheet jumps to Auto without sliding.
  • 4. Receive → Edit → amount above inbound LN capacity → Continue → QR Code (no Auto tab) → close → Receive again: opens on Auto.
  • 5. regression: No channels → Receive → Edit → amount → CJIT flow: still switches to Spending with the CJIT invoice.

A journey covering tests 1 to 4 is committed at journeys/receive/receive-auto-tab-selection.xml;
the same patch inline:

receive-auto-tab-selection.xml
diff --git a/journeys/receive/receive-auto-tab-selection.xml b/journeys/receive/receive-auto-tab-selection.xml
new file mode 100644
index 000000000..c70f76736
--- /dev/null
+++ b/journeys/receive/receive-auto-tab-selection.xml
@@ -0,0 +1,87 @@
+<journey name="receive auto tab selection">
+  <description>
+    Verifies that the Receive sheet opens on the Auto tab without sliding to it, that a tab the user
+    picks is not overridden by the Auto tab, and that a leftover invoice amount from an earlier Edit
+    Invoice session does not open the sheet on Savings. Requires a wallet with a usable spending
+    channel so the Auto tab is available once the Lightning node is running. The selected tab is
+    shown only by its underline, which `android layout` does not expose, so tab assertions need a
+    screenshot; the Auto tab's presence can be read from the testTag "Tab-auto". The underline itself
+    crossfades over 200 ms even when the switch is instant, so the two steps that check for an
+    animation read the QR pager out of a screen recording instead of the underline. Each of those
+    steps first requires the recording to contain the frames it reasons about; a recording that ended
+    before the node started, or before the sheet opened, is a rerun with a longer `--time-limit`, not
+    a pass.
+  </description>
+  <actions>
+    <action>
+      Launch the Bitkit app and go to the wallet home screen
+    </action>
+    <action>
+      Tap the Receive button (testTag "Receive"), take a screenshot within one second, and verify the
+      Auto tab (testTag "Tab-auto") is selected on the first frame of the sheet
+    </action>
+    <action>
+      Swipe the QR code area from left to right once and verify the Savings tab (testTag
+      "Tab-savings") is selected
+    </action>
+    <action>
+      Wait 3 seconds, take a screenshot and verify the Savings tab is still selected
+    </action>
+    <action>
+      Close the sheet, then run `adb shell am force-stop to.bitkit.dev` and launch the app again
+    </action>
+    <action>
+      As soon as the home screen shows, tap the Receive button and verify the tab row has no Auto tab
+      (testTag "Tab-auto" absent) while the Lightning node starts
+    </action>
+    <action>
+      Tap the Trezor tab (testTag "Tab-trezor") if present, then tap the Savings tab (testTag
+      "Tab-savings")
+    </action>
+    <action>
+      Wait until the Auto tab (testTag "Tab-auto") appears, wait 5 more seconds, take a screenshot and
+      verify the Savings tab is still selected
+    </action>
+    <action>
+      Close the sheet, force-stop and relaunch the app, and start `adb shell screenrecord
+      --time-limit 20 /sdcard/receive-auto.mp4`. As soon as the home screen shows, tap the Receive
+      button without touching the tabs and verify the Auto tab (testTag "Tab-auto") is absent; do not
+      wait for the node, the tab row must still be Savings-first when the sheet opens
+    </action>
+    <action>
+      Wait until the Auto tab (testTag "Tab-auto") appears, and check that it appeared while the
+      recording was still running. The node has to finish `lightningService.start` and load its
+      channels before the Auto tab exists, which on a cold process can outlast the 20 second window;
+      if the Auto tab is still absent when the recording ends, the jump happened off-tape and the run
+      holds no evidence, so repeat the previous step with a longer `--time-limit` rather than reading
+      the empty recording as a pass
+    </action>
+    <action>
+      Let the recording finish, pull it (`adb pull /sdcard/receive-auto.mp4`), extract frames (`ffmpeg
+      -i receive-auto.mp4 -vf fps=30 frames/%03d.png`) and verify first that the recording contains
+      the jump at all: a frame showing the Savings page centred in the QR pager, followed by a later
+      frame showing the Auto page centred. If either frame is missing, the step did not capture the
+      switch and has to be rerun; it is not a pass. Then verify that no frame between those two shows
+      the QR pager part-way between the Savings and the Auto page: the sheet jumps to Auto in one
+      frame instead of sliding
+    </action>
+    <action>
+      Tap "Edit" (testTag "SpecifyInvoiceButton"), tap the amount field (testTag
+      "ReceiveNumberPadTextField"), enter an amount above the inbound Lightning capacity (for example
+      N3 N0 N0 N0 in USD), tap Continue (testTag "ReceiveNumberPadSubmit") and then "QR Code" (testTag
+      "ShowQrReceive"); verify the Auto tab (testTag "Tab-auto") is absent
+    </action>
+    <action>
+      Start `adb shell screenrecord --time-limit 20 /sdcard/receive-reopen.mp4`, close the sheet and
+      tap the Receive button again, then verify the Auto tab (testTag "Tab-auto") is present
+    </action>
+    <action>
+      Let the recording finish, pull it (`adb pull /sdcard/receive-reopen.mp4`), extract frames
+      (`ffmpeg -i receive-reopen.mp4 -vf fps=30 frames/%03d.png`) and verify that the recording holds
+      frames in which the sheet is visible — if it holds none, rerun the step, it is not a pass — then
+      that the first such frame already shows the Auto tab in the tab row with the selected underline,
+      and that no frame shows Savings selected. A sheet that opens on Savings and only then gains the
+      Auto tab means the leftover invoice amount was not cleared
+    </action>
+  </actions>
+</journey>

Automated Checks

  • Unit tests added: ReceiveAutoTabSwitchTest.kt covers when the sheet should switch to Auto (initial tab, user selection, Auto already selected, Lightning availability, CJIT invoice).
  • Unit tests added: AppViewModelSendFlowTest.kt checks that opening Receive clears the leftover amount before the sheet is set, and that opening another sheet keeps it.
  • Compose test added: ReceiveAutoTabSelectionTest.kt renders ReceiveQrScreen, flips Lightning to ready, and checks the tab row wiring: a tab picked by tap survives the Auto tab appearing, and with no tap the sheet lands on Auto.
  • Local: just compile, just test, just lint and compileDevDebugAndroidTestKotlin passed; the instrumented test itself runs in the UI Tests workflow.

jvsena42 and others added 2 commits September 17, 2026 07:50
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jvsena42 jvsena42 self-assigned this Sep 17, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

The implementation appears safe to merge, with a non-blocking gap in automated coverage for the Compose interaction behavior at the center of the fix.

Findings

  1. P2 Tests Miss UI Wiring

Summary

This PR changes Receive-sheet initialization and tab-selection behavior so a user-selected mode is not replaced when Auto becomes available.

  • Records tab-row taps and QR-pager drags before deciding whether to switch to Auto.
  • Replaces the animated Auto transition with an immediate list jump.
  • Clears a stale receive amount before presenting a new Receive sheet.
  • Adds predicate and view-model tests, although the Compose interaction wiring remains uncovered.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Receive requested] --> B[Clear stale receive amount]
  B --> C[Compose available receive tabs]
  C --> D{Auto available?}
  D -- No --> E[Show fallback receive tab]
  D -- Yes --> F{Initial tab requested or user touched tabs?}
  F -- Yes --> G[Preserve selected tab]
  F -- No --> H[Jump instantly to Auto]
Loading

Reviews (1) · Last reviewed commit: "fix: clear stale receive amount before o..."

@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 0877e1d (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Approved — no HIGH/MEDIUM findings at head fbc1dc281. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Traced the tab state machine in ReceiveQrScreen.kt at the PR head (effects at lines 194, 213, 221, 232, 239, 255) for cold open (no Auto tab yet), warm open (Auto from the first frame), tap and drag before Auto appears, Auto appearing with a Trezor tab in front (LazyRow keys by tab name so the Savings index is preserved and the jump targets the right index), channel drop and return, CJIT confirm, and return from Edit Invoice or the Paykit contact picker (fresh composition, initialTab set by ReceiveInvoiceEditState so the auto-switch is skipped exactly as on master). hasUserSelectedTab is only set by the tab row click (CustomTabRowWithSpacing invokes onTabChange solely from clickableAlpha) and by DragInteraction.Start on the LazyRow interaction source, which programmatic scrollToItem/animateScrollToItem never emit, so the guard cannot be tripped by the screen's own scrolls. The early return on firstVisibleItemIndex == autoIndex cannot suppress a needed jump: the only way to sit on a non-Auto page equal to the new Auto index without a user pick is a CJIT invoice, which hides the Auto tab. invoicesByTab is unchanged, so every page still renders the URI for its own tab; the transient on-chain-only URI under Auto while the old bip21 string waits for refreshBip21 (behind syncState and the geoblock check in WalletViewModel.refreshReceiveState) is the limitation the author already listed as out of scope and existed on master on the Savings page. The showSheet change in AppViewModel.kt:4727 is an in-memory StateFlow update after the transition delay and before the sheet is published; no caller sets bip21AmountSats before showSheet(Sheet.Receive) (all writers are inside the open sheet), cancel by a competing showSheet, hideSheet, or process death leaves nothing half-applied, and refreshBip21 clears the same field on open anyway, so Paykit request routes and the ReceiveRoute.Amount funding entry are unaffected.

@jvsena42
jvsena42 requested a review from ovitrif September 18, 2026 13:26
jvsena42 and others added 3 commits September 18, 2026 10:33
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…o-tab-switch

# Conflicts:
#	journeys/README.md
…o-tab-switch

# Conflicts:
#	journeys/README.md

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Approved — no blocking HIGH/MEDIUM findings at head 007e933a6. (Posted as a comment because GitHub does not allow approving your own PR.)

Inline: MEDIUM (journey, non-blocking) — Instant-jump step waits for the node first, so Savings never shows and the jump cannot be observed; LOW (journey, non-blocking) — First-frame checks rely on one screenshot and pass if the leftover-amount fix is reverted

Checked: Round 4 (7ae8457, new journey): Reviewed the journey commit 7ae8457 only. Every testTag it names exists at head: Receive (TabBar.kt:156), Tab-auto/Tab-savings/Tab-trezor (CustomTabRowWithSpacing.kt:62, Tab-${tab.name.lowercase()}), SpecifyInvoiceButton, ReceiveNumberPadTextField, ReceiveNumberPadSubmit, ShowQrReceive and the N keys. The Edit Invoice route in the last two steps matches the code: source AUTO always yields ReceiveAdditionalLiquidityAction.None (ReceiveLiquidityDecision.kt:42), so Show QR updates the invoice and pops back to the QR screen with no Auto tab, and reopening hits the new bip21AmountSats clear in AppViewModel.showSheet. The cold-start window with no Auto tab is real: channels are only populated by syncState() after lightningService.start returns (LightningRepo.kt:390-395). The README index row and the cross-platform not-ported row are in place.

Comment thread journeys/receive/receive-auto-tab-selection.xml Outdated
Comment thread journeys/receive/receive-auto-tab-selection.xml Outdated
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Approved — no blocking HIGH/MEDIUM findings at head 049c57fdf. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Follow-up on 049c57f: the journey MEDIUM and the first-frame LOW are fixed. One remaining LOW (nothing asserts the recording actually contains the jump) is a reply on the existing thread. Reviewed 049c57f only. The cold-start step no longer waits for the node: it taps Receive as soon as home shows and asserts Tab-auto absent, which is the reachable window before LightningRepo.start sets Running and runs syncState(). AUTO is inserted after SAVINGS in visibleTabs (ReceiveQrScreen.kt:132-143), so Savings stays index 0 and the jump targets index 1; on master animateScrollToItem leaves intermediate pager frames that the ffmpeg frame check catches. The reopen step now records and requires the first visible sheet frame to already show Auto selected and no Savings frame, which fails if the bip21AmountSats clear in showSheet is reverted; the underline has no initial crossfade on a first composition, so that check is sound.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ovi-reviewer[bot]

This comment was marked as resolved.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Approved — no blocking HIGH/MEDIUM findings at head c5b492353. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Follow-up: Follow-up on c5b4923, which answers the LOW that nothing required the recording to contain the jump. The cold-start step is now split into two. The first requires Tab-auto to appear while the recording is still running, and tells the runner to rerun with a longer --time-limit otherwise. The second requires the extracted frames to hold a Savings-centred frame followed by a later Auto-centred frame before it runs the part-way check, and treats a missing frame as a rerun, not a pass. This is the positive requirement asked for. It matches the code: AUTO is inserted after SAVINGS, and the scrollToItem(autoIndex) jump in ReceiveQrScreen.kt:239-251 fires only after the node is ready. The reopen step now also rejects a recording with no sheet frames. Journey-only commit.

@ovitrif

ovitrif commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

@jvsena42 conflicts, I am approving

ovitrif

This comment was marked as resolved.

ovi-reviewer[bot]

This comment was marked as resolved.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ✅ Approve


Reaudit: diff 1 file.

Findings:
N/A

Audit:
Already done in comment.

QA: emu-1 redroid container on Android 15, dev build 2.4.1 (188)

  1. passed: Auto was selected on the first Receive frame; no pager slide appeared.

    Test 1:
    Node running → Home → Receive: Auto tab selected on the first frame, no slide.
    1.mp4
  2. passed: Savings remained selected five seconds after the swipe.

    Test 2:
    Receive → swipe QR to Savings → wait a few seconds: Savings stays selected.
    2.mp4
  3. passed: Savings remained selected when Auto appeared after a user selection.

    Test 3:
    Force-stop app → relaunch → Receive right away (no Auto tab yet) → tap Savings → wait until the Auto tab…
    3.mp4
  4. passed: The untouched flow jumped from Savings to Auto in one frame, without an intermediate pager position.

    Test 4:
    Force-stop app → relaunch → Receive right away without touching tabs → wait until the Auto tab appears: sheet…
    4.mp4
  5. passed: An above-capacity invoice reopened Receive directly on Auto, never Savings.

    Test 5:
    Receive → Edit → amount above inbound LN capacity → Continue → QR Code (no Auto tab) → close → Receive again:…
    5.mp4
  6. passed: Zero-channel CJIT returned to Receive with Spending selected and a Lightning invoice.

    Test 6:
    `regression:` No channels → Receive → Edit → amount → CJIT flow: still switches to Spending with the CJIT…
    6.mp4

Coverage:
QA: 6 of 6 manual tests passed


Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)

@jvsena42
jvsena42 requested a review from ovitrif September 18, 2026 20:19
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.

Tab always transitions Spending → Auto when there are LN channel every time Receive Sheet opens

2 participants