Skip to content

feat(rn_cli_wallet): redesign payment options UI with per-option fee estimates#480

Draft
ignaciosantise wants to merge 14 commits intomainfrom
chore/review-staged-ui
Draft

feat(rn_cli_wallet): redesign payment options UI with per-option fee estimates#480
ignaciosantise wants to merge 14 commits intomainfrom
chore/review-staged-ui

Conversation

@ignaciosantise
Copy link
Copy Markdown
Collaborator

Summary

Refactor the WalletConnect Pay flow in rn_cli_wallet so each payment option shows its gas/approval fee upfront, the review and select screens share UI primitives, and the previously paid token is remembered across sessions.

  • Per-option fee estimates: PaymentStore now fetches required actions and estimates approval/transaction fees for each option in parallel as soon as the modal opens, instead of doing it sequentially after the user picks one.
  • Shared OptionItem component: select and review screens now render the same row, including chain badge, token amount, fee shimmer, and a right-side action slot (info / pencil).
  • New GasFeeView: tapping the gas-pump info button on a Permit2 option opens a dedicated explainer with the estimated fee.
  • Persisted last-paid token: PaymentStore writes the chosen amount.unit to MMKV (PAY_LAST_TOKEN_UNIT) and uses it to pre-select the same token on the next payment. Cleared on wallet import.
  • New visual primitives: Shimmer (Reanimated-driven), GasPump, Info, Pencil, SelectToken icons; Text gains an underline prop.
  • Tests: new PaymentStore.test.ts (615 lines) covering option-fee loading, persistence and confirm flow; existing PaymentUtil.test.ts updated.

Flow

flowchart TD
    A[QR scanned] --> B[PaymentStore.loadPaymentOptions]
    B --> C[Fetch options]
    C --> D[Parallel per-option<br/>getRequiredPaymentActions<br/>+ estimateTransactionFee]
    D --> E{Last paid<br/>token unit?}
    E -- yes --> F[Pre-select matching option]
    E -- no  --> G[Show SelectOptionView]
    F --> H[ReviewPaymentView<br/>shared OptionItem]
    G --> H
    H -- info tap --> I[GasFeeView]
    H -- pay --> J[confirmPayment + wait]
    J --> K[Persist last unit on success]
    K --> L[ResultView]
Loading

Test plan

  • yarn jest in wallets/rn_cli_wallet (PaymentStore + PaymentUtil suites pass locally)
  • Scan a Pay QR with multiple options and verify each row shows its gas estimate after a brief shimmer
  • Confirm a Permit2 payment, reopen Pay, and confirm the same token is pre-selected
  • Import a different wallet and verify the pre-selection clears
  • Tap the gas-pump info button and verify the GasFeeView explainer shows the estimate

🤖 Generated with Claude Code

…estimates

Refactor PaymentOptionsModal to estimate gas/approval fees per option
upfront, share a new OptionItem component across select and review
views, and add a dedicated gas fee explainer. Persist the last paid
token unit via MMKV so it appears first on subsequent payments, and
clear it on wallet import. Add Shimmer loading component, GasPump,
Info, Pencil and SelectToken icons. Add PaymentStore unit tests.
Copilot AI review requested due to automatic review settings April 29, 2026 14:44
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 29, 2026

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

Project Deployment Actions Updated (UTC)
pos-demo Ready Ready Preview, Comment May 7, 2026 10:33am

Request Review

@claude
Copy link
Copy Markdown

claude Bot commented Apr 29, 2026

Claude finished @ignaciosantise's task in 58s —— View job


Code Review

🚨 PR Too Large
Files: 32 Lines: 2,622 Severity: HIGH Category: maintainability

This PR packages a complete store refactor, new shared components, new assets, and new tests in one commit. Suggested splits:

  • PR 1: PaymentTransactionUtil.ts + PaymentStore.ts store logic (fee preloading, persistence, sequence guards)
  • PR 2: New visual primitives — Shimmer, OptionItem, GasPump/Info/Pencil/SelectToken icons, Text.underline
  • PR 3: Modal screen rewrites — SelectOptionView, ReviewPaymentView, GasFeeView, index.tsx orchestration
  • PR 4: Tests (PaymentStore.test.ts, PaymentUtil.test.ts)

Found 5 issue(s)

Issue 1: Hardcoded token name "USDT" in gas-fee explainer link

ID: reviewpaymentview-hardcoded-usdt-b3a1
File: wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/ReviewPaymentView.tsx:108
Severity: HIGH
Category: code_quality

Context:

  • Pattern: The tappable explainer text is a string literal "Why does USDT require a gas fee?" regardless of the selected option's actual symbol.
  • Risk: Every token other than USDT (USDC, DAI, etc.) will show "USDT" in the gas-fee explainer.
  • Impact: Confusing and incorrect UX for non-USDT options; the GasFeeView title correctly uses tokenNameUppercase (dynamic), so there is a visible inconsistency between the link text and the modal it opens.
  • Trigger: Any requiresApproval option whose symbol is not USDT.

Recommendation:

// Use selectedOption.amount.display.assetSymbol instead of the literal
Why does {selectedOption.amount.display.assetSymbol.toUpperCase()} require a gas fee?

Fix this →


Issue 2: Shimmer memo comparison always returns true — component never re-renders

ID: shimmer-memo-never-rerender-c2f4
File: wallets/rn_cli_wallet/src/components/Shimmer.tsx:120
Severity: MEDIUM
Category: code_quality

Context:

  • Pattern: export const Shimmer = memo(Shimmer_, () => true) — the areEqual function unconditionally returns true, meaning React considers props perpetually unchanged.
  • Risk: The component is frozen after first mount. backgroundColor / foregroundColor are sourced from useTheme() inside the component, but since memo prevents re-renders, dark/light mode switches will not update the shimmer's colors. Width, height, duration, and borderRadius prop changes from parent are also silently swallowed.
  • Impact: Visual glitch on theme switch; potential stale sizing if a parent conditionally changes Shimmer dimensions.
  • Trigger: Dark/light mode toggle while a Shimmer is mounted.

Recommendation: Remove the custom comparison to use React's default shallow equality, or omit memo entirely since Shimmer_ already has stable identity via useSharedValue:

// Before
export const Shimmer = memo(Shimmer_, () => true);

// After — use default shallow prop comparison
export const Shimmer = memo(Shimmer_);

Fix this →


Issue 3: Hardcoded PII placeholder logged to application log store

ID: dcl-CollectDataWebView-pii-hardcoded-prefill-a3f1
File: wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/CollectDataWebView.tsx:66
Severity: MEDIUM
Category: data_classification

Context:

  • Pattern: PREFILL_DATA = { fullName: 'John Doe', dob: '1990-06-15', pobAddress: 'Buenos Aires' } is Base64-encoded and appended to the KYC WebView URL on every mount. The full constructed URL (including the encoded PII blob) is then written to LogStore at line 114–117 (originalUrl + finalUrl).
  • Risk: Even though this is a demo wallet with placeholder data, the pattern establishes that PII-laden URLs flow through LogStore. If this code is ever adapted with real user data (DOB, address from secure storage), the logging call will silently exfiltrate it.
  • Impact: Base64 is not encryption — any log consumer can trivially decode it. The finalUrl log entry persists the full PII payload alongside application logs.
  • Trigger: Any time the collectData step is reached.

Recommendation:

// Don't log the full URL containing prefill data
LogStore.log('WebView URL with prefill', 'CollectDataWebView', 'finalUrl', {
  originalUrl: url,
  // Remove finalUrl — it contains the encoded prefill blob
  prefillApplied: true,
});

Fix this →


Issue 4: SelectOptionViewselectedOption prop declared but never consumed

ID: selectoptionview-unused-prop-d5c3
File: wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/SelectOptionView.tsx:18
Severity: LOW
Category: code_quality

Context:

  • Pattern: SelectOptionViewProps declares selectedOption: PaymentOption | null but the component destructuring at line 32 omits it — the prop is never read.
  • Risk: Callers pass the value with no effect; it won't be highlighted by TypeScript since it's declared in the interface as a valid prop.
  • Impact: Dead interface surface; may confuse future maintainers who expect selection state to be used for highlighting the pre-selected row.
  • Trigger: Anytime SelectOptionView is rendered.

Recommendation: Remove selectedOption from SelectOptionViewProps (and its corresponding prop pass in index.tsx) or implement the highlighting behaviour it was presumably intended to drive.


Issue 5: getPaymentContext removed — callers outside this PR will break

ID: brk-paymentutil-remove-getpaymentcontext-a3f2
File: wallets/rn_cli_wallet/src/utils/PaymentUtil.ts
Severity: LOW
Category: breaking_change

Context:

  • Pattern: Named export getPaymentContext (and PaymentContext type) was removed; replaced by getApprovalAction / requiresApproval. The PR updates the only known consumer (PaymentUtil.test.ts).
  • Risk: Any code on other branches or forks that calls getPaymentContext({ paymentActions: ... }) will get a module-not-found compile error. The calling convention (object wrapper) is not directly compatible with the new functions.
  • Impact: Contained to this single-repo example wallet — no published package surface.
  • Trigger: Import of getPaymentContext from @/utils/PaymentUtil.

Recommendation: Confirm no other branches or open PRs import getPaymentContext before merging. If any are found, add a brief shim: export const getPaymentContext = ... delegating to the two new helpers.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the rn_cli_wallet WalletConnect Pay modal to show per-option fee estimates up front, reuse a shared option row UI across select/review screens, and persist the last-used payment token for faster repeat payments.

Changes:

  • Preload per-option approval fee estimates in parallel on modal open and expose them via PaymentStore.
  • Redesign select/review UI around a shared OptionItem, add GasFeeView, and enhance loading text (message + note).
  • Persist and clear “last paid token unit” using MMKV-backed storage; add/expand unit tests for the new behavior.

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
wallets/rn_cli_wallet/src/utils/TypesUtil.ts Adds gasFee to the Pay flow step union.
wallets/rn_cli_wallet/src/utils/PaymentUtil.ts Splits approval detection into getApprovalAction + requiresApproval.
wallets/rn_cli_wallet/src/utils/PaymentTransactionUtil.ts Adds fiat-aware fee estimate struct and native-token price fetching with caching.
wallets/rn_cli_wallet/src/store/PaymentStore.ts Implements per-option fee estimate preloading + last-token persistence; updates confirm flow.
wallets/rn_cli_wallet/src/services/ERC20BalanceService.ts Formatting-only changes to improve readability.
wallets/rn_cli_wallet/src/screens/Settings/index.tsx Minor JSX formatting simplification.
wallets/rn_cli_wallet/src/screens/SecretPhrase/index.tsx Minor formatting change for snapshot destructuring.
wallets/rn_cli_wallet/src/screens/LogList/components/LogFilterSheet.tsx Minor formatting and adds lint suppression comment for inline style.
wallets/rn_cli_wallet/src/modals/SessionProposalModal.tsx Import formatting for Canton constants.
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/index.tsx Reworks step-resolution logic, wires new views/props, adds gasFee step routing.
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/SelectOptionView.tsx Replaces custom row with shared OptionItem, shows per-option fee shimmer/estimate.
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/ReviewPaymentView.tsx Uses OptionItem, updates pay CTA text, adds gas-fee explainer link.
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/ResultView.tsx Minor formatting changes to helper functions.
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/LoadingView.tsx Adds optional note under the loading message.
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/InfoExplainerView.tsx Centers content and aligns layout.
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/GasFeeView.tsx New explainer screen for approval gas fees.
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/ExpiryWarningView.tsx Minor formatting cleanup.
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/CollectDataWebView.tsx Trailing comma formatting fix.
wallets/rn_cli_wallet/src/modals/ImportWalletModal.tsx Clears persisted last-paid token on wallet import.
wallets/rn_cli_wallet/src/hooks/useNfc.ts Minor formatting change for return type.
wallets/rn_cli_wallet/src/constants/Canton.ts Formatting for chain constant object spread.
wallets/rn_cli_wallet/src/components/Text.tsx Adds underline prop + style support.
wallets/rn_cli_wallet/src/components/Shimmer.tsx New Reanimated-based shimmer primitive.
wallets/rn_cli_wallet/src/components/OptionItem.tsx New shared option row component used by select/review screens.
wallets/rn_cli_wallet/src/components/FadeGradient.tsx Minor formatting change for SVG gradient props.
wallets/rn_cli_wallet/src/assets/SelectToken.tsx New icon asset for select-token header.
wallets/rn_cli_wallet/src/assets/QuestionMark.tsx Removes old QuestionMark icon asset (no longer used).
wallets/rn_cli_wallet/src/assets/Pencil.tsx New pencil icon asset for “change option” affordance.
wallets/rn_cli_wallet/src/assets/Info.tsx New info icon asset for collect-data hint.
wallets/rn_cli_wallet/src/assets/GasPump.tsx New gas-pump icon asset for fee UI.
wallets/rn_cli_wallet/tests/PaymentUtil.test.ts Updates tests for new approval helper functions.
wallets/rn_cli_wallet/tests/PaymentStore.test.ts Adds comprehensive tests for fee preloading + persistence + confirm flow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/ReviewPaymentView.tsx Outdated
Comment thread wallets/rn_cli_wallet/src/components/Shimmer.tsx
Comment thread wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/SelectOptionView.tsx Outdated
@ignaciosantise ignaciosantise marked this pull request as draft April 29, 2026 15:08
Gate the "(incl. gas fee)" badge on whether the fee was actually combined
into the total, and use the selected option's token symbol in the gas-fee
explainer link instead of a hardcoded "USDT". Also extracts
OptionFeeEstimateStatus into TypesUtil and drops the unused selectedOption
prop on SelectOptionView.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The message container had a fixed minHeight of 48 with overflow:hidden,
but its child uses absoluteFill, so it couldn't grow to fit a wrapped
message plus the note. Bump the base minHeight and apply a taller
variant when a note is present.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

2 participants