Conversation
|
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
jvsena42
left a comment
There was a problem hiding this comment.
No findings. The title says refactor, but this PR touches the PIN prompt, force transfer, update and backup sheets, so I reviewed it as a behaviour change.
Old and new behaviour compared on each migrated sheet (swipe-dismiss and Back included):
- Backup / high balance / update. The same actions and the same persisted keys (
backupWarningIgnoredMillis,balanceWarningTimes/balanceWarningIgnoredMillis). Critical updates still route toCriticalUpdateScreen, becauseAppUpdateTimedSheet.shouldShow()is false when critical. - QuickPay timed sheet. The new Later sets
quickPayIntroSeenand dismisses, which is the same flag a swipe wrote before. No state lost. - Background payments. The timed sheet keeps Later/Enable. The full-screen intro is Enable-only. That matches iOS
NotificationsIntro, and the user still reaches the settings screen when they decline the permission. - PIN prompt.
showLaterButtonstill gates Later (only Home passestrue), so there is no new skip path. The duplicateSecureWalletContinuetag is gone, and the e2e taps still resolve. - Force transfer.
PrimaryButtondebounce plusenabled && !isLoadingare unchanged. Dismiss never callsforceTransfer. - Scheduling. Nothing under
utils/timedsheets/or the AppViewModel timed-sheet code changed.docs/timed-sheets.mdmatches the code: the 2s resume delay, priority order, intervals, thresholds and persisted keys. - Strings. The new
settings__quickpay__sheet__descriptionis in base and all 15 locales with no format args. - Test tags. Referenced tags are preserved. The renamed root tags have no references in androidTest, journeys or bitkit-e2e-tests.
Figma, Handoff v62: Backup Intro, BackgroundPayments Intro, the QuickPay and Background Payments sheets, High Wallet Balance, Update, Force close and the PIN prompt all match. Title, accent, copy, button order and variant, artwork, 32dp sides and the 32/8/32 rhythm are consistent. One question for design, not this PR: the full-screen BackgroundPayments Intro frame accents the title in Blue, while the timed sheet frame and this code use Purple.
There was a problem hiding this comment.
Verdict: ✅ Approve
Review: diff 30 files.
Findings:
3 inline (non-blocking)
Audit:
Skipped - only auth touched (score 2, threshold 3).
QA: emu-1 redroid container on Android 15, dev build 188
-
✅ passed: Backup Intro artwork, copy, Later, and Back Up Now fit without overlap; Later returned Home.
-
✅ passed: Later dismissed QuickPay Intro, and the sheet stayed dismissed after restarting the app.
-
✅ passed: Learn More opened QuickPay Settings.
-
✅ passed: The full-screen Background Payments intro showed only Enable; the timed sheet showed Later and Enable.
-
✅ passed: The Secure Wallet prompt retained Later, and Continue opened Choose 4-Digit PIN.
-
✅ passed: A real 10,000-sat on-chain send showed a fully visible checkmark matching the intro artwork width.
-
✅ passed: QuickPay Intro showed the updated description in French.
Tip
Test 1 worth a journey:
- Fund the wallet with a confirmed transaction
- Return to Home and wait for Backup Intro
- Verify the artwork, copy, Later, and Back Up fit without overlap
- Tap Later and verify Home returns
Test 2 worth a journey:
- Fund the Spending balance and return to Home
- Wait for the QuickPay timed sheet
- Tap Later
- Restart the app and wait on Home
- Verify QuickPay does not return
Test 3 worth a journey:
- Fund the Spending balance and return to Home
- Wait for the QuickPay timed intro
- Tap Learn More
- Verify QuickPay Settings opens
Test 4 worth a journey:
- Fund the Spending balance and return to Home
- Wait for the Background Payments timed sheet
- Verify Later and Enable are both shown
- Tap Later
- Reset the settings state in Dev Settings
- Open Settings, General, and Background Payments
- Verify the full-screen intro shows only Enable
Test 5 worth a journey:
- Open Settings, Security, PIN Code, and Enable PIN
- Verify the PIN chooser opens
- Return to a fresh empty wallet and reveal the Secure suggestion
- Open Secure and verify Later is shown
- Tap Secure Wallet and verify the PIN chooser opens
Test 6 worth a journey:
- Receive and confirm a transaction in the wallet
- Open Send with a regtest on-chain address
- Enter 10 000 sats and continue
- Swipe to pay
- Verify the transaction checkmark matches the intro artwork width and remains fully visible
Test 7 worth a journey:
- Change the app locale to French
- Fund the Spending balance and return to Home
- Wait for QuickPay Intro
- Verify the description is translated into French
Coverage:
QA: 7 of 7 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)
327a153 to
9c79f4d
Compare
There was a problem hiding this comment.
Verdict: ✅ Approve
Reaudit: diff 20 files.
Findings:
2 inline (non-blocking)
Audit:
Already done in comment.
Coverage:
QA: journeys and manual tests await all reviewers to approve, author can run it now via comment: @ovi-reviewer test
Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)











This PR:
Description
Out of Scope
Design
Back up wallet › Backup Intro,Settings > General › BackgroundPayments Intro, andSettings > General › QuickPay Intro.docs/screens-map.md.Preview
demo.mp4
QA Notes
Manual Tests
Automated Checks
just compile,just test(2,602 tests), andjust lintpass.