Refine avatar picker, profile settings, and verification cooldown UX - #3028
Conversation
# Conflicts: # src/components/Avatar/AvatarPicker.tsx # src/components/Profile/views/About.view.tsx # src/hooks/usePullToRefresh.ts
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Code-analysis diffPainscore total: 7268.94 → 7279.31 (+10.37) 🆕 New findings (113)
…and 93 more. ✅ Resolved (107)
…and 87 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff —
|
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Found a global pull-to-refresh regression and three smaller release/copy defects. Exact-head required CI is red in the unit job.
Findings
-
MAJOR · src/hooks/usePullToRefresh.ts:169 · Ignore only dialogs that are actually open
The selector treats every mounted role=dialog element without data-state=closed or hidden as open. SupportDrawer is always mounted by both app layouts and, while closed, still has role=dialog with aria-modal=false and neither of those attributes. Therefore every touchstart hits this guard and pull-to-refresh is disabled across the app even when no overlay is visible. Restrict the check to real open state (for example data-state=open or aria-modal=true), and add a layout-level regression case with a closed always-mounted SupportDrawer. -
MINOR · src/i18n/app/messages/en.json:597 · Update delete-account tests for the new label
The required unit job fails all ten DeleteAccountButton cases because they still query the exact old text 'Delete My Account'; after this line changes the rendered label, none of those tests reaches its behavioral assertions. Update the shared test query/fixture to the new sentence-case label (preferably by accessible role and name) so the required unit gate can pass. -
MINOR · src/i18n/app/messages/en.json:314 · Resolve the exchange-rate translation drift
Changing this menu value to 'Exchange Rates and Fees' makes it identical to exchangeRate.title, but the resolved Spanish catalogs render the two keys differently ('Tipos de cambio y comisiones' versus 'Tipo de cambio y comisiones'). The exact-head duplicate-value drift tests fail for es-419 and es-AR. Align the translations, reuse one canonical key, or explicitly document a genuine context distinction in the test allowlist. -
MINOR · src/i18n/app/messages/en.json:310 · Carry the Payment Channels rename into every locale
Only the English menu and page title are renamed here. Switching to es-419, es-AR, or pt-BR still shows the old imperative 'Unlock payments' wording for both keys, so the product rename disappears with the language setting even though the other new copy in this PR is localized. Add approved equivalents for profile.menu.unlockedRegions and profile.unlockPayments.title in each supported catalog.
Checked clean
- Confirmed the detached worktree HEAD, PR metadata, supplied base SHA, and merge base exactly match the requested review coordinates.
- Reviewed the avatar picker deal, persistence fallback, save serialization, responsive grid, reduced-motion path, haptics cleanup, and drawer lifecycle.
- Reviewed restart-cooldown parsing and presentation against the sibling API policy branch, including 429 retryAt and Retry-After behavior; no security, privacy, or money surface was introduced.
- Checked the live canonical Lexicon and all four locale catalogs for the changed payment terminology.
- Reviewed the profile header, About ordering, profile-edit toggle, payment-channel rows, limit bars, and associated focused tests.
- Exact-head CI: typecheck, eslint, format, native-export, design-system checks, workflow analysis, and deployment passed; required unit failed with 12 tests across DeleteAccountButton and translation drift, causing ci-success to fail.
- Focused local execution passed 26 tests in usePullToRefresh, Sumsub refusal parsing, and avatar dealing; five additional focused suites could not resolve next-intl from the available external dependency tree, while exact-head CI did execute them.
- git diff --check reported no whitespace errors.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion: did not run — claude-unparseable-reply. This review is one reviewer short.
Exact head: 277e4f4017a7 · Context: repo, product, api · Took 23m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The avatar, profile, dialog, and verification-cooldown behavior is sound, but the Payment Channels rename remains English-only and exact-head unit CI fails on Spanish exchange-rate translation drift.
Findings
-
MINOR · src/i18n/app/messages/en.json:314 · Make the Spanish exchange-rate labels agree
Exact-headunitCI failsduplicate-value driftbecause these two English labels now both readExchange Rates and Fees, while es-419 renders the profile menu asTipos de cambio y comisionesand the destination page asTipo de cambio y comisiones; es-AR inherits the same mismatch. Pick one Spanish rendering for both keys so the menu and page title agree and the required unit gate passes. -
MINOR · src/i18n/app/messages/en.json:310 · Translate the Payment Channels rename
The English menu and page are renamed toPayment Channels, but es-419 and pt-BR still render the oldUnlock paymentsconcept, and es-AR explicitly overrides both labels with that old name. A user switching languages therefore sees the previous information architecture return. Update bothprofile.menu.unlockedRegionsandprofile.unlockPayments.titlein es-419, es-AR, and pt-BR to carry the rename through every supported app locale. -
MAJOR · src/i18n/app/messages/en.json:390 · [claude-opus] Europe rail relabelled "Bank Transfers" over-promises non-EUR European bank support
profile.unlockPayments.rows.sepachanged from "SEPA transfers" to "Bank Transfers" (same in es-419/es-AR/pt-BR: "Transferencias bancarias" / "Transferências bancárias"). That row is the sole row of theeuropegroup (src/utils/unlock-payments.utils.ts:168,bankRow('sepa','sepa','bank','europe',['bridge'])), so a user in the Europe section now reads "Europe → Bank Transfers → Active" with no mention of the scheme or currency.
Product truth says the European rail is SEPA and EUR-only: product/countries.md:416 ("SEPA deposits currently EUR-only... Multi-currency SEPA on roadmap"), product/countries.md:452, product/currencies.md:124 (EUR (SEPA) only), product/quick-ref.md:37, and the customer-facing phrasing in product/support-answers/deposit-options-by-region.md:16 ("EU (SEPA, 41 countries): EUR bank transfer"). product/countries.md:459 further records that UK residents are blocked from every bank rail (TASK-20729), so the broader label is not more accurate for GB/FPS either. The code is the side that is wrong.
This is the exact failure mode already logged as recurring in product/feedback/problems/non-eur-sepa-deposits-unsupported.md — "docs/UI imply they're supported" while PLN/GBP/CZK transfers bounce, sometimes with fees, flagged as the "10th time" it recurs. Every sibling row on this screen keeps its rail and country explicit ("ACH, Wire (US) & SPEI (Mexico)", "PIX (Brazil)..."), so this row is now the only vague one.
Fix: restore the scheme and currency in the label, e.g. "SEPA transfers (EUR)" in en.json and the three locales (product truth allows naming the SEPA zone; it only forbids asserting a country count). Note the label also feeds the unlock modal title (UnlockPayments.view.tsx:313), so the fix propagates there for free.
Checked clean
- Confirmed the detached worktree HEAD, supplied base SHA, merge base, trusted author, and dev target exactly match the review coordinates.
- P1 is fixed at this head: Vaul dialogs are selected only by data-state=open, HeadlessUI exposes aria-modal only in its open state, and the always-mounted support dialog sets aria-modal=false while closed; the new regression test covers that support-drawer case.
- P2 is fixed at this head: every changed delete-account interaction now queries the sentence-case
Delete my accountbutton label. - Reviewed avatar deal eligibility and art deduplication, responsive 3x3/4x4 sizing, staged confirmation, duplicate-save prevention, device-local letter fallback, dice timer and haptic cleanup, and reduced-motion handling.
- Reviewed the verification restart 429 contract, retryAt and Retry-After normalization, cooldown state reset, localized date rendering, and the single dismiss action.
- Reviewed profile-link composition, About ordering, name-visibility toggle placement, payment-row wrapping and status treatments, and the unlimited Peanut-to-Peanut progress bar.
- Exact-head CI passed format, eslint, typecheck, ds-lint, native-export, ds-shots, analyze, human-authors, bot-approval, and Deploy Preview; unit failed only the two Spanish duplicate-value drift assertions described in P3, leaving ci-success red.
- Focused local Jest execution was unavailable because the detached worktree has no installed Jest binary; git diff --check passed.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 8164af9483ef · Context: repo · Took 22m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Two localization defects remain: non-English users still see the retired verification name, and the Spanish exchange-rate labels disagree and fail exact-head unit CI. The other supplied prior findings are fixed or refuted.
Findings
-
MINOR · src/i18n/app/messages/en.json:310 · Carry the Payment Channels rename into every locale
The English menu and destination now say Payment Channels, but es-419, es-AR, and pt-BR still render their old Unlock payments wording for bothprofile.menu.unlockedRegionsandprofile.unlockPayments.title. Switching the app locale therefore makes the rename disappear. Translate the new product name in both keys for each shipped locale. -
MINOR · src/i18n/app/messages/en.json:314 · Make the Spanish exchange-rate labels agree
This head makes the English menu and page title shareExchange Rates and Fees, but es-419 resolves those keys toTipos de cambio y comisionesandTipo de cambio y comisiones; es-AR inherits the same mismatch. Exact-head unit CI now fails both duplicate-value drift cases. Align the two Spanish renderings, or mark the contexts divergent only if that grammatical difference is intentional.
Checked clean
- Confirmed the supplied detached worktree is exactly the requested head and its merge base is the supplied dev base SHA.
- Reviewed avatar dealing, selection confirmation, save fallback, duplicate-submit guard, reduced-motion behavior, and responsive grid sizing.
- Reviewed restart cooldown parsing and presentation, non-429 recovery behavior, and KYC flow state reset paths.
- Reviewed profile-header formatting, About ordering, delete-account label coverage, and pull-to-refresh overlay cancellation.
- Exact-head typecheck, lint, formatting, design-system checks, native export, visual snapshots, and deploy preview passed; unit CI ran 6,119 tests and failed only the two Spanish duplicate-value drift assertions reported above.
- P5 is the same surviving Spanish catalog defect as P3 and is represented once under P3; P6 is the same surviving locale rename defect as P4 and is represented once under P4.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: a49e4f70bb5e · Context: repo, product · Took 22m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The locale and test findings are fixed, but restart cooldown UX still reaches only the profile Payment Channels screen; the other payment flows discard the retry time.
Findings
- MAJOR · src/hooks/useMultiPhaseKycFlow.ts:547 · Render restart cooldowns in every payment flow
errorCooldownis returned by the shared KYC flow, but onlyUnlockPaymentsconsumes it. Add-money, withdraw, claim, and QR restart paths still pass onlyflow.errortoInitiateKycModalor a notification. When restart returns 429 withretryAt, those screens therefore show the raw error—some replacing the action with Contact support—instead of the dated cooldown and single I'll try later action promised by this change. Centralize cooldown presentation in the shared modal layer, or thread the metadata through every consumer, and cover one non-profile restart path.
Checked clean
- Confirmed the detached worktree HEAD, trusted author, dev target, supplied base SHA, and merge base exactly match the requested review coordinates.
- Reviewed restart 429 parsing, retryAt and Retry-After normalization, cooldown state reset, the profile cooldown modal, and every useMultiPhaseKycFlow payment consumer.
- Reviewed avatar dealing, staged confirmation, duplicate-submit prevention, device-local letter fallback, responsive sizing, reduced motion, haptic cleanup, and drawer lifecycle.
- Reviewed profile-link formatting, About ordering, full-name toggle placement, payment-row wrapping and status treatments, the unlimited Peanut-to-Peanut bar, and pull-to-refresh overlay isolation.
- Checked the live canonical Lexicon and all four resolved locale catalogs; Payment Channels is not defined in the Lexicon, so the PR description is the available naming intent rather than an invented Lexicon definition.
- Exact-head required CI is green: unit, typecheck, eslint, format, native export, design-system lint, workflow analysis, and deployment passed. Advisory ds-shots failed during next build with its log ending inside the webpack bundle and no actionable test or compile diagnostic; the immediately preceding PR head passed that job.
- git diff --check passed; the detached worktree has no installed dependencies, so no additional local Jest or build run was available.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 5bf6420533e0 · Context: repo, product · Took 22m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The supplied P1-P9 findings are fixed, and the avatar, profile, locale, and pull-to-refresh changes are otherwise sound. One minor cooldown-dismissal bug remains in payment flows whose restart prompt stays mounted.
Findings
- MINOR · src/components/Kyc/SumsubKycModals.tsx:21 · Dismiss the cooldown back to the payment screen
When QR Pay is in PROVIDER_RESTART_IDENTITY, its parent ActionModal remains hard-coded visible while this line mounts the cooldown as a second dialog. Tapping ‘I’ll try later’ only clears errorCooldown, so the underlying restart dialog immediately takes over and still offers the blocked restart; the bank modal/page hosts likewise keep their visibility state or verify step active. Make cooldown dismissal also close/reset the host initiation surface (or pass a host onCooldownClose callback and suppress direct restart dialogs) so only one dialog is mounted and the dismiss action actually returns to the payment screen.
Checked clean
- Confirmed the detached worktree HEAD and merge base exactly match the supplied head and base SHAs.
- Rechecked P1: pull-to-refresh now ignores only open or aria-modal dialogs, covers the always-mounted closed support drawer, and cancels gestures when a dialog opens mid-pull.
- Rechecked P2-P8: delete-account tests use the sentence-case label, and Payment Channels plus exchange-rate wording is consistent across en, es-419, es-AR, and pt-BR.
- Rechecked P9: restart cooldown state and the dated shared renderer now reach every payment-flow host; the remaining dismissal behavior is reported separately.
- Reviewed avatar selection, random dealing, duplicate-art exclusion, save serialization, local letter fallback, cancellation, reduced motion, haptics, and focused tests.
- Reviewed the profile header, About ordering, name-visibility toggle, payment-channel rows, limit-bar presentation, and their focused tests.
- Required exact-head CI is green. Advisory ds-shots failed inside its duplicate Next build with no actionable diagnostic, while native-export and the aggregate required CI passed.
- Checked the live Peanut Lexicon; it does not define these menu-label terms, so their wording was assessed against the trusted PR scope and locale consistency rather than an invented definition.
- Local Jest and TypeScript binaries are absent from the detached worktree, so focused tests were verified through exact-head CI rather than rerun locally.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 99bfe77e23d3 · Context: repo, product · Took 24m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No actionable defects found at the pinned head. All ten supplied prior findings are fixed in code, tests, or locale catalogs.
Checked clean
- P1: pull-to-refresh now matches only open dialog states, cancels interrupted gestures, and retains coverage for the closed always-mounted support dialog.
- P2: every delete-account interaction test now targets the sentence-case label.
- P3-P8: Payment Channels and exchange-rate labels now agree across English, Spanish, Argentine Spanish inheritance, and Brazilian Portuguese.
- P9-P10: restart cooldowns render through the shared KYC modal host in payment flows, suppress the underlying initiation prompt, and dismiss back to the payment screen; the sibling API contract supplies the sanitized retryAt field.
- Avatar picker state, explicit save behavior, art deduplication, responsive square deals, keyboard navigation, reduced motion, cancellation, and haptic cleanup were traced without finding a reachable regression.
- Profile share-pill formatting, About-page ordering, full-name toggle presentation, payment-row wrapping, unavailable-card status, and unlimited P2P bar were checked against the stated PR behavior.
- Exact-head unit, eslint, typecheck, format, native-export, analyze, required ci-success, and deployment checks passed. Advisory ds-shots failed during its build after compilation and before screenshots; the separate build-bearing checks passed.
- A local focused Jest run was unavailable because the detached worktree has no node_modules; exact-head CI unit coverage passed instead.
- The canonical Lexicon was checked for the changed product terminology; it defines rails and related payment concepts but does not define Payment Channels, while the trusted PR description explicitly requires that UI label.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 36acbdd7670a · Context: repo, product, sibling · Took 22m
Avatar selection now starts with a horizontal initials row and an explicit confirmation. Rolling the dice plays a full-screen CSS animation with haptics, then offers a unique 3×3 or 4×4 avatar grid sized for the screen. The flow selectively incorporates the avatar-dealing logic from #3014 and uses existing app colors and components without adding libraries.
Latest dev is merged, including its long-press callout guard, drawer body drag handling, and shadow-padding fixes. Those fixes remain intact; the avatar layout is reconciled with the updated shared drawer padding.
Backend companion: https://github.com/peanutprotocol/peanut-api-ts/pull/1543. It allows reopening unfinished uploads without another reset cooldown and advances the longer cooldown on completed provider decisions.
Validation: 148 tests passed across 14 focused suites; UI TypeScript check and diff whitespace check passed. Native device checks remain for long-press behavior, drawer gestures, VoiceOver/TalkBack, and haptics. No deployment performed.