Messaging icon improvements #2967
Conversation
Drawing a red circle on the icon when there are unread messages.
OrangeAndGreen
requested review from
Jignesh-dimagi,
pm-dimagi and
shubham1g5
📝 WalkthroughWalkthroughThis pull request refactors and updates resources and notification handling related to messaging. Two drawable XML files ( Sequence Diagram(s)sequenceDiagram
participant FMS as FirebaseMessagingService
participant CF as CommCareFirebaseMessagingService
participant NC as CommCareNoficationManager
participant NM as NotificationManager
FMS->>CF: Receive Push Message (with action type)
alt Action is Messaging
CF->>CF: Convert vector drawable to Bitmap
CF->>NC: Select NOTIFICATION_CHANNEL_MESSAGING_ID and set priority to MAX
else Default Notification
CF->>NC: Select default notification channel and set high priority
end
CF->>NM: Build and send notification with selected channel and icon
NM-->>CF: Notification displayed
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/res/drawable/ic_connect_menu_notification.xml(0 hunks)app/res/drawable/ic_connect_menu_notification_none.xml(0 hunks)app/res/drawable/ic_connect_messaging_base.xml(1 hunks)app/res/drawable/ic_connect_messaging_unread.xml(1 hunks)app/res/menu/menu_connect.xml(1 hunks)app/res/menu/menu_connect_messaging.xml(1 hunks)app/res/values-pt/strings.xml(1 hunks)app/res/values-ti/strings.xml(1 hunks)app/res/values/strings.xml(2 hunks)app/src/org/commcare/CommCareNoficationManager.java(2 hunks)app/src/org/commcare/activities/connect/ConnectActivity.java(1 hunks)app/src/org/commcare/services/CommCareFirebaseMessagingService.java(4 hunks)
💤 Files with no reviewable changes (2)
- app/res/drawable/ic_connect_menu_notification_none.xml
- app/res/drawable/ic_connect_menu_notification.xml
✅ Files skipped from review due to trivial changes (4)
- app/res/drawable/ic_connect_messaging_unread.xml
- app/res/menu/menu_connect.xml
- app/res/drawable/ic_connect_messaging_base.xml
- app/res/menu/menu_connect_messaging.xml
🔇 Additional comments (9)
app/src/org/commcare/activities/connect/ConnectActivity.java (1)
163-171: Icon resource changes look good.The replacement of notification icons with dedicated messaging icons provides better context for the functionality, aligning with the PR objective to change from a bell icon to a chat bubble.
app/res/values-ti/strings.xml (1)
286-287: New localized strings for messaging notification channel added correctly.The implementation adds proper Tigrinya translations for the messaging notification channel title and description, which is important for the full internationalization of the new messaging feature.
app/res/values-pt/strings.xml (1)
298-299: String resources properly added for messaging notification channel.The new notification channel strings for Portuguese translation have been correctly implemented, aligning with the new messaging icon feature described in the PR objectives.
app/src/org/commcare/CommCareNoficationManager.java (2)
43-43: New messaging notification channel ID added.This constant properly defines the channel ID for the new messaging notifications.
200-203: Notification channel created with highest priority.The messaging notification channel is correctly configured with
IMPORTANCE_MAXpriority level, which is appropriate for immediate user attention to new messages.app/src/org/commcare/services/CommCareFirebaseMessagingService.java (2)
102-111: Utility method added to convert vector drawables to bitmaps.This method will enable dynamic generation of notification icons from vector drawables, supporting the new messaging icon feature.
214-222: Notification builder updated to use dynamic properties.The notification builder has been correctly updated to use the dynamic notification channel and large icon based on the notification type.
app/res/values/strings.xml (2)
354-355: New string resources added for messaging notifications.These strings properly define the title and description for the new messaging notification channel.
555-557: Bell emojis added to notification titles.Adding bell emojis to the notification titles makes them more visually distinct and draws attention to these important alerts.
| String notificationChannel = CommCareNoficationManager.NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID; | ||
| int priority = NotificationCompat.PRIORITY_HIGH; | ||
| Bitmap largeIcon = null; | ||
|
|
||
| if (hasCccAction(action)) { | ||
| FirebaseAnalyticsUtil.reportNotificationType(action); | ||
|
|
||
| if(action.equals(ConnectMessagingActivity.CCC_MESSAGE)) { | ||
| notificationChannel = CommCareNoficationManager.NOTIFICATION_CHANNEL_MESSAGING_ID; | ||
| priority = NotificationCompat.PRIORITY_MAX; | ||
| largeIcon = getBitmapFromVectorDrawable(this, R.drawable.commcare_actionbar_logo); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Notification properties set based on action type.
The code properly sets the notification channel and priority for messaging notifications, but there may be a minor inconsistency with the PR description.
The PR mentions changing to a chat bubble icon, but line 133 is using the CommCare action bar logo instead of a messaging-specific icon. Verify if this is intended or if the chat bubble should be used here as well.
🏁 Script executed:
#!/bin/bash
# Check if there are any drawable resources related to messaging icons
echo "Searching for messaging icon resources..."
find app/res -name "ic_connect_messaging*"Length of output: 233
Update Notification Icon to Use Messaging-Specific Resource
- The PR description indicates that notifications should use a chat bubble icon for messaging, yet the current code (line 133) still references the CommCare action bar logo.
- The drawable resources search reveals available messaging icons (
ic_connect_messaging_unread.xmlandic_connect_messaging_base.xml), which implies that one of these would be more appropriate. - Please verify which of these icons should be used and update the resource reference accordingly if the chat bubble icon was intended.
|
@OrangeAndGreen there is a bunch of common code between this and https://github.com/dimagi/commcare-android/pull/2966/files , think we should only be keeping the icon change in this PR and rest should form part of other part to keep it clean. |
OrangeAndGreen
changed the base branch from
connect_qa
to
dv/notification_improvements
|
Ah, my mistake, I branched off of the branch for that other PR by mistake. I rebased this PR to that branch to isolate the changes for the icon. We'll be trying to get both of these merged pretty quickly for the next beta release. |
https://dimagi.atlassian.net/browse/CCCT-710
cross-request: dimagi/commcare-core#1455
Product Description
Changed messaging icon to be a chat bubble.
Drawing a red circle on the icon when there are unread messages or new channels.
New icons:
Technical Summary
Replaced the existing drawables that used a bell icon.
Feature Flag
ConnectID Messaging
Safety Assurance
Safety story
Just a graphics change, should be pretty safe.
Automated test coverage
No automated tests for ConnectID yet.
QA Plan
Verify the new icon appears in the Connect home page, and includes a red dot when there are unread messages/channels.
Labels and Review