Conversation
|
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
jvsena42
left a comment
There was a problem hiding this comment.
One MEDIUM: the e2e-tests-local - settings job is deterministically red on this branch, and the fix lives in bitkit-e2e-tests rather than here. Details on the inline comment. COMMENT rather than REQUEST_CHANGES because the app code is right — pointing REQUEST_CHANGES at this diff would blame the wrong repo — but it does block the merge queue until the companion branch exists.
The app behaviour itself is clean. What I checked:
Displayed amount. The rendered unit comes from CurrencyState.primaryDisplay, derived from settingsStore.data. switchBalanceUnit() writes primaryDisplay and ignoresSwitchUnitToast in one updateData, and the toast text is built from the pre/post values captured inside that same transform — so the header and the toast cannot disagree. The rate-refresh path is untouched.
Hidden-balance leak. Toast args are only "Bitcoin" or the currency code; the hide toast takes no args. No amount is ever interpolated, so nothing reveals a balance while it is hidden.
Upgrade default. Both new fields default false, v2.4.1's SettingsData has no such keys, and the Json is configured ignoreUnknownKeys = true, so kotlinx fills them with defaults. Every existing user upgrades to "not yet seen" and sees each toast once on their next gesture — which matches the intent. An older app restoring a newer backup ignores the two extra keys, and restoreFromBackup resets both flags explicitly. DataStore.updateData returns only after the durable write and the toast is sent after it returns, so a crash can't re-show it.
Strings. All four keys exist in values/strings.xml and in all 13 locale files, with %1$s present in both arg-taking strings in every locale and absent from the no-arg ones — no format-arg crash. Keys are alphabetically placed, nothing is hardcoded, and both ViewModels localize via injected @ApplicationContext.
Concurrency. Toasts go through ToastEventBus (extraBufferCapacity = 1, SUSPEND overflow, so nothing is dropped) collected once in AppViewModel.init. The flag flips inside the same updateData as the state change, so a rapid double tap can't produce two toasts. No LaunchedEffect, so no toast on recomposition.
0cae3c667 (scoping the guidance to wallet screens, answering greptile's P1): the fallback setHideBalance(!hideBalance) on the ten non-wallet surfaces looked like it might bypass the enableSwipeToHideBalance guard that toggleHideBalanceFromSwipe carries — it doesn't. allowSwipeToHide = useSwipeToHide && isSwipeToHideEnabled already gates the swipe modifier itself, so with the setting off the branch is unreachable. Clean.
Design. Frames are linked and I diffed against them. Wallet Balance Hidden (46920-147840) specifies the toast as title "Wallet Balance Hidden" / body "Swipe your wallet balance to reveal it again." — the implementation's copy matches verbatim, and the blue accent matches the frame's Accents/Blue. No delta worth raising.
| title = context.getString(R.string.wallet__balance_unit_switched_title, newUnit), | ||
| description = context.getString(R.string.wallet__balance_unit_switched_message, previousUnit), | ||
| visibilityTime = 5000L, | ||
| testTag = "BalanceUnitSwitchedToast", |
There was a problem hiding this comment.
This needs a companion bitkit-e2e-tests branch or the settings suite stays red.
The spec already waits for and dismisses both of these toasts — but only on iOS, because when these toasts shipped on iOS, Android didn't have them. Now Android emits the same test tags and the guards are stale.
bitkit-e2e-tests main (30d5f98), test/specs/settings.e2e.ts:
47: if (driver.isIOS) {
48: await waitForToast('BalanceUnitSwitchedToast');
49: }
...
210: if (driver.isIOS) {
211: await waitForToast('BalanceHiddenToast', { waitToDisappear: false, dismiss: true });
212: }So on Android the toast is never dismissed, and the next step is openSettings() at :52 / :215, which is waitForDisplayed('HeaderMenu') → sleep(500) → sleep(200) → a single click(). ToastOverlay is a fillMaxSize Box at Alignment.TopCenter, mounted in MainActivity.kt:200 after the nav content, so its pointerInput node is the topmost sibling and shadows HeaderMenu in the top band. The click lands on the toast.
The 5s auto-hide does not rescue it, which is the part worth being explicit about: the toast does expire, but openSettings only clicks once and then waits 30s for DrawerSettings with no re-tap. So the drawer never opens and the wait times out. The failure screenshot shows exactly that — Home with $ 0.00 (the unit did switch), no toast, drawer closed.
Evidence it's this PR and not flake — run 34992953927, job 104463600597, all three attempts identical:
Error: element ("android=new UiSelector().resourceId("DrawerSettings")") still not displayed after 30000ms
at async <anonymous> (.../test/specs/settings.e2e.ts:52:7) # @settings_01
at async <anonymous> (.../test/specs/settings.e2e.ts:215:7) # @settings_06
Every other test in the suite that calls openSettings (03, 04, 05, 07, 09, 10, 11) passes, so the drawer is fine — only the two tests that trigger these toasts fail. The same job is green today on five other PR branches and on master.
That run is against e2f61e95. Head 0cae3c667 keeps showFirstHideToast = true on HomeScreen.kt:647 and doesn't touch switchBalanceUnit, so the in-flight run 34995922345 should fail the same way.
Fix: push a bitkit-e2e-tests branch named exactly codex/first-balance-gesture-toasts dropping the two if (driver.isIOS) wrappers — keep the waitForToast calls, they're already platform-agnostic and the tags now exist on both apps. determine-e2e-branch does git ls-remote --exit-code on the app branch name and falls back to main, so it'll pick that up automatically on the next run; merge it alongside this PR. I confirmed no such branch exists today.
Two things I'd avoid: gating the toasts on Env.isE2eTest (removes the only assertion that proves the feature works, and diverges e2e builds from what ships), and a blanket "dismiss any toast" in openSettings (broader than needed, and it would mask future overlay regressions).
Unrelated: the red lightning_security job is infra — failed to bind host port 0.0.0.0:39388 … address already in use during regtest setup, before any test ran.
Description
This PR shows one-time guidance when a user first taps any balance display to switch between Bitcoin and fiat, or first swipes a visible balance to hide it. Both toasts follow the existing Android gestures across balance surfaces, including Activity Detail. The toasts use the new and previous display units, remain visible for five seconds, and stop appearing after their first eligible gesture. The guidance resets for a restored wallet and uses the available iOS translations on Android.
Design
Bitkit - Handoff v62: Wallet On-chain › Wallet BTC and Wallet Balance Hidden.
Preview
QA Notes
Manual Tests
regression:Settings → Security → disable Swipe balance to hide → Home → swipe the total balance: the balance remains visible and no hide toast appears.regression:On a fresh wallet, tap the amount in Activity Detail before tapping Home’s total balance: the currency toast appears on Activity Detail and does not repeat on Home.regression:On a fresh wallet, swipe a visible amount in Activity Detail before swiping Home’s total balance: the hide toast appears on Activity Detail and does not repeat on Home.Automated Checks
SettingsStoreTest.ktcovers the first unit switch, first visible-to-hidden transition, disabled swipes, persistence, and first-use flags after restore.just compile,just test, andjust lintchecks passed.