feat: support intent URL to start a new WalletConnect session #271
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…h the Linking library
manu0466
added
Type: Enhancement
WalkthroughThe application underwent significant updates to enhance its deep linking capabilities, wallet connection management, and UI/UX improvements. Key changes include the addition of new URI schemes for better inter-app communication, refactoring of hooks and components to support new functionalities like handling wallet connect actions and URI actions more efficiently, and updates to the navigation and theme management to provide a smoother user experience. These changes collectively aim to make the app more intuitive, responsive, and integrated with external services. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
SonarQube Quality Gate Result
updated: 1/31/2024, 14:07:45 (UTC+0) |
manu0466
force-pushed
the
manuel/DPM-190/wallet-connect-intent-uri
branch
from
91e431d to
6b8225e
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #271 +/- ##
==========================================
- Coverage 80.87% 79.19% -1.69%
==========================================
Files 124 124
Lines 481 495 +14
Branches 63 64 +1
==========================================
+ Hits 389 392 +3
- Misses 92 103 +11 ☔ View full report in Codecov by Sentry. |
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
android/app/src/main/AndroidManifest.xmlis excluded by:!**/*.xmlsrc/assets/locales/en/walletConnect.jsonis excluded by:!**/*.json
Files selected for processing (24)
- ios/DesmosProfileManager/Info.plist (1 hunks)
- src/App.tsx (2 hunks)
- src/contexts/GraphQLClientProvider.tsx (1 hunks)
- src/contexts/ThemeProvider.tsx (2 hunks)
- src/hooks/analytics/useTrackWalletConnectSessionEstablished.ts (2 hunks)
- src/hooks/uriactions/useHandleUriAction.tsx (4 hunks)
- src/hooks/uriactions/useInitLinkingUriActions.ts (1 hunks)
- src/hooks/useHandleReceivedActions.ts (1 hunks)
- src/hooks/useReturnToCurrentScreen.ts (2 hunks)
- src/hooks/walletconnect/useInitWalletConnectLogic.ts (1 hunks)
- src/hooks/walletconnect/useWalletConnectApproveSessionProposal.ts (3 hunks)
- src/hooks/walletconnect/useWalletConnectOnSessionRequest.ts (4 hunks)
- src/lib/UriActions/caching.ts (2 hunks)
- src/lib/UriActions/parsing.ts (1 hunks)
- src/modals/LoadingModal/useStyles.ts (1 hunks)
- src/navigation/RootNavigator/index.tsx (2 hunks)
- src/recoil/uriaction.ts (1 hunks)
- src/screens/DevScreen/index.tsx (3 hunks)
- src/screens/UnlockApplication/index.tsx (5 hunks)
- src/screens/WalletConnectRequest/index.tsx (3 hunks)
- src/screens/WalletConnectRequest/useHooks.ts (2 hunks)
- src/screens/WalletConnectSessionProposal/index.tsx (1 hunks)
- src/types/uri.ts (2 hunks)
- src/types/walletConnect.ts (1 hunks)
Files skipped from review due to trivial changes (4)
- src/hooks/useReturnToCurrentScreen.ts
- src/hooks/walletconnect/useInitWalletConnectLogic.ts
- src/hooks/walletconnect/useWalletConnectOnSessionRequest.ts
- src/modals/LoadingModal/useStyles.ts
Additional comments: 28
src/contexts/GraphQLClientProvider.tsx (1)
- 5-5: The modification to
GraphQLClientProviderto usePropsWithChildrenis correct and aligns with React's type definitions for components that accept children. This change enhances type safety and readability.src/hooks/analytics/useTrackWalletConnectSessionEstablished.ts (1)
- 13-17: The change to accept
appNameandnamespacesdirectly instead of asessionobject simplifies the hook's interface and focuses on the required data, improving the hook's usability and maintainability.src/recoil/uriaction.ts (1)
- 8-11: The introduction of
uriActionAtomto manage URI actions within the application's state using Recoil is a structured approach to state management, ensuring a centralized and reactive way to handle URI actions.src/hooks/uriactions/useInitLinkingUriActions.ts (1)
- 9-31: The implementation of
useInitLinkingUriActionsto handle initial and background URI actions using React Native's Linking API is well-structured. It ensures that URI actions are processed both at app launch and when the app is brought to the foreground, enhancing the app's responsiveness to external triggers.src/lib/UriActions/caching.ts (2)
- 6-15: The addition of
NEW_ACTION_EVENTand the use ofEventEmitterto notify subscribers about new URI actions is a robust pattern for event-driven updates, improving the modularity and decoupling of components that need to react to new URI actions.- 40-44: Implementing
onCachedUriActionChangeto allow subscription to URI action changes usingEventEmitteris a clean and effective way to observe changes, facilitating better separation of concerns and reactivity within the application.src/contexts/ThemeProvider.tsx (1)
- 15-15: Similar to
GraphQLClientProvider, the update toThemeProviderto utilizePropsWithChildrenis a good practice, ensuring that the component's ability to accept children is explicitly defined and type-safe.src/screens/WalletConnectRequest/useHooks.ts (1)
- 53-60: The introduction of
useRejectAllRequestshook provides a clear and concise way to reject all pending WalletConnect requests, encapsulating this functionality within a reusable hook to improve code organization and reuse.src/App.tsx (1)
- 25-36: The integration of URI action handling in the
Appcomponent usinggetCachedUriActionandisUriActionPendingfunctions, along with theuseSetUriActionhook, demonstrates a cohesive approach to managing and responding to URI actions at the application level.src/hooks/walletconnect/useWalletConnectApproveSessionProposal.ts (1)
- 52-69: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [21-66]
The refactoring of
useWalletConnectApproveSessionProposalto return aPromise<Result<void, Error>>and the adjustments to use properties from theproposalobject directly enhance the hook's clarity and type safety. This change also simplifies the handling of session proposals by focusing on the essential data, improving the maintainability of the code.src/hooks/useHandleReceivedActions.ts (1)
- 18-71: The implementation of
useHandleReceivedActionsto manage actions received while the application was in the background, including URI actions and WalletConnect session requests, is well-organized. It ensures that the application responds appropriately to external triggers, enhancing the user experience by handling these actions efficiently.src/types/walletConnect.ts (1)
- 69-74: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The removal of the
urlproperty from theWalletConnectSessioninterface indicates a refinement in the data model to better align with the actual requirements for managing WalletConnect sessions. This change likely simplifies the session handling by focusing on the necessary properties.src/types/uri.ts (1)
- 108-123: The extension of the
UriActionTypeenum to includeWalletConnectPairand the addition of theWalletConnectPairActionUriinterface are significant improvements. They introduce a structured way to handle WalletConnect pairing requests via URI actions, enhancing the application's capability to interact with external triggers in a type-safe manner.src/lib/UriActions/parsing.ts (1)
- 127-140: The implementation of
parseNativeActionUrito parse and generate aUriActionfrom a given URI string is a robust addition. It specifically handles the newWalletConnectPairaction type, demonstrating a clear and focused approach to parsing URI actions, which enhances the application's extensibility and interoperability with external triggers.ios/DesmosProfileManager/Info.plist (1)
- 35-40: The addition of a new
<dict>entry with the<key>CFBundleURLSchemes</key>and the<string>dpm</string>array in theInfo.plistfile correctly configures the iOS application to recognize thedpmURL scheme. This change is essential for supporting the new intent URI schema, facilitating the application's ability to handle custom URI schemes.src/screens/WalletConnectSessionProposal/index.tsx (2)
- 24-28: The update to the
WalletConnectSessionProposalParamsinterface to include an optionalreturnToAppboolean parameter is a thoughtful addition. It allows the component to conditionally adjust its behavior based on whether the user should be directed back to the initiating app after session approval, enhancing the flexibility and user experience of the WalletConnect session proposal flow.- 52-66: The conditional logic within
showSuccessModalto customize the modal message and action based on thereturnToAppparameter is well-implemented. It demonstrates attention to detail in providing a tailored user experience depending on the context of the WalletConnect session initiation.src/screens/WalletConnectRequest/index.tsx (2)
- 42-48: The addition of a navigation event listener in the
WalletConnectRequestcomponent to reject all requests when navigating back is a proactive measure to ensure that pending requests are not left unaddressed, enhancing the application's robustness in handling WalletConnect requests.- 75-77: Enhancing the
showErrorModalfunction to support an additional action allows for more flexible error handling. This change, particularly in the context of rejecting WalletConnect requests, provides a clearer path for cleaning up after an error, improving the application's resilience and user experience.src/screens/UnlockApplication/index.tsx (1)
- 73-92: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [34-84]
The changes in the
UnlockApplicationcomponent, including the removal ofuseHandleUriActionand the addition ofuseResetToHomeScreen, streamline the unlocking process. The reorganization of local variable declarations and the modification of the logic for unlocking the application and navigating to the home screen enhance the component's clarity and functionality.src/hooks/uriactions/useHandleUriAction.tsx (3)
- 10-10: The import of
WalletConnectPairActionUriis correctly added to support the new WalletConnect URI actions.- 22-27: The addition of new imports (
useActiveAccountAddress,useWalletConnectPair,ErrorModal,LoadingModal,useModal,useGetUriAction,useSetUriAction) aligns with the requirements for handling the new WalletConnect URI actions. Ensure these modules provide the expected functionality and are correctly integrated within the hook.- 145-173: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [139-199]
The
useHandleUriActionhook's integration ofhandleWalletConnectPairActionwithin its switch-case logic is correctly implemented. This ensures that the new WalletConnect pair action is handled alongside other URI actions. Ensure that theUriActionType.WalletConnectPairis correctly defined and that the casting ofactiontoWalletConnectPairActionUriis safe based on the incoming URI structure.src/screens/DevScreen/index.tsx (3)
- 24-24: The import of
useUriActionfrom@recoil/uriactionis correctly added to fetch the current URI action within theDevScreencomponent.- 65-65: The initialization of
uriActionusinguseUriAction()is correctly placed to fetch the current URI action for handling within theDevScreen.- 202-206: The modification to the
Buttoncomponent'sonPresshandler to passuriActiontohandleUriActionand the disabling of the button based on the presence ofuriActionare correctly implemented. This ensures that the URI action is only handled when one is present, enhancing the user experience by preventing unnecessary button presses.src/navigation/RootNavigator/index.tsx (2)
- 65-66: The addition of
useInitLinkingUriActionsanduseHandleReceivedActionsimports in place ofuseWalletConnectAutoReconnectsuggests a shift towards a more comprehensive handling of URI actions within the application's navigation setup. Ensure these hooks are correctly implemented to initialize and handle URI actions as the application starts or resumes.- 135-136: The use of
useInitLinkingUriActionsanduseHandleReceivedActionswithin theRootNavigatorcomponent is correctly placed to ensure that URI actions are initialized and handled appropriately as part of the application's lifecycle. This integration is crucial for the new feature's functionality, ensuring that URI actions are processed correctly upon app launch or when the app resumes from the background.
Comment on lines
+106
to
+138
| const useHandleWalletConnectPairAction = () => { | ||
| const { t } = useTranslation('walletConnect'); | ||
| const activeAccountAddress = useActiveAccountAddress(); | ||
| const pair = useWalletConnectPair(); | ||
| const navigation = useNavigation<StackNavigationProp<RootNavigatorParamList>>(); | ||
| const { showModal, hideModal } = useModal(); | ||
|
|
||
| return React.useCallback( | ||
| async (action: WalletConnectPairActionUri) => { | ||
| if (activeAccountAddress === undefined) { | ||
| // Ignore it if we don't have an account. | ||
| return; | ||
| } | ||
|
|
||
| showModal(LoadingModal, { | ||
| text: t('initializing new session'), | ||
| }); | ||
| const pairResult = await pair(action.uri); | ||
| hideModal(); | ||
| if (pairResult.isErr()) { | ||
| showModal(ErrorModal, { | ||
| text: pairResult.error.message, | ||
| }); | ||
| } else { | ||
| navigation.navigate(ROUTES.WALLET_CONNECT_SESSION_PROPOSAL, { | ||
| proposal: pairResult.value, | ||
| returnToApp: action.returnToApp, | ||
| }); | ||
| } | ||
| }, | ||
| [activeAccountAddress, hideModal, navigation, pair, showModal, t], | ||
| ); | ||
| }; |
There was a problem hiding this comment.
The implementation of useHandleWalletConnectPairAction correctly handles the WalletConnect pairing process, including showing loading and error modals as appropriate. However, consider adding error logging for the case when pairResult.isErr() is true, to aid in debugging and monitoring.
if (pairResult.isErr()) {
showModal(ErrorModal, {
text: pairResult.error.message,
});
+ // Consider logging this error for better monitoring and debugging
+ console.error(`WalletConnect pairing failed: ${pairResult.error.message}`);
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| const useHandleWalletConnectPairAction = () => { | |
| const { t } = useTranslation('walletConnect'); | |
| const activeAccountAddress = useActiveAccountAddress(); | |
| const pair = useWalletConnectPair(); | |
| const navigation = useNavigation<StackNavigationProp<RootNavigatorParamList>>(); | |
| const { showModal, hideModal } = useModal(); | |
| return React.useCallback( | |
| async (action: WalletConnectPairActionUri) => { | |
| if (activeAccountAddress === undefined) { | |
| // Ignore it if we don't have an account. | |
| return; | |
| } | |
| showModal(LoadingModal, { | |
| text: t('initializing new session'), | |
| }); | |
| const pairResult = await pair(action.uri); | |
| hideModal(); | |
| if (pairResult.isErr()) { | |
| showModal(ErrorModal, { | |
| text: pairResult.error.message, | |
| }); | |
| } else { | |
| navigation.navigate(ROUTES.WALLET_CONNECT_SESSION_PROPOSAL, { | |
| proposal: pairResult.value, | |
| returnToApp: action.returnToApp, | |
| }); | |
| } | |
| }, | |
| [activeAccountAddress, hideModal, navigation, pair, showModal, t], | |
| ); | |
| }; | |
| const useHandleWalletConnectPairAction = () => { | |
| const { t } = useTranslation('walletConnect'); | |
| const activeAccountAddress = useActiveAccountAddress(); | |
| const pair = useWalletConnectPair(); | |
| const navigation = useNavigation<StackNavigationProp<RootNavigatorParamList>>(); | |
| const { showModal, hideModal } = useModal(); | |
| return React.useCallback( | |
| async (action: WalletConnectPairActionUri) => { | |
| if (activeAccountAddress === undefined) { | |
| // Ignore it if we don't have an account. | |
| return; | |
| } | |
| showModal(LoadingModal, { | |
| text: t('initializing new session'), | |
| }); | |
| const pairResult = await pair(action.uri); | |
| hideModal(); | |
| if (pairResult.isErr()) { | |
| showModal(ErrorModal, { | |
| text: pairResult.error.message, | |
| }); | |
| // Consider logging this error for better monitoring and debugging | |
| console.error(`WalletConnect pairing failed: ${pairResult.error.message}`); | |
| } else { | |
| navigation.navigate(ROUTES.WALLET_CONNECT_SESSION_PROPOSAL, { | |
| proposal: pairResult.value, | |
| returnToApp: action.returnToApp, | |
| }); | |
| } | |
| }, | |
| [activeAccountAddress, hideModal, navigation, pair, showModal, t], | |
| ); | |
| }; |
RiccardoM
approved these changes
Jan 31, 2024
Comment on lines
+25
to
+26
| * Tells if the application should return to the app that | ||
| * has triggered the paring request after the session is approved. |
There was a problem hiding this comment.
Suggested change
| * Tells if the application should return to the app that | |
| * has triggered the paring request after the session is approved. | |
| * Whether the user should be brought back to the app that | |
| * has triggered the paring request after the session is approved. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Closes: DPM-190
This PR adds support for a new intent URI to start a new WalletConnect session from an external application.
The new intent URI has the following schema:
dpm://wcV2?uri=${WALLET_CONNECT_URI}&returnToApp=${true | false}.Here are the details about the query parameters:
uri- The URL-encoded WalletConnect URI to start the new session;returnToApp- Indicates whether the application should return to the application that has triggered the session start after the session is established.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor
UnlockApplicationandWalletConnectRequestcomponents for improved logic and user flow.Documentation