Skip to content

fix: coachmark position + recurring sync-mode default override#146

Merged
RaheemJnr merged 3 commits intomainfrom
fix/coachmark-position-and-sync-mode-default
Apr 30, 2026
Merged

fix: coachmark position + recurring sync-mode default override#146
RaheemJnr merged 3 commits intomainfrom
fix/coachmark-position-and-sync-mode-default

Conversation

@RaheemJnr
Copy link
Copy Markdown
Owner

Two fixes from v1.5.2 smoke + user reports. Queues for v1.6.0 (per the deferred-bumps plan).

1. Coachmark tooltip clipped at the bottom of Home

Reported by a user on Telegram: the "Got it" button on the sync coachmark gets cut off by the bottom nav bar on most phones because the sync card sits low on Home.

`SyncCoachmark` now uses `BoxWithConstraints` to measure viewport height and flips the tooltip ABOVE the spotlight when there is not enough room below. Otherwise keeps existing below-spotlight placement.

2. Recurring sync-mode default override

User report: "I set sync option to new wallet, so it's not needed to sync. That should be the default if a new wallet is created. I don't think I set it to something else before that."

This bug has been "fixed" multiple times. Each previous patch addressed a single write site. The actual root cause is at the registration layer:

  1. `WalletRepository.createWallet` calls `markFreshWalletSyncMode` which writes `NEW_WALLET` to the per-wallet sync mode key.
  2. `HomeViewModel.registerAccount` then computes `firstTimeSyncMode` based purely on the current network: mainnet → `RECENT`, testnet → `NEW_WALLET`. Whenever `hasCompletedInitialSync` is false (which it is for any fresh wallet), `firstTimeSyncMode` wins regardless of what `markFreshWalletSyncMode` just wrote.
  3. `registerAccount` then writes that mainnet `RECENT` back into the per-wallet key with `savePreference = true`, silently overwriting the `NEW_WALLET` from step 1.

So fresh wallets on mainnet always end up at `RECENT` and start a 30-day re-scan the user did not ask for.

Fix in two layers

Layer 1 — WalletPreferences

  • New `getSyncModeOrNull(network, walletId)` that returns null when no value has been written for the wallet/network. Lets callers distinguish "user explicitly chose RECENT" from "nothing written yet".
  • `getSyncMode` default flipped from `RECENT` to `NEW_WALLET`. RECENT was the wrong choice for a fresh wallet with no past activity to find. Callers that need a network-aware first-time default should use `getSyncModeOrNull` and apply their own fallback.
  • Tests updated.

Layer 2 — HomeViewModel

Three-step resolution on first-registration:

  1. If `hasCompletedInitialSync` is true, respect `savedSyncMode`.
  2. Otherwise, if `getSyncModeOrNull` returns a value (i.e. `markFreshWalletSyncMode` or the post-import sheet wrote one), use it. This is the path that was being silently overwritten.
  3. Only when neither (1) nor (2) applies (legacy upgraders whose per-wallet key was never written) do we fall back to the network-default heuristic.

Test plan

  • `assembleDebug` succeeds.
  • `testDebugUnitTest` passes including new `getSyncModeOrNull` tests.
  • Manual: create a fresh wallet on mainnet, verify it does NOT kick off a RECENT-mode sync. Should land at NEW_WALLET (sync completes near-instantly).
  • Manual: import a wallet on mainnet, verify the post-import sheet still shows and the chosen sync mode is honored after registration.
  • Manual: legacy single-wallet user upgrading from pre-M3 build should still get a sensible default (network heuristic kicks in).
  • Manual: trigger the coachmark on a small phone (Pixel 4a-class), verify the tooltip renders above the spotlight without clipping.

Three references in website/index.html (hero, secondary CTA, footer
nav link) bumped from v1.5.1 to v1.5.2 so the download link lands
on the v1.5.2 hotfix release.
Smoke report: on Home, the sync card sits low enough that the
tooltip below it gets clipped by the bottom-nav bar. Switch to
BoxWithConstraints so we can measure viewport height; if the
distance from spotlight.bottom to viewport.bottom is less than the
tooltip's minimum height, render the tooltip ABOVE the spotlight
instead. Otherwise keep the existing below-spotlight placement.
…stration

Recurring user report: a freshly created wallet would default to
RECENT sync mode and start a 30-day re-scan, even though
WalletRepository.markFreshWalletSyncMode had just written
NEW_WALLET to the per-wallet key on creation.

Root cause: HomeViewModel computed firstTimeSyncMode based purely
on the network (mainnet -> RECENT, testnet -> NEW_WALLET) and used
it whenever hasCompletedInitialSync was false. registerAccount
then wrote that value into the per-wallet key with savePreference
= true, silently overwriting whatever markFreshWalletSyncMode had
set seconds earlier. Each previous fix patched a single write
site; the override at the registration layer kept the bug alive.

Two-layer fix:

1. WalletPreferences gains getSyncModeOrNull() that returns null
   when no value has been written for the wallet/network. The
   existing getSyncMode() still has a sensible default but is now
   NEW_WALLET (was RECENT) — RECENT was the wrong choice for a
   fresh wallet with no past activity. Tests updated.

2. HomeViewModel checks getSyncModeOrNull first. If a per-wallet
   value exists (from markFreshWalletSyncMode or the post-import
   sheet), it wins. The network-default heuristic only kicks in
   for legacy upgraders whose per-wallet key was never written.
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pocket-node Ready Ready Preview, Comment Apr 30, 2026 0:22am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

Warning

Rate limit exceeded

@RaheemJnr has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 26 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 998fafdd-480f-420e-8d2d-c48806e49679

📥 Commits

Reviewing files that changed from the base of the PR and between ac9fac6 and 0df09fa.

📒 Files selected for processing (5)
  • android/app/src/main/java/com/rjnr/pocketnode/data/wallet/WalletPreferences.kt
  • android/app/src/main/java/com/rjnr/pocketnode/ui/education/coachmark/SyncCoachmark.kt
  • android/app/src/main/java/com/rjnr/pocketnode/ui/screens/home/HomeViewModel.kt
  • android/app/src/test/java/com/rjnr/pocketnode/data/wallet/WalletPreferencesTest.kt
  • website/index.html
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/coachmark-position-and-sync-mode-default

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 11 minutes and 26 seconds.

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

@RaheemJnr RaheemJnr merged commit 45af5da into main Apr 30, 2026
5 checks passed
@RaheemJnr RaheemJnr deleted the fix/coachmark-position-and-sync-mode-default branch April 30, 2026 14:01
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.

1 participant