feat: in-app snackbar notifications for chat messages outside chat screen#578
feat: in-app snackbar notifications for chat messages outside chat screen#578
Conversation
…ages - Add activeChatScreensProvider to track which chat screens are currently open - Show in-app snackbar when new P2P or dispute messages arrive while user is on a different screen - Suppress notifications for user's own messages and when already viewing the conversation - Register/unregister chat screens in initState/dispose lifecycle hooks - Add chatNewMessage and disputeChatNewMessage notification message keys - Include comprehensive
WalkthroughAdds lifecycle tracking of active chat/dispute screens and shows in-app snackbar notifications for new chat messages only when the corresponding screen is not currently active; notifications are invoked via the existing notification actions provider and failures are logged. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Screen as Chat/Dispute Screen
participant Notifier as Chat/Dispute Notifier
participant ActiveProvider as Active Chat<br/>Screens Provider
participant NotifProvider as Notification<br/>Actions Provider
User->>Screen: Navigate to screen
Screen->>ActiveProvider: register(id)
activate ActiveProvider
ActiveProvider->>ActiveProvider: add id to set
deactivate ActiveProvider
Note over Notifier,ActiveProvider: Incoming message event
Notifier->>Notifier: process & append message
Notifier->>ActiveProvider: isActive(id)?
ActiveProvider-->>Notifier: true/false
alt screen not active
Notifier->>NotifProvider: showCustomMessage('chatNewMessage'/'disputeChatNewMessage')
NotifProvider->>User: display snackbar
else screen active
Note right of Notifier: suppress in-app notification
end
User->>Screen: Leave screen
Screen->>ActiveProvider: unregister(id)
activate ActiveProvider
ActiveProvider->>ActiveProvider: remove id from set
deactivate ActiveProvider
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/features/chat/providers/active_chat_screens_provider.dart (1)
10-24: Potential edge case: duplicate screen stacking could cause stale "inactive" state.If two instances of the same chat screen are ever pushed onto the navigation stack (e.g., due to rapid navigation or deep link while a chat is already open), the
Set<String>tracks only a single membership entry. When the first instance is popped andunregisterruns, the id is removed even though the second instance is still visible — subsequent messages will then trigger an in-app snackbar despite the chat being on screen.Consider switching to a reference-count map (
Map<String, int>) if this scenario is reachable in the app's current navigation flow. Otherwise, this is safe to defer with a comment documenting the single-instance assumption.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/chat/providers/active_chat_screens_provider.dart` around lines 10 - 24, The current ActiveChatScreensNotifier uses a Set<String> (state) so duplicate instances of the same chatId can produce stale inactive state when one instance is popped; update ActiveChatScreensNotifier to use a Map<String,int> reference-count (e.g., state: Map<String,int>), change register(chatId) to increment the count (initialize to 1 if absent) and unregister(chatId) to decrement the count and remove the key when the count reaches 0, and update isActive(chatId) to return count != null && count > 0; alternatively, if duplicate instances are impossible, add a clear comment on ActiveChatScreensNotifier/register/unregister documenting the single-instance assumption.lib/features/chat/notifiers/chat_room_notifier.dart (1)
287-303: LGTM — defensive self-message check plus active-screen gate.The
chat.pubkey == session.tradeKey.publicguard is a good defense-in-depth layer on top of the existinghasItem-based relay-echo dedup. Failure to show the snackbar is non-fatal thanks to the try/catch.Optional: the helper is near-duplicated in
dispute_chat_notifier.dart. If this pattern grows (muting, throttling, per-chat sounds), a small shared utility taking the active-screen id + custom message key would consolidate both call sites. Not worth doing today.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/chat/notifiers/chat_room_notifier.dart` around lines 287 - 303, No fix required for correctness; optional cleanup: extract the duplicated logic in _maybeShowInAppNotification (in chat_room_notifier.dart) and the similar helper in dispute_chat_notifier.dart into a shared utility (e.g., showInAppChatNotification) that accepts the current Session.tradeKey.public check, the activeScreens id (use activeChatScreensProvider), and the message key, then call notificationActionsProvider.notifier.showCustomMessage from that utility so both files reuse the same guard (self-message suppression and active-screen gate) and reduce duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/features/chat/notifiers/chat_room_notifier.dart`:
- Around line 287-303: No fix required for correctness; optional cleanup:
extract the duplicated logic in _maybeShowInAppNotification (in
chat_room_notifier.dart) and the similar helper in dispute_chat_notifier.dart
into a shared utility (e.g., showInAppChatNotification) that accepts the current
Session.tradeKey.public check, the activeScreens id (use
activeChatScreensProvider), and the message key, then call
notificationActionsProvider.notifier.showCustomMessage from that utility so both
files reuse the same guard (self-message suppression and active-screen gate) and
reduce duplication.
In `@lib/features/chat/providers/active_chat_screens_provider.dart`:
- Around line 10-24: The current ActiveChatScreensNotifier uses a Set<String>
(state) so duplicate instances of the same chatId can produce stale inactive
state when one instance is popped; update ActiveChatScreensNotifier to use a
Map<String,int> reference-count (e.g., state: Map<String,int>), change
register(chatId) to increment the count (initialize to 1 if absent) and
unregister(chatId) to decrement the count and remove the key when the count
reaches 0, and update isActive(chatId) to return count != null && count > 0;
alternatively, if duplicate instances are impossible, add a clear comment on
ActiveChatScreensNotifier/register/unregister documenting the single-instance
assumption.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5d6c3c0-71a1-4c00-87e5-71e9eb32c2fa
📒 Files selected for processing (7)
lib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/providers/active_chat_screens_provider.dartlib/features/chat/screens/chat_room_screen.dartlib/features/disputes/notifiers/dispute_chat_notifier.dartlib/features/disputes/screens/dispute_chat_screen.dartlib/shared/widgets/notification_listener_widget.darttest/features/chat/providers/active_chat_screens_provider_test.dart
|
@AndreaDiazCorreia don't forget to check the rabbit nitpicks please |
…cking Add inline documentation explaining why a simple Set is sufficient for tracking active chat screens instead of reference counting. The navigation graph guarantees at most one screen instance per chatId through context.push() from non-chat screens, ensuring dispose() runs before any duplicate registration attempt.
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code is clean and well-structured. The ActiveChatScreensNotifier correctly addresses CodeRabbit's nitpick by documenting the single-instance assumption in the class comment — no need for a reference-counted map given the current navigation graph. The self-message suppression via chat.pubkey == session.tradeKey.public is a solid defense-in-depth layer. The helper duplication in dispute_chat_notifier.dart is optional and not blocking. Tests cover all edge cases. CI passes. ✅
3 participants
Summary
Phase 3 of the chat notifications plan: show a transient snackbar when a P2P chat or dispute chat message arrives while the user is inside the app but not on that specific chat screen (e.g. browsing the order book).
Follows Phase 1 (#498, admin DMs in background) and Phase 2 (#529, P2P chat in background).
How it works
activeChatScreensProvidertracks aSet<String>of chat IDs whose screens are currently open.ChatRoomScreenandDisputeChatScreenregister oninitStateand unregister ondispose.ChatRoomNotifier._onChatEventfiresshowCustomMessage('chatNewMessage')for incoming P2P messages when the chat'sorderIdis not in the registry. Self-messages (relay echoes of sent messages) are filtered by comparingevent.pubkeyagainstsession.tradeKey.public.DisputeChatNotifier._onChatEventfiresshowCustomMessage('disputeChatNewMessage')for incoming admin messages, gated by the existingisFromAdmincheck.NotificationListenerWidgetresolves the two new keys to the existing ARB stringsnotification_new_message_messageandnotification_admin_message_message(already present in en/es/it/de/fr).No content is exposed in the snackbar; no history entry is persisted; snackbar is not tappable.
Test plan
flutter analyze— zero issues.flutter test— all tests pass, including the newActiveChatScreensNotifierunit tests.Summary by CodeRabbit
New Features
Tests