feat(rn_cli_wallet): per-option collectData for WalletConnect Pay#387
feat(rn_cli_wallet): per-option collectData for WalletConnect Pay#387ganchoradkov wants to merge 4 commits intomainfrom
Conversation
The getPaymentOptions API now returns collectData on each option rather than only at the top level. This updates the payment flow to: - Show the payment options list immediately after scanning a QR - Display an "Info required" pill on options that need collectData - Change the pay button to "Next" when the selected option needs data - Load the per-option collectData URL in WebView on "Next" tap - Show only the completed option after returning from the WebView - Add "Why info required?" explainer accessible from the header - Simplify the options list to a flat layout (remove collapsible select) Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @ganchoradkov's task in 2m 28s —— View job Code Review CompletePR Size: 11 files, 274 additions, 277 deletions ✅ Within acceptable range Found 4 issue(s)Issue 1: WebView originWhitelist doesn't validate final URL matches allowed originsID: collectdata-origin-bypass-a8c4 Context:
Recommendation: Validate URL origin before constructing finalUrl: const finalUrl = useMemo(() => {
const allowedOrigins = ['https://dev.pay.walletconnect.com', 'https://staging.pay.walletconnect.com', 'https://pay.walletconnect.com'];
const urlOrigin = getBaseUrl(url);
if (!allowedOrigins.includes(urlOrigin)) {
LogStore.error('Invalid collectData URL origin', 'CollectDataWebView', 'finalUrl', { url });
onError('Invalid payment form URL');
return '';
}
return buildUrlWithPrefill(url, themeMode);
}, [url, themeMode, onError]);Issue 2: Hardcoded PII in WebView prefill dataID: collectdata-hardcoded-pii-7f2e Context:
Recommendation: For example app, either remove prefill or make it configurable: // Option 1: Remove prefill for production
const PREFILL_DATA = __DEV__ ? { fullName: 'John Doe', dob: '1990-06-15' } : {};
// Option 2: Make it a debug setting
const PREFILL_DATA = SettingsStore.state.enablePrefillDebug
? { fullName: 'John Doe', dob: '1990-06-15' }
: {};Issue 3: Missing error boundary for InfoExplainerView renderID: index-explainer-error-boundary-3b8f Context:
Recommendation: Wrap in React error boundary or add defensive null check in InfoExplainerView. For example app, low priority but good practice: {showInfoExplainer && (
<ErrorBoundary fallback={<View />}>
<InfoExplainerView onDismiss={() => setShowInfoExplainer(false)} />
</ErrorBoundary>
)}Issue 4: Non-strict equality check could allow falsy bypassID: index-needs-collect-logic-9c2a Context:
Recommendation: Add explicit string length check: const selectedNeedsCollectData = !!(
snap.selectedOption &&
(snap.selectedOption as PaymentOptionWithCollectData).collectData?.url?.trim() &&
!snap.collectDataCompletedIds.includes(snap.selectedOption.id)
);SummaryReviewed per-option collectData refactor. Main concern: WebView security - Critical Action Required: Validate collectData URL origin against whitelist before constructing finalUrl to prevent phishing attacks. Positive Notes:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ); | ||
|
|
||
| const renderContent = useCallback(() => { | ||
| if (showInfoExplainer) { |
There was a problem hiding this comment.
🤖 Auto Review Issue: Missing error boundary for InfoExplainerView render
Severity: LOW
Category: code_quality
Tool: Claude Auto Review
Context:
- Pattern:
showInfoExplainerstate at line 26 conditionally renders InfoExplainerView without error boundary - Risk: If InfoExplainerView throws (rare), entire modal crashes instead of graceful fallback
- Impact: Modal becomes unresponsive, user must restart app
- Trigger: Unexpected error in InfoExplainerView (e.g., theme value missing)
Recommendation: Wrap in React error boundary or add defensive null check in InfoExplainerView. For example app, low priority but good practice:
{showInfoExplainer && (
<ErrorBoundary fallback={<View />}>
<InfoExplainerView onDismiss={() => setShowInfoExplainer(false)} />
</ErrorBoundary>
)}| } | ||
| }, [snap.step, snap.paymentOptions?.options, snap.selectedOption, onSelectOption]); | ||
|
|
||
| const selectedNeedsCollectData = !!( |
There was a problem hiding this comment.
🤖 Auto Review Issue: Non-strict equality check could allow falsy bypass
Severity: LOW
Category: code_quality
Tool: Claude Auto Review
Context:
- Pattern:
selectedNeedsCollectDataat lines 142-146 uses!!coercion but doesn't validate URL is non-empty string - Risk: If API returns
collectData: { url: '' }(empty string),!!''is falsy but URL is technically present - Impact: Empty URL would skip collectData flow, potentially breaking payment if server expects it
- Trigger: Malformed API response with empty string collectData.url
Recommendation: Add explicit string length check:
const selectedNeedsCollectData = !!(
snap.selectedOption &&
(snap.selectedOption as PaymentOptionWithCollectData).collectData?.url?.trim() &&
!snap.collectDataCompletedIds.includes(snap.selectedOption.id)
);There was a problem hiding this comment.
Pull request overview
This PR implements a per-option collectData flow for WalletConnect Pay, improving the user experience by showing payment options immediately after scanning and handling data collection on a per-option basis rather than globally.
Changes:
- Added per-option collectData tracking with
collectDataCompletedIdsstate in PaymentStore - Replaced collapsible "Pay with" dropdown with a flat list of payment options showing "Info required" pills
- Added an info explainer modal accessible via a "Why info required?" button
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
wallets/rn_cli_wallet/yarn.lock |
Updated @walletconnect/pay and @walletconnect/react-native-compat to canary versions with per-option collectData support |
wallets/rn_cli_wallet/package.json |
Added @walletconnect/pay canary version to resolutions |
wallets/rn_cli_wallet/ios/Podfile.lock |
Updated iOS dependencies to match new package versions |
wallets/rn_cli_wallet/src/utils/TypesUtil.ts |
Added PaymentOptionWithCollectData interface extending PaymentOption with optional collectData field |
wallets/rn_cli_wallet/src/store/PaymentStore.ts |
Added collectDataCompletedIds tracking and methods to mark/check completion status |
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/index.tsx |
Refactored flow to support per-option collectData with info explainer modal |
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/ViewWrapper.tsx |
Added headerLeftContent prop for custom left header content and bottom safe area padding |
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/ConfirmPaymentView.tsx |
Simplified UI by removing collapsible dropdown and showing flat list with "Info required" pills |
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/InfoExplainerView.tsx |
New component explaining why user information is required |
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/ResultView.tsx |
Added bottom margin to footer for proper spacing |
wallets/rn_cli_wallet/src/modals/PaymentOptionsModal/CollectDataWebView.tsx |
Added viewport JavaScript injection to fit form content without scrolling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
Summary
getPaymentOptionsAPI now returnscollectDataon each option. The payment modal shows options immediately after scanning, with an "Info required" pill on options needing data collection. Selecting such an option changes the button to "Next" which opens the per-option WebView.@walletconnect/react-native-compatand@walletconnect/payto canary versions with per-option collectData support.flowchart TD A[Scan QR] --> B[Loading] B --> C[Confirm - show all options] C -->|Option needs collectData| D{Button: Next} C -->|Option has no collectData| E{Button: Pay} D -->|Tap Next| F[CollectData WebView] F -->|Complete| G[Confirm - show only completed option] G --> E E -->|Tap Pay| H[Confirming] H --> I[Result] C -->|Why info required?| J[Info Explainer] J -->|Got it!| CTest plan
collectDataMade with Cursor