feat(wallet-mobile): Add screen navigation for notification press#3919
feat(wallet-mobile): Add screen navigation for notification press#3919stackchain merged 10 commits intodevelopfrom
Conversation
WalkthroughThis change refactors notification handling and navigation integration within a mobile wallet application. The initialization of notifications is moved into a dedicated component within the navigation container. Notification action handling is updated to include navigation context, allowing for both immediate and deferred navigation actions based on the source of the notification (operating system or app). New helper functions are introduced to manage deferred navigation triggered by notifications. The navigation system is extended with a method for direct navigation to the notifications screen, and related type definitions are updated. Metadata for translation messages is also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant OS
participant AppNavigator
participant NotificationContainer
participant NotificationHooks
participant WalletNavigation
participant NotificationTools
OS->>AppNavigator: Launch app / deliver notification
AppNavigator->>NotificationContainer: Render
NotificationContainer->>NotificationHooks: useInitNotifications({localEnabled, pushEnabled})
NotificationHooks->>NotificationTools: initPushNotifications(walletNavigation)
NotificationTools->>NotificationHooks: Register notification opened handler
OS->>NotificationHooks: Notification opened event
NotificationHooks->>NotificationTools: triggerNotificationAction({manager, id, walletNavigation, source: 'os'})
NotificationTools->>WalletNavigation: (deferred or immediate navigation based on source)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code Graph Analysis (1)apps/wallet-mobile/src/features/Notifications/common/tools.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (9)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3919 +/- ##
===========================================
- Coverage 22.23% 22.18% -0.06%
===========================================
Files 1140 1140
Lines 21764 21820 +56
Branches 3467 3480 +13
===========================================
Hits 4840 4840
- Misses 16779 16835 +56
Partials 145 145
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/wallet-mobile/src/features/Notifications/common/tools.ts (1)
62-69: Distinguish internal-navigation storage key & encapsulate itThe literal
'triggerNotificationInternalNavigationAction'is repeated three times.
A single constant reduces the risk of typos and eases future refactors:- if (source === 'os') { - await uiStorage.setItem('triggerNotificationInternalNavigationAction', event.id) + if (source === 'os') { + await uiStorage.setItem(INTERNAL_NAV_KEY, event.id) ... - const id = await uiStorage.getItem('triggerNotificationInternalNavigationAction') + const id = await uiStorage.getItem(INTERNAL_NAV_KEY) ... - await uiStorage.removeItem('triggerNotificationInternalNavigationAction') + await uiStorage.removeItem(INTERNAL_NAV_KEY)Define the key once at the top of the file:
const INTERNAL_NAV_KEY = 'triggerNotificationInternalNavigationAction' as const
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/wallet-mobile/src/AppNavigator.tsx(2 hunks)apps/wallet-mobile/src/features/Notifications/common/hooks.ts(3 hunks)apps/wallet-mobile/src/features/Notifications/common/tools.ts(3 hunks)apps/wallet-mobile/src/features/Notifications/useCases/ViewNotificationHistory/ViewNotificationHistoryScreen.tsx(2 hunks)apps/wallet-mobile/src/features/WalletManager/useCases/SelectWalletFromListScreen/SelectWalletFromListScreen.tsx(3 hunks)apps/wallet-mobile/src/kernel/navigation.tsx(2 hunks)apps/wallet-mobile/translations/messages/src/AppNavigator.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
apps/wallet-mobile/src/features/Notifications/useCases/ViewNotificationHistory/ViewNotificationHistoryScreen.tsx (2)
apps/wallet-mobile/src/kernel/navigation.tsx (1)
useWalletNavigation(405-676)apps/wallet-mobile/src/features/Notifications/common/tools.ts (1)
triggerNotificationAction(43-70)
apps/wallet-mobile/src/AppNavigator.tsx (1)
apps/wallet-mobile/src/features/Notifications/common/hooks.ts (1)
useInitNotifications(61-72)
apps/wallet-mobile/src/features/WalletManager/useCases/SelectWalletFromListScreen/SelectWalletFromListScreen.tsx (4)
apps/wallet-mobile/src/kernel/navigation.tsx (1)
useWalletNavigation(405-676)packages/types/src/index.ts (1)
Meta(665-665)apps/wallet-mobile/src/features/Notifications/common/tools.ts (2)
shouldHandleNotificationInternalNavigationAction(76-79)handleNotificationInternalNavigationAction(81-94)apps/wallet-mobile/src/features/Notifications/common/notification-manager.ts (1)
pushNotificationsManager(11-14)
apps/wallet-mobile/src/features/Notifications/common/hooks.ts (3)
apps/wallet-mobile/src/kernel/navigation.tsx (2)
WalletNavigation(678-678)useWalletNavigation(405-676)apps/wallet-mobile/src/features/Notifications/common/tools.ts (1)
triggerNotificationAction(43-70)apps/wallet-mobile/src/features/Notifications/common/notification-manager.ts (1)
pushNotificationsManager(11-14)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: check
🔇 Additional comments (8)
apps/wallet-mobile/translations/messages/src/AppNavigator.json (1)
7-9: Line numbers updated correctly to match new component structure.The metadata adjustments for translation messages accurately reflect the structural changes in the AppNavigator.tsx file, where the notification initialization was moved into a dedicated NotificationContainer component.
Also applies to: 12-14, 22-24, 27-29, 37-39, 42-44, 52-54, 57-59
apps/wallet-mobile/src/kernel/navigation.tsx (2)
639-641: Added navigation method to notifications screen.The new navigateToNotifications method properly implements direct navigation to the notifications screen, which is essential for the notification-triggered navigation feature.
678-678: Added type definition for wallet navigation.The WalletNavigation type definition provides proper type safety for the navigation object used throughout the notification handling system.
apps/wallet-mobile/src/AppNavigator.tsx (2)
120-120: Relocated notification initialization for proper navigation context.Moving the NotificationContainer inside NavigationContainer ensures notifications have access to the navigation context.
243-246: Created dedicated component for notification initialization.The new NotificationContainer component correctly encapsulates notification initialization logic, maintaining the same configuration while enabling proper integration with navigation context.
apps/wallet-mobile/src/features/Notifications/common/hooks.ts (1)
65-68: Trim unused dependency to avoid redundant effect resets
manageris not referenced inside this effect body, yet it appears in the dependency list.
React will not re-run the effect becausewalletNavigationis stable, but automated linting may complain and newcomers might wonder why it is there.- React.useEffect( - () => (pushEnabled ? initPushNotifications(walletNavigation) : undefined), - [walletNavigation, pushEnabled, manager], - ) + React.useEffect( + () => (pushEnabled ? initPushNotifications(walletNavigation) : undefined), + [walletNavigation, pushEnabled], + )[ suggest_nitpick ]
apps/wallet-mobile/src/features/Notifications/useCases/ViewNotificationHistory/ViewNotificationHistoryScreen.tsx (2)
15-15: Navigation integration looks goodAdding the
useWalletNavigationhook here allows the notification component to properly navigate to different screens in response to notification actions. This matches the broader refactoring pattern seen in the codebase.Also applies to: 32-32
39-41: Correctly implemented notification action handlingThe updated
triggerNotificationActioncall properly passes the navigation context and identifies the source as 'app'. This ensures that when a user taps a notification within the app, it will directly handle navigation instead of deferring it through persistent storage, which is the correct behavior.
...rc/features/WalletManager/useCases/SelectWalletFromListScreen/SelectWalletFromListScreen.tsx
Show resolved
Hide resolved
Screencast_20250506_123055.webm |
jorbuedo
changed the title
3 participants
Description / Change(s) / Related issue(s)
Add screen navigation for notification press
TODO
Ticket
YV-401
Summary by CodeRabbit
New Features
Chores