From 3e1d54d7e4f1d939b8ef6fab729ea029b2feb2ed Mon Sep 17 00:00:00 2001 From: Rachel Hyman Date: Sat, 6 Aug 2022 19:50:03 -0500 Subject: [PATCH] notifications: Remove Android summary notification. Removes the summary notification from the notification group in NotificationManager.kt, and removes logic around removing the summary notification when Zulip messages are read. This change allows the user to tap on the body of the notification to open the app, which was not possible with the summary notification (only the chevron would expand the notification, which would then allow the user to interact with the individual notifications below it. Please note that removing the summary notification breaks proper grouping: multiple private messages from the same user will correctly be grouped together, but notifications for different stream conversations appear separately. This was tested manually. Fixes part of #5242. --- .../notifications/NotificationUiManager.kt | 38 +------------------ 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/android/app/src/main/java/com/zulipmobile/notifications/NotificationUiManager.kt b/android/app/src/main/java/com/zulipmobile/notifications/NotificationUiManager.kt index 6b65d5180a1..241dc6c678e 100644 --- a/android/app/src/main/java/com/zulipmobile/notifications/NotificationUiManager.kt +++ b/android/app/src/main/java/com/zulipmobile/notifications/NotificationUiManager.kt @@ -108,8 +108,7 @@ internal fun onReceived(context: Context, mapData: Map) { private fun removeNotification(context: Context, fcmMessage: RemoveFcmMessage) { // We have an FCM message telling us that some Zulip messages were read // and should no longer appear as notifications. We'll remove their - // conversations' notifications, if appropriate, and then the whole - // notification group if it's now empty. + // conversations' notifications, if appropriate. // There may be a lot of messages mentioned here, across a lot of // conversations. But they'll all be for one identity, so they'll @@ -121,7 +120,6 @@ private fun removeNotification(context: Context, fcmMessage: RemoveFcmMessage) { // 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 - var haveRemaining = false for (statusBarNotification in context.notificationManager.activeNotifications) { // The StatusBarNotification object describes an active notification in the UI. // Its relationship to the Notification and to our metadata is a bit confusing: @@ -136,27 +134,14 @@ private fun removeNotification(context: Context, fcmMessage: RemoveFcmMessage) { // Don't act on notifications that are for other Zulip accounts/identities. if (notification.group != groupKey) continue; - // Don't act on the summary notification for the group. - if (statusBarNotification.tag == groupKey) continue; - 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) - } else { - // This notification is for another conversation that's still unread. - // We won't cancel the summary notification. - haveRemaining = true } } - - if (!haveRemaining) { - // The notification group is now empty; it had no notifications we didn't - // just cancel, except the summary notification. Cancel that one too. - NotificationManagerCompat.from(context).cancel(groupKey, NOTIFICATION_ID) - } } /** @@ -345,31 +330,10 @@ private fun updateNotification( setAutoCancel(true) }.build() - val summaryNotification = NotificationCompat.Builder(context, CHANNEL_ID).apply { - setGroup(groupKey) - setGroupSummary(true) - - color = context.getColor(R.color.brandColor) - setSmallIcon(if (BuildConfig.DEBUG) R.mipmap.ic_launcher else R.drawable.zulip_notification) - - // For the summary we use an "inbox-style" notification, as recommended here: - // https://developer.android.com/training/notify-user/group#set_a_group_summary - setStyle(NotificationCompat.InboxStyle() - // TODO(#5115): Use the org's friendly name instead of its URL. - .setSummaryText(fcmMessage.identity.realmUri.toString()) - // TODO: Use addLine and setBigContentTitle to add some summary info when collapsed? - // (See example in the linked doc.) - ) - - // TODO Does this do something useful? There isn't a way to open these summary notifs. - setAutoCancel(true) - }.build() - NotificationManagerCompat.from(context).apply { // This posts the notifications. If there is an existing notification // with the same tag and ID as one of these calls to `notify`, this will // replace it with the updated notification we've just constructed. - notify(groupKey, NOTIFICATION_ID, summaryNotification) notify(conversationKey, NOTIFICATION_ID, notification) } }