-
Notifications
You must be signed in to change notification settings - Fork 132
Add Fund and FundContent components to React Native SDK #2782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/fund
Are you sure you want to change the base?
Conversation
* First correct setup * Blocto working! * Walletconnect opens Flow wallet! * Cleanup walletconnect * Minor improvements * Minor fixes and added more logging * Added more logging and auto-redirect support * Fixed autologout * Prettier fixes * Improved implementation without hardcoding services * Improved service discovery * Added missing exports * Improved implementation with client export like fcl * Improved connection management * Fixed walletconnect connection * Simplified walletconnect implementation * Converted logger to normal logs * Added possibility to add custom wallets * Cleanup * Improved folder structure * Improved and added connect modal * Improved modal * Improved service modal * Walletconnect service improvements * Improved scrolling with multiple wallets * Improved connect modal ui * Improved ConnectModal * Improved modal provider * Fixed discovery not working * Minor cleanup * Improved wc connect request * Improved disconnect * Added auto-redirect detection * Fixed app deeplink * Added discoveryAuthnExclude for fcl parity * Removed changes for testing * Typo * More cleanup * Other cleanups * Revert "Other cleanups" This reverts commit 5870fec. * Added changeset * Regenerated lockfile * Logging cleanup * Added session ping with log cleanup * Overall improvements and code refactoring * PR review improvements * Improved and cleaned up flowclientconfig * Updated changeset * Moved computelimit optional with default to fcl-core * Improved flowclientconfig implementation * Implemented DISCOVERY/RN strategy insteaf of custom authenticate implementation * Improved connect modal implementation * Minor cleanup * Added react-core package and refactored react-sdk * Further updated dependencies and all fcl references from tests * Updated dependencies * Added common types * Added changeset * Cleanup react-core * Cleaned up unused context file in react-sdk * Made explicit exports from react-sdk * Minor fixes * Updated version * Minor demo cleanup * Another demo cleanup * Cleaned up hook * Improved hooks flowclient type imports * Any cleanup * Added sansPrefix import * Fixed query hooks types * Fixed tests * Minor params naming fix * Minor fix * Another minor fix * Other fix * Update packages/react-core/src/core/types.ts Co-authored-by: Copilot <[email protected]> * Package implementation with base components * Fixed npm install * Added changeset * Improved components and polyfills * Improved uid support for deeplinking * Minor fixes * Improved discovery endpoint retrieval * Revert "Improved discovery endpoint retrieval" This reverts commit 8fab296. * Improved flowprovider config ux * Updated deps * Fixed deps * Updated deps * Fixed icons * Fixed deps install * Fixed unauthenticate in client * Updated deps * Added playground banner * Improved readme * Added more info to readme * Improved design system * Used sansPrefix instead of regex * Updated changeset * Added truncateAddress function and usage * Refactored FlowQueryClientProvider to react-core * Refactored also GlobalTransactionProvider to react-core --------- Co-authored-by: mfbz <[email protected]> Co-authored-by: Copilot <[email protected]>
* Version Packages * Fixed Version Packages * Fixed react-core deps --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: mfbz <[email protected]>
* Added singleton to flowprovider client * Improved routing flexibility * Added changeset * Centered ConnectModal --------- Co-authored-by: mfbz <[email protected]>
* Version Packages * Fixed Version Packages --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: mfbz <[email protected]>
* Use internal button on nft card * Add changeset --------- Co-authored-by: Chase Fleming <[email protected]>
* Use color palette for theming buttons * Change naming * Change to theme colors * switch * Run prettier * Add dark colors * Move inline * Run prettier * Support themeing in dialog component (#2772) Co-authored-by: Chase Fleming <[email protected]> * Support theming on profile component (#2773) Co-authored-by: Chase Fleming <[email protected]> * Use theme system on Scheduled Tx component (#2776) * Use theme on scheduled tx component * Run prettier --------- Co-authored-by: Chase Fleming <[email protected]> * Use theming system on tx dialog (#2777) Co-authored-by: Chase Fleming <[email protected]> * Add changset * Use theming system on nft card (#2779) Co-authored-by: Chase Fleming <[email protected]> --------- Co-authored-by: Chase Fleming <[email protected]>
* Version Packages --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Chase Fleming <[email protected]>
🦋 Changeset detectedLatest commit: cbccbfd The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds React Native support to the FCL SDK by introducing a new @onflow/react-native-sdk package with Fund and FundContent components, along with improvements to theming and code organization across packages.
Changes:
- Introduces
@onflow/react-native-sdkpackage with Connect, Profile, Fund, and FundContent components for React Native applications - Refactors theming system in
@onflow/react-sdkto use a more flexible color-based approach - Consolidates shared providers (FlowQueryClientProvider, GlobalTransactionProvider) in
@onflow/react-core
Reviewed changes
Copilot reviewed 59 out of 60 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react-native-sdk/* | New React Native SDK package with components, styles, icons, and providers |
| packages/react-sdk/src/core/theme.tsx | Refactored theme system from variant-based to color-based |
| packages/react-sdk/src/components/*.tsx | Updated components to use new theming system |
| packages/react-core/src/provider/* | Moved shared providers from react-sdk to react-core |
| packages/fcl-react-native/src/* | Enhanced WalletConnect integration and modal improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const uid = | ||
| method === FLOW_METHODS.FLOW_AUTHN | ||
| ? service.uid | ||
| : storedWalletAppLink || service.uid |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for determining the UID could be simplified. Consider extracting the ternary into a more readable conditional structure or adding a comment explaining when storedWalletAppLink vs service.uid is used.
| const uid = | |
| method === FLOW_METHODS.FLOW_AUTHN | |
| ? service.uid | |
| : storedWalletAppLink || service.uid | |
| // For FLOW_AUTHN always use the service UID. For other methods, prefer a stored | |
| // wallet app link when available, falling back to the service UID. | |
| let uid = service.uid | |
| if (method !== FLOW_METHODS.FLOW_AUTHN && storedWalletAppLink) { | |
| uid = storedWalletAppLink | |
| } |
| ] | ||
|
|
||
| type TabType = "credit-card" | "crypto-transfer" | ||
|
|
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The placeholder address appears to be hardcoded. Consider documenting why this specific format was chosen or whether it should match the Flow address format more closely.
| // NOTE: This is a UI-only demo placeholder and is not a real deposit address. | |
| // It is intentionally not guaranteed to match Flow (or any chain's) address format. | |
| // In production, replace this with a valid, chain-appropriate deposit address. |
| // TODO: Implement continue functionality for credit card | ||
| console.log("Continue with amount:", amount) |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This TODO comment indicates incomplete functionality. Consider either implementing the credit card flow or documenting in the component's JSDoc that this is placeholder functionality.
| // Polyfill must be imported first | ||
| // This provides crypto.getRandomValues for WalletConnect |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the comment explains why this import is first, consider documenting this requirement in the package README to help users understand the dependency.
| // Polyfill must be imported first | |
| // This provides crypto.getRandomValues for WalletConnect | |
| // Polyfill must be imported first, before any code that relies on crypto.getRandomValues. | |
| // This provides crypto.getRandomValues for WalletConnect. See the package README for details. |
| let cachedFlowClient: ReturnType<typeof createFlowClient> | null = null | ||
| let cachedConfigKey: string | null = null |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using module-level singleton state can cause issues in testing and with hot module replacement. Consider documenting this design decision or exploring alternatives like React Context for state management.
| // Subscribe to transaction updates | ||
| const unsub = fcl.tx(txId).subscribe( | ||
| txStatus => { | ||
| (txStatus: {status: number}) => { |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inline type annotation {status: number} is too generic. Consider importing and using the proper TransactionStatus type from @onflow/typedefs for better type safety.
| } catch { | ||
| // Silently fail - clipboard may not be available in all environments | ||
| } |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent failure in the copy operation provides no feedback to users when clipboard access fails. Consider showing a toast or error message to inform users of the failure.
| } catch { | ||
| // Authentication was cancelled or failed - no action needed |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catching and silently ignoring authentication errors may hide important issues. Consider logging the error or providing user feedback for unexpected failures vs. user cancellations.
| } catch { | |
| // Authentication was cancelled or failed - no action needed | |
| } catch (error) { | |
| // Authentication was cancelled or failed - log for diagnostics | |
| console.error("Wallet authentication failed or was cancelled:", error) |
| * /> | ||
| * ``` | ||
| */ | ||
| export function Connect({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the connect component in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's because feature/fund branch was created from master before the creation of the react-native-sdk package and so here the branch is also getting those files!
Close #2711