Conversation
b38a79d to
46f6a4d
Compare
chaitika
changed the title
|
"simple bitcoin wallet" text should be larger 
|
|
shows step 2 directly instead of starting off with 1. 
|
|
|
colors off. 
|
|
@notTanveer thanks for the review. this PR is a WIP with dark themed tokens not in place yet, as well as some other nits. will update once the PR is in place with screenshots and cleanups. |
Rebuilds the Onboarding, Backup Intro, Recovery Phrase reveal, and Verify screens against new Clash Grotesk mockups: a blur-then-reveal seed grid, a consent checkbox gating Continue, per-tap correct/ incorrect feedback on verification, and ten new custom icon components matching the provided SVGs exactly. Also fixes the wallet creation screen rendering a duplicate native header back button. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- SeedVerification: wire "Show Phrase Again" text/icon and heading typography to theme tokens instead of hardcoded hex; track selection order for incorrect taps too, so the wrong-word badge shows a number. - themes.ts: correct verifyIncorrectFill's dark value (was maroon900, should be red950) and textPrimary's light/dark values (pure black / gray75) to match the Figma spec. - PleaseBackup: fix backup-intro progress bar (was showing step 2 instead of 1), align tip-card sizing/spacing/text color and heading typography with Figma measurements, and blur the hidden seed-word grid with @react-native-community/blur instead of hiding it behind empty placeholder cells. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018FWaE77e8evSWJBiGcDuFN
…d verify flows Follow-up to 9061b09, fixing bugs in and completing that commit's onboarding/backup redesign work: - PleaseBackup: fix a purple halo/glow rendering around the reveal circle, caused by BlurView's Android capture root being hardcoded (in its native source, not exposed as a prop) to the whole Activity content view — any foreground content sharing that window, regardless of RN-level nesting, gets swept into the blur. Move the reveal circle into a Modal (a genuinely separate native window) to keep it out of that capture; since the Modal's Dialog swallows all screen touches while visible (Android default), add an invisible same-position tap target so the header back button keeps working, and reset the reveal/checkbox state on returning to the intro step so the privacy gate re-arms. Also: tune blur intensity down for Android specifically (its blur reads far stronger per unit than iOS's), rework the revealed-state pill colors/borders to match the design (transparent word half, only the index box filled), and add accessibility props to the "confirm written down" checkbox. - SeedVerification, OnboardingScreen, BackupStepHeader: further color-token fixes (default/error-banner colors, CTA/secondary-button colors, progress-bar fill) pixel- verified against the design exports, replacing values that were close but not exact matches (or, for the verify screen, reused from tokens shared with unrelated screens). - ShieldReceiveIcon: correct stroke width to match the design's vector measurements. - Add CameraOffIcon, replacing a generic info icon in the backup screenshot-warning banner per the design. - themes.ts: new/corrected tokens backing all of the above. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rGxnMoV64dQEkZ8vJYwBR
… screen - WalletsList: correct empty-state colors (active pay button, empty card, share-address button/text, shield icon accent) and spacing against the design export. - Settings: dedicated tokens for the main-menu card and Delete Wallet button, instead of reusing tokens shared with other Settings screens that have no design coverage yet. The Delete Wallet button was also missing its background fill entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rGxnMoV64dQEkZ8vJYwBR
056f968 to
589f775
Compare
Detox resolves element(by.id(...)) against the currently focused window only. The Modal added to keep BlurView's Activity-wide capture root from swallowing the reveal circle takes real window focus once shown, which made PleaseBackupScrollView and SkipVerifyBackdoor (in the main window, not the Modal) unreachable to every e2e spec via the shared helperCreateWallet — CI's e2e job failed all 21 tests across 6 suites. Render the reveal circle as a plain sibling (no Modal) under isE2E() instead: no current e2e spec exercises the reveal interaction directly (all bypass verification via SkipVerifyBackdoor), so this costs no real coverage while unblocking the tests Detox actually runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rGxnMoV64dQEkZ8vJYwBR
| "@noble/secp256k1": "1.6.3", | ||
| "@react-native-async-storage/async-storage": "2.2.0", | ||
| "@react-native-clipboard/clipboard": "1.16.3", | ||
| "@react-native-community/blur": "^4.4.1", |
There was a problem hiding this comment.
Blocking: new native module, but ios/Podfile.lock was never regenerated.
@react-native-community/blur ships a native pod (RNCBlurView), yet this PR touches no files under ios/ or android/ — grep blur ios/Podfile.lock returns nothing. Consequences:
- CI running
pod install --deploymentfails on the lockfile mismatch. - A build that reuses a cached Pods dir links no
RNCBlurView, so<BlurView>(PleaseBackup.tsx:295) throws "requireNativeComponent: 'BlurView' was not found in the UIManager" — or silently renders nothing, leaving the seed grid unconcealed.
npx tsc --noEmit already fails on this branch:
screen/wallets/PleaseBackup.tsx(15,26): error TS2307: Cannot find module '@react-native-community/blur' or its corresponding type declarations.
Please run npm i && (cd ios && pod install) and commit the resulting ios/Podfile.lock.
Also: caret range breaks the pinning convention here. Every other dependency in this file is exact ("@react-native-clipboard/clipboard": "1.16.3", "@react-native-community/cli": "15.1.3", "@react-native-documents/picker": "10.1.5"). A caret on a native module lets a later npm install pull 4.5.x with different native sources and silently desync the committed Podfile.lock. Please pin to 4.4.1.
| (isE2E() ? ( | ||
| revealContent | ||
| ) : ( | ||
| <Modal transparent animationType="none" onRequestClose={handleBackToIntro}> |
There was a problem hiding this comment.
Android: this Modal needs statusBarTranslucent, otherwise every absolutely-positioned child is off by one status-bar height.
RN 0.78 ReactModalHostView.kt sets fitsSystemWindows = true on the dialog's content root whenever statusBarTranslucent is false — its own comment says this saves you "having to preface all Modals with top: statusBarHeight". But revealWindowLayout comes from measureInWindow (line 90), which is activity-window coordinates including the status bar, and backButtonWindowLayout (line 108) is built from insets.top. Both coordinate spaces are then stacked inside an already-inset container.
Net effect on Android (targetSdk 35 ⇒ edge-to-edge, so insets.top > 0):
- the "Tap to reveal" circle and its tap target land ~24–48dp below the blurred grid;
backButtonGhostlands at roughly2 × statusBarHeight + 16, so it never overlaps the real header back button it exists to restore — and since the Dialog swallows all touches, the user is stuck on this step.
WalletsList.tsx:690 already does this correctly:
<Modal transparent visible={showZeroBalanceToast} statusBarTranslucent animationType="none" onRequestClose={dismissToast}>|
|
||
| <View | ||
| style={[styles.wordGridWrapper, { backgroundColor: isRevealed ? colors.transparent : colors.gridContainerBackground }]} | ||
| onLayout={handleGridLayout} |
There was a problem hiding this comment.
The blur overlay and the reveal tap target are measured once and never follow the scroll.
onLayout fires on layout changes; scrolling a ScrollView does not fire onLayout on its children. This step's content (32pt title + subtitle + warning banner + 6-row grid + checkbox, between a 72pt header and an 80pt footer) overflows the viewport on any normal phone, so it is scrollable in practice.
Once the user scrolls, gridRevealOverlay (line 294) and revealOverlay (line 165) stay frozen at the original screen rect: the blur rectangle floats over unrelated content while the grid slides out from under it, and the "Tap to reveal" target no longer sits on the grid.
Re-measure on scroll — e.g. onScroll on the ScrollView (throttled) calling the same handleGridLayout, or track contentOffset.y and subtract it from the stored rect.
| ))} | ||
| {currentStep === BackupStep.INTRO && ( | ||
| <View style={styles.stepRoot}> | ||
| <BackupStepHeader onBack={() => navigation.goBack()} filledSteps={1} totalSteps={3} testID="BackupIntroBackButton" /> |
There was a problem hiding this comment.
This re-opens the escape hatch that the rest of the screen exists to close.
AddWalletStack.tsx dropped headerBackVisible: false in favour of headerShown: false, but the screen still has gestureEnabled: false and blocks hardware back:
BackHandler.addEventListener('hardwareBackPress', () => true); // line 176So hardware back is blocked while a visible chevron now calls navigation.goBack(). A user who just created a wallet via OnboardingScreen (navigate('AddWalletRoot', { screen: 'PleaseBackup' }), stack [AddWallet, PleaseBackup]) can tap it and land back on the "Add wallet" screen holding an unbacked-up wallet.
Either the three guards should agree (drop the intro back button, or make it an explicit "skip backup" confirmation), or the BackHandler block and gestureEnabled: false should go.
| const { walletID } = useRoute<RouteProps>().params; | ||
| const wallet = wallets.find(w => w.getID() === walletID)!; | ||
| const seedPhrase = wallet.getSecret(); | ||
| const seedWords = seedPhrase.split(' '); |
There was a problem hiding this comment.
seedWords gets a new array identity on every render, which re-shuffles the verification grid mid-flow.
SeedVerification re-shuffles on seed identity:
useEffect(() => {
const shuffled = [...seed].sort(() => 0.5 - Math.random());
setShuffledWords(shuffled);
}, [seed]); // SeedVerification.tsx:103-106Any re-render of PleaseBackup — a useStorage wallets refresh, a useSettings change, a theme change — produces a fresh array and reshuffles. But selectedIndices / wordStatus / selectionOrder stay keyed by the old positions, so after a reshuffle the green badges sit on the wrong words, isDisabled = selectedIndices.includes(index) disables arbitrary pills, and expectedWord = seed[selectedIndices.length] no longer matches what's displayed — verification becomes unpassable until the user backs out.
This PR also adds four new state variables to this screen, so such re-renders are far more frequent than before.
| const seedWords = seedPhrase.split(' '); | |
| const seedWords = useMemo(() => seedPhrase.split(' '), [seedPhrase]); |
(needs useMemo added to the react import on line 2)
Separately, while you're in that effect: sort(() => 0.5 - Math.random()) is a biased, engine-dependent permutation (a random comparator breaks sort's transitivity contract, and V8's TimSort leaves words near their original index). For a check whose whole job is to prove the user wrote the phrase down, that's worth replacing with a Fisher–Yates shuffle.
| // check ALL words are selected | ||
| if (selectedIndices.length + 1 === seed.length) { | ||
| if (expectedPosition + 1 === seed.length) { | ||
| setTimeout(() => { |
There was a problem hiding this comment.
Neither timer is stored or cleared, so one of them can navigate after unmount.
Sequence: user taps the final correct word (this schedules onSuccess at +500ms), then immediately taps Show Phrase Again. onBack sets the step back to SHOW_SEED and this component unmounts — but the orphaned timer still calls handleVerifyComplete → navigation.navigateToWalletsList(), yanking the user off the seed screen 500ms later.
Same for the 2500ms reset at line 135: tapping two wrong words inside the error window schedules two independent resets and writes the same expectedPosition badge number onto both pills.
Keep both in a ref and clear them in a useEffect cleanup.
|
|
||
| {/* onRequestClose mirrors the header's back handler — the Modal's Dialog intercepts | ||
| the hardware back key natively before BackHandler would see it. */} | ||
| {!isRevealed && |
There was a problem hiding this comment.
In the e2e branch the reveal children keep window coordinates but lose the window.
Under isE2E(), revealContent renders as a plain flex child (modalRoot: { flex: 1 }), so it is laid out between the ScrollView and the footer rather than at the window origin — while its children are still positioned in screen-absolute measureInWindow coordinates.
Result: revealOverlay at top: revealWindowLayout.top (~300) resolves to ~700 absolute, covering the footer and ContinueToVerify with a full-grid-sized invisible TouchableOpacity; backButtonGhost becomes a floating 32×32 target mid-screen wired to handleBackToIntro(). Any Detox tap landing on either silently changes state instead of hitting the intended control.
If the e2e path has to skip the Modal, it should also skip the window-coordinate positioning — position both children relative to modalRoot (or render the overlay absoluteFill with the circle centred) under isE2E().
| // Not settingsCardBackground/Border: those are shared by every Settings screen, unverified here. | ||
| verifyDefaultPillBackground: pair('#F9F9FB', 'transparent'), | ||
| verifyDefaultPillBorder: pair('#F0F0F0', '#212129'), | ||
| verifyErrorBannerBackground: pair('#FDFBF5', '#2E2518'), // not surfaceError — that's tuned for a different screen |
There was a problem hiding this comment.
These re-declare hexes that already exist in the palette, which is exactly what the palette layer is there to prevent.
Lines 5–8 of this file:
Primitive palette: raw color values, each declared exactly once. The semantic tokens below alias these, so a value change happens in one place.
Three new tokens break that:
verifyErrorBannerBackgrounddark'#2E2518'===palette.brown900(line 41)gridContainerBackgrounddark'#1A1A28'===palette.slate890(line 47)revealedPillBorderdark'#1A1A28'===palette.slate890(line 47)
Retuning brown900 or slate890 now silently skips these three screens. Please alias the palette entries (the per-token comments explaining why these screens need their own token stay valid either way — it's the raw value that should come from the palette).
| gridContainerBackground: pair('#F8F8FC', '#1A1A28'), // blurred together with every pill as one unit; gridScrimBackground darkens it further | ||
| gridScrimBackground: pair('#FFFFFF33', '#1A1A284D'), | ||
| revealedPillBorder: pair(palette.gray200Alpha, '#1A1A28'), // light matches borderDefault's --border-default; dark is pixel-verified for this screen specifically | ||
| transparent: same('transparent'), // named so conditional styles stay lint-clean (no-inline-styles rejects literal ternary branches) |
There was a problem hiding this comment.
A lint workaround promoted into the semantic color system.
The comment is explicit that this token exists only because no-inline-styles rejects literal ternary branches. It's consumed at PleaseBackup.tsx:113 and :251 as colors.transparent — a themeable token that can never be themed, and restoreButtonBackground: same('transparent') (line 156) is now a second same-valued alias for the same non-color.
The rule is really asking for the style shape to change, not for the color system to grow an entry. Two StyleSheet.created variants picked by isRevealed (which this file already does for pillColors) fixes the lint and keeps the palette to colors.
| borderRadius={16} | ||
| disabled={!hasConfirmedWritten} | ||
| disabledBackgroundColor={colors.backupContinueDisabledBackground} | ||
| disabledTextColor={colors.white} |
There was a problem hiding this comment.
Disabled label fails contrast, on the one control that tells the user what to do next.
colors.white on backupContinueDisabledBackground is #9AA0AA in light (~2.4:1) and #8E8E93 in dark (~2.9:1) — both below the 4.5:1 minimum for 16pt body text.
Until the user ticks ConfirmWrittenDown this button is the primary affordance and its label is barely legible. Note Button.tsx already defaults disabled text to alternativeTextColor; this explicitly overrides it to produce the low-contrast pairing.
Addresses @theanmolsharma's 13-thread review on this PR (verified each against current code before fixing): - Pin @react-native-community/blur to exact 4.4.1 and install it (was missing from node_modules entirely, causing the cited tsc failure) - Modal: add statusBarTranslucent so the reveal-circle overlay doesn't land a status-bar height off on Android - Re-measure the blur/reveal overlay on scroll instead of only on initial layout - Memoize seedWords and replace the biased sort(() => 0.5 - Math.random()) shuffle with real Fisher-Yates, so a mid-verification re-render can't reshuffle and desync the selection state - Move SeedVerification's hardcoded English ordinals into loc/en.json - Store and clear SeedVerification's reset/success timers on unmount so a stale one can't navigate away after the user leaves the screen - Fix e2e-branch coordinate math: the non-Modal reveal content needs stepRoot-relative coordinates, not window-absolute ones - themes.ts: alias three tokens that duplicated raw palette hex values, remove the transparent lint-workaround token, and split progressTrack into its own backup-specific token so the shared one no longer regresses SyncScreen's dark-mode progress bar - useScreenProtect: wrap its callbacks in useCallback so the screen-protect focus effect isn't torn down and re-armed on every render while the phrase is on screen - Stop overriding the Continue button's disabled text color to white, which failed contrast against its background - Turn the intro step's back button into an explicit "skip backup" confirmation instead of a silent goBack(), so the screen's hardware- back/swipe guards aren't undercut by an unguarded chevron Podfile.lock regeneration for the blur pod still needs a macOS/ CocoaPods run - not done here.
Uh oh!
There was an error while loading. Please reload this page.