feat(native-app): Implement universal links #15961
Conversation
WalkthroughThe pull request introduces several changes to the iOS application, including the addition of support for Universal Links and deep linking through modifications to entitlement files, package dependencies, and the implementation of custom hooks. Key updates include the integration of the Changes
Possibly related PRs
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (10)
Files not reviewed due to no reviewable changes (3)
Files skipped from review due to trivial changes (1)
Files skipped from review as they are similar to previous changes (4)
Additional context usedPath-based instructions (2)
Additional comments not posted (2)
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
eirikurn
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
apps/native/app/src/lib/deep-linking.ts (1)
Line range hint
191-222: LGTM with a minor suggestion!The
navigateToUniversalLinkfunction is well-structured and handles both cases of navigating within the app and opening external links in a browser. The addition of theopenBrowserparameter allows for flexibility in how links are opened.Consider adding a comment to explain the purpose of the
ComponentRegistry.HomeScreenfallback whencomponentIdis not provided. For example:// If componentId is not provided, use the HomeScreen component as a fallback openBrowser(link, componentId ?? ComponentRegistry.HomeScreen)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (8)
- apps/native/app/ios/IslandApp/IslandApp.entitlements (1 hunks)
- apps/native/app/package.json (1 hunks)
- apps/native/app/src/hooks/use-deep-link-handling.ts (1 hunks)
- apps/native/app/src/index.tsx (2 hunks)
- apps/native/app/src/lib/deep-linking.ts (2 hunks)
- apps/native/app/src/screens/home/home.tsx (4 hunks)
- apps/native/app/src/screens/notifications/notifications.tsx (4 hunks)
- apps/native/app/src/utils/lifecycle/setup-event-handlers.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- apps/native/app/src/index.tsx
Additional context used
Path-based instructions (7)
apps/native/app/ios/IslandApp/IslandApp.entitlements (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/hooks/use-deep-link-handling.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/package.json (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/utils/lifecycle/setup-event-handlers.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/lib/deep-linking.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/screens/notifications/notifications.tsx (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/screens/home/home.tsx (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
Additional comments not posted (15)
apps/native/app/ios/IslandApp/IslandApp.entitlements (1)
10-10: LGTM!The added
applinks:island.isentry for thecom.apple.developer.associated-domainskey is correct and matches the expected format for supporting Universal Links.apps/native/app/src/hooks/use-deep-link-handling.ts (2)
13-33: LGTM!The custom hook
useLastNotificationResponseis implemented correctly. It handles the initial notification and listens for new notifications using the Firebase messaging API. The state is managed using theuseStateanduseEffecthooks, and the unsubscribe function is returned to clean up the listener when the component unmounts.
35-73: LGTM!The custom hook
useDeepLinkHandlingis implemented correctly. It handles deep links and notifications by using theuseURLhook fromexpo-linkingto get the current URL and theuseLastNotificationResponsehook to get the last notification response. ThehandleUrlfunction navigates to the universal link if the conditions are met, and theuseEffecthooks handle the URL and notification changes. The hook also marks the notification as read using themarkUserNotificationAsReadmutation if the notification was handled and has anotificationId.The hook uses the
useRefhook to store the last URL to avoid unnecessary navigation and theuseCallbackhook to memoize thehandleUrlfunction and avoid unnecessary re-creation.apps/native/app/package.json (1)
60-60: LGTM!The addition of the
expo-linkingpackage with version6.3.1to the dependencies is approved. This change aligns with the PR objective of implementing universal links.apps/native/app/src/utils/lifecycle/setup-event-handlers.ts (1)
Line range hint
1-160: LGTM!The code changes in the
setupEventHandlersfunction adhere to TypeScript best practices, use efficient state management techniques, and are well-organized and modular. The function uses external libraries and custom utility functions appropriately and handles errors effectively.The code changes are approved.
apps/native/app/src/lib/deep-linking.ts (2)
189-190: LGTM!The comment clearly explains the purpose and behavior of the
navigateToUniversalLinkfunction.
Line range hint
226-248: LGTM!The changes to the
urlMappingobject are approved. The addition of the new entry for/minarsidur/postholfexpands the app's navigation capabilities by allowing direct access to the/inboxscreen.apps/native/app/src/screens/notifications/notifications.tsx (3)
37-37: LGTM!The change in the import statement is approved as it aligns with the shift towards supporting universal links for notifications.
48-48: LGTM!The addition of the
useBrowserhook is approved as it enables the application to open links externally in a browser, providing more flexibility in handling notification links.
152-164: LGTM!The changes to the
onNotificationPresscallback function are approved as they enhance the notification handling logic, allowing for a more versatile approach to link navigation. The application can now either navigate within the app or open the link externally in a browser, improving the user experience.apps/native/app/src/screens/home/home.tsx (5)
23-23: LGTM!The code changes are approved.
25-28: LGTM!The code changes are approved.
33-37: LGTM!The code changes are approved.
49-53: LGTM!The code changes are approved.
153-153: LGTM!The code changes are approved.
- Only runs when the user is logged in and has passed the pin screen. - Improves notification handling to support passkey browser.
eirikurn
force-pushed
the
app-universal-links
branch
from
3ea1200 to
3c11e85
Compare
Datadog ReportAll test runs ✅ 101 Total Test Services: 0 Failed, 99 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
There was a problem hiding this comment.
Nice! I think it would be good to also get review from @snaerseljan, since he has done most of the work on these files that you are changing.
I did not test it, only read over the code 👀
|
Tested on iOS. Didn't have an Android phone. Will test with internal testing. |
eirikurn
added
the
deprecated:automerge
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes