[Android] Improve notification UI. #4842
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,25 +3,28 @@ | |
| package com.zulipmobile.notifications | ||
|
|
||
| import android.annotation.SuppressLint | ||
| import android.app.Notification | ||
| import android.app.NotificationChannel | ||
| import android.app.NotificationManager | ||
| import android.app.PendingIntent | ||
| import android.content.Context | ||
| import android.content.Intent | ||
| import android.media.AudioAttributes | ||
| import android.net.Uri | ||
| import android.os.Build | ||
| import android.os.Bundle | ||
| import android.provider.Settings | ||
| import android.service.notification.StatusBarNotification | ||
| import android.util.Log | ||
| import androidx.core.app.NotificationCompat | ||
| import androidx.core.app.NotificationManagerCompat | ||
| import androidx.core.app.Person | ||
| import androidx.core.graphics.drawable.IconCompat | ||
| import com.facebook.react.ReactApplication | ||
| import com.zulipmobile.BuildConfig | ||
| import com.zulipmobile.R | ||
| import com.zulipmobile.ZLog | ||
| import me.leolin.shortcutbadger.ShortcutBadger | ||
|
|
||
| private val CHANNEL_ID = "default" | ||
| private val NOTIFICATION_ID = 435 | ||
|
|
@@ -33,6 +36,8 @@ val ACTION_CLEAR = "ACTION_CLEAR" | |
| val EXTRA_NOTIFICATION_DATA = "data" | ||
|
|
||
| fun createNotificationChannel(context: Context) { | ||
| val audioAttr: AudioAttributes = AudioAttributes.Builder() | ||
| .setUsage(AudioAttributes.USAGE_NOTIFICATION).build() | ||
| if (Build.VERSION.SDK_INT >= 26) { | ||
| val name = context.getString(R.string.notification_channel_name) | ||
|
|
||
|
|
@@ -43,6 +48,7 @@ fun createNotificationChannel(context: Context) { | |
| NotificationChannel(CHANNEL_ID, name, NotificationManagerCompat.IMPORTANCE_HIGH).apply { | ||
| enableLights(true) | ||
| enableVibration(true) | ||
| setSound(getNotificationSoundUri(), audioAttr) | ||
| } | ||
| NotificationManagerCompat.from(context).createNotificationChannel(channel) | ||
| } | ||
|
|
@@ -53,7 +59,7 @@ private fun logNotificationData(msg: String, data: Bundle) { | |
| Log.v(TAG, "$msg: $data") | ||
| } | ||
|
|
||
| internal fun onReceived(context: Context, mapData: Map<String, String>) { | ||
| // TODO refactor to not need this; reflects a juxtaposition of FCM with old GCM interfaces. | ||
| val data = Bundle() | ||
| for ((key, value) in mapData) { | ||
|
|
@@ -70,124 +76,211 @@ internal fun onReceived(context: Context, conversations: ConversationMap, mapDat | |
| } | ||
|
|
||
| if (fcmMessage is MessageFcmMessage) { | ||
| updateNotification(context, fcmMessage) | ||
| } else if (fcmMessage is RemoveFcmMessage) { | ||
| removeNotification(context, fcmMessage) | ||
| } | ||
| } | ||
|
|
||
| fun removeNotification(context: Context, fcmMessage: RemoveFcmMessage) { | ||
| val statusBarNotifications = getActiveNotifications(context) ?: return | ||
| val groupKey = extractGroupKey(fcmMessage.identity) | ||
| // Find any conversations we can cancel the notification for. | ||
| // The API doesn't lend itself to removing individual messages as | ||
| // they're read, so we wait until we're ready to remove the whole | ||
| // conversation's notification. | ||
| // See: https://github.com/zulip/zulip-mobile/pull/4842#pullrequestreview-725817909 | ||
| for (statusBarNotification in statusBarNotifications) { | ||
| // Each statusBarNotification represents one Zulip conversation. | ||
| val notification = statusBarNotification.notification | ||
| val lastMessageId = notification.extras.getInt("lastZulipMessageId") | ||
| if (fcmMessage.messageIds.contains(lastMessageId)) { | ||
| // The latest Zulip message in this conversation was read. | ||
| // That's our cue to cancel the notification for the conversation. | ||
| NotificationManagerCompat.from(context).cancel(statusBarNotification.tag, statusBarNotification.id) | ||
| } | ||
| } | ||
| var counter = 0 | ||
| for (statusBarNotification in statusBarNotifications) { | ||
| if (statusBarNotification.notification.group == groupKey) { | ||
| counter++ | ||
| } | ||
| } | ||
| if (counter == 2) { | ||
| // counter will be 2 only when summary notification and last notification are | ||
| // present; in this case, we remove summary notification. | ||
| NotificationManagerCompat.from(context).cancel(groupKey, NOTIFICATION_ID) | ||
| } | ||
| } | ||
|
|
||
| private fun createViewPendingIntent(fcmMessage: MessageFcmMessage, context: Context): PendingIntent { | ||
| val uri = Uri.fromParts("zulip", "msgid:${fcmMessage.zulipMessageId}", "") | ||
| val viewIntent = Intent(Intent.ACTION_VIEW, uri, context, NotificationIntentService::class.java) | ||
| viewIntent.putExtra(EXTRA_NOTIFICATION_DATA, fcmMessage.dataForOpen()) | ||
| return PendingIntent.getService(context, 0, viewIntent, 0) | ||
| } | ||
|
|
||
| private fun createDismissAction(context: Context): NotificationCompat.Action { | ||
| val dismissIntent = Intent(context, NotificationIntentService::class.java) | ||
| dismissIntent.action = ACTION_CLEAR | ||
| val piDismiss = PendingIntent.getService(context, 0, dismissIntent, 0) | ||
| return NotificationCompat.Action( | ||
| android.R.drawable.ic_menu_close_clear_cancel, | ||
| "Clear", | ||
| piDismiss | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * @param context | ||
| * @param conversationKey Unique Key identifying a conversation, the current | ||
| * implementation assumes that each notification corresponds to 1 and only 1 | ||
| * conversation. | ||
| * | ||
| * A conversation can be any of: Topic in Stream, GroupPM or PM for a | ||
| * specific user in a specific realm. | ||
| */ | ||
| private fun getActiveNotification(context: Context, conversationKey: String): Notification? { | ||
| // activeNotifications are not available in NotificationCompatManager | ||
| // Hence we have to use instance of NotificationManager. | ||
| val notificationManager = | ||
| context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager? | ||
|
|
||
| val activeStatusBarNotifications = notificationManager?.activeNotifications | ||
| if (activeStatusBarNotifications != null) { | ||
| for (statusBarNotification in activeStatusBarNotifications) { | ||
| if (statusBarNotification.tag == conversationKey) { | ||
| return statusBarNotification.notification | ||
| } | ||
| } | ||
| } | ||
| return null | ||
| } | ||
|
|
||
|
|
||
| private fun getActiveNotifications(context: Context): Array<StatusBarNotification>? = | ||
| (context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager?)?.activeNotifications | ||
|
|
||
| // TODO: Add a Text saying n messages in m conversations. (this will | ||
| // only be visible in API < 24) | ||
| private fun createSummaryNotification( | ||
|
Member
There was a problem hiding this comment. How did you find that it's required by Android even on newer versions? From the guide doc for this feature:
If we don't need this when on Android 7+ (aka API 24+), then it's probably time to drop support for older Android. We'd have to take a quick look at the numbers, but older versions were down to 3% a year ago, so by now they're probably down into the range where we can cheerfully drop support. And being able to keep this code simpler would definitely be a good reason to do it if so.
Member
Author
There was a problem hiding this comment.
I tested a build without summary Notification, specifically without the notify call to summaryNotification, observation is that notification do occur but they don't group together, even in android 7+. It seems that this notify call triggers the actual grouping. Also while they don't appear completely in Android 7+, (since they get overridden by the summary notification built by the Android OS) they still have some visual influence on how the summary notification looks, for instance, setting
Member
There was a problem hiding this comment.
Hmm fascinating -- thanks for determining that. Seems like a bug in the docs that they say it's optional, then. I guess in the example code says: val summaryNotification = NotificationCompat.Builder(this@MainActivity, CHANNEL_ID)
.setContentTitle(emailObject.getSummary())
//set content text to support devices running API level < 24
.setContentText("Two new messages")
// […]which makes it sound more like the actual situation you discovered, that the summary notification itself is required but the text is only for older devices. We did find when I looked recently that indeed Android 6 and older is down to a small enough sliver of our userbase that it makes sense to drop support for it as soon as there's something where supporting it would make things more complicated for us. So I think we won't go and add a summary text, and instead will just drop support and increase the minimum to Android 7. |
||
| context: Context, | ||
| fcmMessage: MessageFcmMessage, | ||
| groupKey: String | ||
| ): NotificationCompat.Builder { | ||
| val realmUri = fcmMessage.identity.realmUri.toString() | ||
| return NotificationCompat.Builder(context, CHANNEL_ID).apply { | ||
| color = context.getColor(R.color.brandColor) | ||
| if (BuildConfig.DEBUG) { | ||
| setSmallIcon(R.mipmap.ic_launcher) | ||
| } else { | ||
| setSmallIcon(R.drawable.zulip_notification) | ||
| } | ||
| setStyle(NotificationCompat.InboxStyle() | ||
| .setSummaryText(realmUri) | ||
| ) | ||
| setGroup(groupKey) | ||
| setGroupSummary(true) | ||
| setAutoCancel(true) | ||
| } | ||
| } | ||
|
|
||
| private fun extractGroupKey(identity: Identity): String { | ||
| return "${identity.realmUri.toString()}|${identity.userId}" | ||
| } | ||
|
|
||
| private fun extractConversationKey(fcmMessage: MessageFcmMessage): String { | ||
| val groupKey = extractGroupKey(fcmMessage.identity) | ||
| val conversation = when (fcmMessage.recipient) { | ||
| is Recipient.Stream -> "stream:${fcmMessage.recipient.stream}\u0000${fcmMessage.recipient.topic}" | ||
| is Recipient.GroupPm -> "groupPM:${fcmMessage.recipient.pmUsers.toString()}" | ||
| is Recipient.Pm -> "private:${fcmMessage.sender.id}" | ||
| } | ||
| return "$groupKey|$conversation" | ||
| } | ||
|
|
||
| private fun updateNotification( | ||
| context: Context, fcmMessage: MessageFcmMessage) { | ||
| val selfUser = Person.Builder().setName(context.getString(R.string.selfUser)).build() | ||
| val sender = Person.Builder() | ||
| .setName(fcmMessage.sender.fullName) | ||
| .setIcon(IconCompat.createWithBitmap(fetchBitmap(fcmMessage.sender.avatarURL))) | ||
| .build() | ||
|
|
||
| val title = when (fcmMessage.recipient) { | ||
| is Recipient.Stream -> "#${fcmMessage.recipient.stream} > ${fcmMessage.recipient.topic}" | ||
| // TODO use proper title for GroupPM, we will need | ||
| // to have a way to get names of PM users here. | ||
| is Recipient.GroupPm -> context.resources.getQuantityString( | ||
| R.plurals.group_pm, | ||
| fcmMessage.recipient.pmUsers.size - 2, | ||
| fcmMessage.sender.fullName, | ||
| fcmMessage.recipient.pmUsers.size - 2 | ||
| ) | ||
| is Recipient.Pm -> fcmMessage.sender.fullName | ||
| } | ||
| val isGroupConversation = when (fcmMessage.recipient) { | ||
| is Recipient.Stream -> true | ||
| is Recipient.GroupPm -> true | ||
| is Recipient.Pm -> false | ||
| } | ||
| val groupKey = extractGroupKey(fcmMessage.identity) | ||
| val conversationKey = extractConversationKey(fcmMessage) | ||
| val notification = getActiveNotification(context, conversationKey) | ||
|
|
||
| val messagingStyle = notification?.let { | ||
| NotificationCompat.MessagingStyle.extractMessagingStyleFromNotification(it) | ||
| } ?: NotificationCompat.MessagingStyle(selfUser) | ||
| messagingStyle | ||
| .setConversationTitle(title) | ||
| .setGroupConversation(isGroupConversation) | ||
gnprice marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| .addMessage(fcmMessage.content, fcmMessage.timeMs, sender) | ||
|
|
||
| val messageCount = messagingStyle.messages.size | ||
|
|
||
| val builder = NotificationCompat.Builder(context, CHANNEL_ID) | ||
| builder.apply { | ||
| color = context.getColor(R.color.brandColor) | ||
| if (BuildConfig.DEBUG) { | ||
| setSmallIcon(R.mipmap.ic_launcher) | ||
| } else { | ||
| setSmallIcon(R.drawable.zulip_notification) | ||
| } | ||
| setAutoCancel(true) | ||
| setStyle(messagingStyle) | ||
| setGroup(groupKey) | ||
| setSound(getNotificationSoundUri()) | ||
| setContentIntent(createViewPendingIntent(fcmMessage, context)) | ||
| setNumber(messageCount) | ||
|
|
||
| val extraData = Bundle() | ||
| extraData.putInt("lastZulipMessageId", fcmMessage.zulipMessageId) | ||
| extras = extraData | ||
| } | ||
|
|
||
| val summaryNotification = createSummaryNotification(context, fcmMessage, groupKey) | ||
|
|
||
| NotificationManagerCompat.from(context).apply { | ||
| // We use `tag` param only, to uniquely identify notifications, | ||
| // and hence `id` param is provided as an arbitrary constant. | ||
| notify(groupKey, NOTIFICATION_ID, summaryNotification.build()) | ||
| notify(conversationKey, NOTIFICATION_ID, builder.build()) | ||
| } | ||
| } | ||
|
|
||
| private fun getNotificationSoundUri(): Uri { | ||
| // Note: Provide default notification sound until we found a good sound | ||
| // return Uri.parse("${ContentResolver.SCHEME_ANDROID_RESOURCE}://${context.packageName}/${R.raw.zulip}") | ||
| return Settings.System.DEFAULT_NOTIFICATION_URI | ||
| } | ||
|
|
||
| internal fun onOpened(application: ReactApplication, data: Bundle) { | ||
| logNotificationData("notif opened", data) | ||
| notifyReact(application, data) | ||
| try { | ||
| ShortcutBadger.removeCount(application as Context) | ||
| } catch (e: Exception) { | ||
| ZLog.e(TAG, e) | ||
| } | ||
|
|
||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, odd that it isn't available in
NotificationCompatManager. Do you know what the story is there?I'd expect the compat library to cover all the functionality that's there in the latest Android SDK. So when a given method or property isn't there, my first thought is that that's a signal that there's some other preferred way to do the same thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this, I couldn't find any reference for why this is the case.
On a side note it is also possible to get active notification via NotificationListenerService#getActiveNotifications so they could have nicely consolidated this for most of the android versions. This is how one can get
activeNotificationsin API < 23.