Fix paused and done dark modes on sync screen - #148
Conversation
chaitika
changed the title
chaitika
changed the title
Adds dark-mode color tokens for the scan banner's paused state (custom pause icon) and the SyncStatusIcon paused/error glyphs. Also fixes header backgrounds defaulting to React Navigation's card color instead of the screen background app-wide, and adds a scan-rate (blk/s) readout to the Update Progress card.
85e305f to
cf967f1
Compare
Adds dark-mode tokens for the scanning and done SyncStatusIcon states (previously same() light-only colors), replaces the Material check-circle and refresh glyphs with custom SVG icons (CheckIcon, RetryIcon) matching design, fixes the pause/check-again button borders and check-again text/icon color to be theme-aware, and makes the sync-screen percent sign switch to black/white at 100% done while staying grey mid-scan.
chaitika
changed the title
| syncOuterScanning: pair(palette.white, '#0D0D0D'), // scanning-state icon backing circle | ||
| syncFillScanning: pair('#DCD2F9', '#3A3166'), | ||
| syncRingPaused: pair('#E6E4E499', '#FFFFFF14'), // alpha baked in: 0.6 opacity light, 0.08 opacity dark | ||
| syncFillPaused: pair('#F5F5F7', '#1A1A28'), | ||
| syncOuterPaused: pair(palette.white, '#0E0E16'), // paused-state icon backing circle | ||
| syncGlyphPaused: pair('#8E8E93', '#8888AA'), // paused-state pause glyph | ||
| syncOuterDone: pair(palette.white, '#0D0D0D'), // done-state icon backing circle | ||
| syncRingDone: pair('#E2FAEA', '#16301F'), | ||
| syncFillDone: pair('#D2F9DC', '#1C3D28'), | ||
| syncRingError: pair('#FDF1F2', '#330C09'), | ||
| syncFillError: pair('#FBE9EB', '#3E0B0B'), | ||
| syncOuterError: pair(palette.white, '#0D0D0D'), // error-state icon backing circle |
There was a problem hiding this comment.
syncOuterScanning, syncOuterDone and syncOuterError are all pair(palette.white, '#0D0D0D') — identical to each other, and identical to background: pair(palette.white, palette.gray950) now that gray950 is #0D0D0D (there's a fourth copy at searchIconBackground). The backing circle behind the sync icon is the page background; it should reference colors.background rather than restate the literal three times, otherwise the next background change silently desyncs four tokens.
syncOuterPaused is #0E0E16 — a sub-perceptual delta from #0D0D0D that looks like a Figma export artifact. Because it's a filled circle sitting on the background, this is the one case where an off-by-one-shade fill actually renders: a faint disc edge on the paused state that the other three states won't have. Please make all four the same value.
| surfaceSubtle: pair(palette.violet50, palette.violet900), // banner / card background | ||
| accentSubtle: pair(palette.violet100, '#2D264F'), // banner & card border, "check again" button bg, scanning icon ring |
There was a problem hiding this comment.
These two are now exact duplicates of tokens already in this file:
surfaceSubtle: pair(palette.violet50, palette.violet900)==bannerBackground(L152)accentSubtle: pair(palette.violet100, '#2D264F')==bannerBorderColor(L155)
Worth collapsing rather than keeping two names per value — otherwise a future tweak has to be applied twice to stay consistent.
| checkAgainButtonColor: pair(palette.violet600, palette.white), // "check again" button icon + text | ||
| progressCardBackground: pair(palette.violet50, '#1A1535'), // sync screen "Update Progress" / privacy card background | ||
| progressCardBorder: pair(palette.violet100, '#25253A'), // sync screen "Update Progress" / privacy card border | ||
| pausedIconColor: pair('#1A1A1A', '#F0F0F5'), // paused-state pause icon on the sync banner |
There was a problem hiding this comment.
These are named after the single control that consumes them (checkAgainButtonColor, progressCardBackground, progressCardBorder, pausedIconColor, and continueButtonBorder/retryButtonBorder/syncPercentSignColor below). The block directly above — textPrimary, textSecondary, surfaceSubtle, accentSubtle — is role-based, which is what makes it reusable. Once a token is called checkAgainButtonColor, the next control that wants the same treatment either gets a second token or an obviously-wrong reference. Could these be expressed as roles (e.g. onBrandSurface, surfaceRaised, surfaceRaisedBorder)?
Separately, these lines introduce raw hex (#1A1535, #25253A, #F0F0F5) rather than going through palette at the top of the file. There are ~16 new literals across this hunk, several repeated across tokens — #1A1535 also appears in progressTrack (L123), #F0F0F5 in syncPercentColor (L131), #8888AA twice (L132, L143).
| pausedIconColor: pair('#1A1A1A', '#F0F0F5'), // paused-state pause icon on the sync banner | ||
| surfaceCaution: same('#FDFBF5'), // caution banner background (address-reuse warning) | ||
| iconCaution: same('#F1AF63'), // caution banner icon (warm amber) | ||
| errorBannerText: pair('#B24334', '#C54A3A'), // "can't connect" sync status icon glyph |
There was a problem hiding this comment.
Name/usage mismatch: the token is called errorBannerText, but the comment and the only consumer (SyncStatusIcon.tsx:40) say it's an SVG glyph fill, not banner text.
| copyHint: same(palette.gray450), // "tap to copy" icon + label | ||
| progressTrack: same('#EAECF0'), | ||
| progressTrack: pair('#EAECF0', '#1A1535'), | ||
| buttonBorder: same('#EBEBEB'), |
There was a problem hiding this comment.
buttonBorder has zero references left in the repo after this PR — the three call sites in SyncScreen.tsx were the last ones and all moved to continueButtonBorder/retryButtonBorder. Same for statusPaused (L107), whose last consumers moved to pausedIconColor and syncGlyphPaused.
Preferred fix is to widen these two in place (give them dark values) instead of adding parallel tokens; failing that, please delete them.
| <Text style={[styles.percentSign, { color: colors.textMeta }]}>%</Text> | ||
| <Text style={[styles.percentNum, { color: colors.syncPercentColor }]}>{Math.round(effectivePct)}</Text> | ||
| <Text | ||
| style={[styles.percentSign, { color: effectiveStatus === 'done' ? colors.syncPercentColor : colors.syncPercentSignColor }]} |
There was a problem hiding this comment.
Every other status-dependent colour on this screen goes through a lookup (SyncStatusIcon's iconColors) or the switch in renderTitle/renderActionButton. This inline ternary is the odd one out — and it's the second place that has to be updated when a status is added.
| <View style={styles.cardRow}> | ||
| <Text style={[styles.cardLabel, { color: colors.textMuted }]}>{loc.sync.scan_rate_label}</Text> | ||
| <Text style={[styles.cardValue, styles.cardValueSemibold, { color: colors.textPrimary }]} numberOfLines={1}> | ||
| {scanState.scanRate !== null ? loc.formatString(loc.sync.scan_rate_value, { rate: scanState.scanRate }) : '--'} | ||
| </Text> | ||
| </View> |
There was a problem hiding this comment.
This card renders for paused as well as scanning (L229), but pauseScan() (hd-bip352-wallet.ts:136) emits 'paused' without clearing scanRate. So a paused screen shows the title "Paused" above a live-looking "12 blk/s". resumeScan() already clears _scanSamples correctly (:143); pauseScan() just needs to null scanRate alongside it, or this row should be hidden while paused.
| const blocksGained = newest.block - oldest.block; | ||
| if (elapsedMs > 0 && blocksGained > 0) { | ||
| scanRate = Math.round(blocksGained / (elapsedMs / 1000)); | ||
| } |
There was a problem hiding this comment.
Two rough edges in how this reaches the UI:
Math.roundfloors anything under 0.5 blk/s to0 blk/s, which reads as "stalled" on exactly the slow scans where the user is most likely to be watching this number.- The
blocksGained > 0guard leavesscanRatenull whenever the rolling window hasn't advanced a block, and the UI renders that as--(SyncScreen.tsx:253). Combined with the point above, the row alternates between a number,0, and--.
A one-decimal value below 1, or holding the last known rate instead of dropping to null, would be steadier.
| "done": "You're all caught up", | ||
| "error_title": "Can't connect right now", | ||
| "error_description": "Shroud can't update right now, but your Bitcoin is secure.", | ||
| "error_description": "Shroud can't update right now,\nbut your Bitcoin is secure.", |
There was a problem hiding this comment.
Hardcoded \n puts layout into the copy. Where the line should break is a function of container width and font scale, not of the string — and it'll be wrong for any locale added later. en is the only locale today so the impact is small, but this belongs in the style (width constraint / textAlign) rather than the content.
| startedAt: number | null; | ||
| eta: number | null; | ||
| etaComputedAt: number | null; | ||
| scanRate: number | null; |
There was a problem hiding this comment.
Scope: the scan-rate metric is a new user-facing feature threaded through ScanStateInfo -> the wallet class -> loc -> the UI, in a PR titled "Fix paused and done dark modes on sync screen". It's also the only part of the diff with runtime behaviour to verify, and the screenshots don't cover it. Could it go out as its own PR so the dark-mode change stays reviewable on its screenshots alone?
Scan State
Current View
Updated View
Paused State
Current View
Updated View
Done State
Current View
Updated View