diff options
| author | 2025-01-02 19:38:48 +0000 | |
|---|---|---|
| committer | 2025-01-08 20:56:51 +0000 | |
| commit | 33c2d33358a41ceb0976eee2bcd6ceae2f6f480a (patch) | |
| tree | 4ff234444f2e2d1eac68f60e08e2a0bbd6c60039 | |
| parent | 3a0b553620bbc24bd45a6a8983835cd26fa6853a (diff) | |
[SB][Notif] Only hide text in chip that was actually tapped.
Previously, if any notification chip was tapped, we'd hide the text in
all chips. Instead, we should just hide the text in the chip that was
actually tapped.
Most of this CL is adding a new flow to `HeadsUpNotificationInteractor`
that emits not just the current `PinnedStatus` but also the key for the
pinned row.
Fixes: 386784519
Bug: 364653005
Flag: com.android.systemui.status_bar_notification_chips
Test: atest HeadsUpNotificationInteractorTest NotifChipsViewModelTest
Test: Have 2 notification chips, tap on one -> verify only that chip
hides text. Tap on other -> verify other chip hides text, previous chip
shows text
Change-Id: I7332e40565ed5b6892645168626a78f31461e243
7 files changed, 223 insertions, 40 deletions
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/chips/notification/ui/viewmodel/NotifChipsViewModelTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/chips/notification/ui/viewmodel/NotifChipsViewModelTest.kt index e561e3ea27d7..902db5e10589 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/chips/notification/ui/viewmodel/NotifChipsViewModelTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/chips/notification/ui/viewmodel/NotifChipsViewModelTest.kt @@ -501,7 +501,7 @@ class NotifChipsViewModelTest : SysuiTestCase() { @Test @DisableFlags(FLAG_PROMOTE_NOTIFICATIONS_AUTOMATICALLY) - fun chips_hasHeadsUpByUser_onlyShowsIcon() = + fun chips_hasHeadsUpBySystem_showsTime() = kosmos.runTest { val latest by collectLastValue(underTest.chips) @@ -523,7 +523,99 @@ class NotifChipsViewModelTest : SysuiTestCase() { ) ) - // WHEN there's a HUN pinned by a user + // WHEN there's a HUN pinned by the system + kosmos.headsUpNotificationRepository.setNotifications( + UnconfinedFakeHeadsUpRowRepository( + key = "notif", + pinnedStatus = MutableStateFlow(PinnedStatus.PinnedBySystem), + ) + ) + + // THEN the chip keeps showing time + // (In real life the chip won't show at all, but that's handled in a different part of + // the system. What we know here is that the chip shouldn't shrink to icon only.) + assertThat(latest!![0]) + .isInstanceOf(OngoingActivityChipModel.Shown.ShortTimeDelta::class.java) + } + + @Test + @DisableFlags(FLAG_PROMOTE_NOTIFICATIONS_AUTOMATICALLY) + fun chips_hasHeadsUpByUser_forOtherNotif_showsTime() = + kosmos.runTest { + val latest by collectLastValue(underTest.chips) + + val promotedContentBuilder = + PromotedNotificationContentModel.Builder("notif").apply { + this.time = + PromotedNotificationContentModel.When( + time = 6543L, + mode = PromotedNotificationContentModel.When.Mode.BasicTime, + ) + } + val otherPromotedContentBuilder = + PromotedNotificationContentModel.Builder("other notif").apply { + this.time = + PromotedNotificationContentModel.When( + time = 654321L, + mode = PromotedNotificationContentModel.When.Mode.BasicTime, + ) + } + val icon = createStatusBarIconViewOrNull() + val otherIcon = createStatusBarIconViewOrNull() + setNotifs( + listOf( + activeNotificationModel( + key = "notif", + statusBarChipIcon = icon, + promotedContent = promotedContentBuilder.build(), + ), + activeNotificationModel( + key = "other notif", + statusBarChipIcon = otherIcon, + promotedContent = otherPromotedContentBuilder.build(), + ), + ) + ) + + // WHEN there's a HUN pinned for the "other notif" chip + kosmos.headsUpNotificationRepository.setNotifications( + UnconfinedFakeHeadsUpRowRepository( + key = "other notif", + pinnedStatus = MutableStateFlow(PinnedStatus.PinnedByUser), + ) + ) + + // THEN the "notif" chip keeps showing time + val chip = latest!![0] + assertThat(chip).isInstanceOf(OngoingActivityChipModel.Shown.ShortTimeDelta::class.java) + assertIsNotifChip(chip, icon, "notif") + } + + @Test + @DisableFlags(FLAG_PROMOTE_NOTIFICATIONS_AUTOMATICALLY) + fun chips_hasHeadsUpByUser_forThisNotif_onlyShowsIcon() = + kosmos.runTest { + val latest by collectLastValue(underTest.chips) + + val promotedContentBuilder = + PromotedNotificationContentModel.Builder("notif").apply { + this.time = + PromotedNotificationContentModel.When( + time = 6543L, + mode = PromotedNotificationContentModel.When.Mode.BasicTime, + ) + } + setNotifs( + listOf( + activeNotificationModel( + key = "notif", + statusBarChipIcon = mock<StatusBarIconView>(), + promotedContent = promotedContentBuilder.build(), + ) + ) + ) + + // WHEN this notification is pinned by the user kosmos.headsUpNotificationRepository.setNotifications( UnconfinedFakeHeadsUpRowRepository( key = "notif", @@ -531,6 +623,7 @@ class NotifChipsViewModelTest : SysuiTestCase() { ) ) + // THEN the chip shrinks to icon only assertThat(latest!![0]) .isInstanceOf(OngoingActivityChipModel.Shown.IconOnly::class.java) } diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/domain/interactor/HeadsUpNotificationInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/domain/interactor/HeadsUpNotificationInteractorTest.kt index 22ef408e266c..fae7d515d305 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/domain/interactor/HeadsUpNotificationInteractorTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/domain/interactor/HeadsUpNotificationInteractorTest.kt @@ -32,6 +32,7 @@ import com.android.systemui.shade.data.repository.fakeShadeRepository import com.android.systemui.shade.shadeTestUtil import com.android.systemui.statusbar.notification.data.repository.FakeHeadsUpRowRepository import com.android.systemui.statusbar.notification.data.repository.notificationsKeyguardViewStateRepository +import com.android.systemui.statusbar.notification.domain.model.TopPinnedState import com.android.systemui.statusbar.notification.headsup.PinnedStatus import com.android.systemui.statusbar.notification.stack.data.repository.headsUpNotificationRepository import com.android.systemui.statusbar.notification.stack.domain.interactor.headsUpNotificationInteractor @@ -412,46 +413,53 @@ class HeadsUpNotificationInteractorTest : SysuiTestCase() { @Test fun statusBarHeadsUpState_pinnedBySystem() = testScope.runTest { - val statusBarHeadsUpState by collectLastValue(underTest.statusBarHeadsUpState) + val state by collectLastValue(underTest.statusBarHeadsUpState) + val status by collectLastValue(underTest.statusBarHeadsUpStatus) headsUpRepository.setNotifications( FakeHeadsUpRowRepository(key = "key 0", pinnedStatus = PinnedStatus.PinnedBySystem) ) runCurrent() - assertThat(statusBarHeadsUpState).isEqualTo(PinnedStatus.PinnedBySystem) + assertThat(state).isEqualTo(TopPinnedState.Pinned("key 0", PinnedStatus.PinnedBySystem)) + assertThat(status).isEqualTo(PinnedStatus.PinnedBySystem) } @Test fun statusBarHeadsUpState_pinnedByUser() = testScope.runTest { - val statusBarHeadsUpState by collectLastValue(underTest.statusBarHeadsUpState) + val state by collectLastValue(underTest.statusBarHeadsUpState) + val status by collectLastValue(underTest.statusBarHeadsUpStatus) headsUpRepository.setNotifications( FakeHeadsUpRowRepository(key = "key 0", pinnedStatus = PinnedStatus.PinnedByUser) ) runCurrent() - assertThat(statusBarHeadsUpState).isEqualTo(PinnedStatus.PinnedByUser) + assertThat(state).isEqualTo(TopPinnedState.Pinned("key 0", PinnedStatus.PinnedByUser)) + assertThat(status).isEqualTo(PinnedStatus.PinnedByUser) } @Test fun statusBarHeadsUpState_withoutPinnedNotifications_notPinned() = testScope.runTest { - val statusBarHeadsUpState by collectLastValue(underTest.statusBarHeadsUpState) + val state by collectLastValue(underTest.statusBarHeadsUpState) + val status by collectLastValue(underTest.statusBarHeadsUpStatus) headsUpRepository.setNotifications( FakeHeadsUpRowRepository(key = "key 0", PinnedStatus.NotPinned) ) runCurrent() - assertThat(statusBarHeadsUpState).isEqualTo(PinnedStatus.NotPinned) + assertThat(state).isEqualTo(TopPinnedState.NothingPinned) + assertThat(status).isEqualTo(PinnedStatus.NotPinned) } @Test fun statusBarHeadsUpState_whenShadeExpanded_false() = testScope.runTest { - val statusBarHeadsUpState by collectLastValue(underTest.statusBarHeadsUpState) + val state by collectLastValue(underTest.statusBarHeadsUpState) + val status by collectLastValue(underTest.statusBarHeadsUpStatus) // WHEN a row is pinned headsUpRepository.setNotifications(fakeHeadsUpRowRepository("key 0", isPinned = true)) @@ -463,13 +471,15 @@ class HeadsUpNotificationInteractorTest : SysuiTestCase() { // should emit `false`. kosmos.fakeShadeRepository.setLegacyShadeExpansion(1.0f) - assertThat(statusBarHeadsUpState!!.isPinned).isFalse() + assertThat(state).isEqualTo(TopPinnedState.NothingPinned) + assertThat(status!!.isPinned).isFalse() } @Test fun statusBarHeadsUpState_notificationsAreHidden_false() = testScope.runTest { - val statusBarHeadsUpState by collectLastValue(underTest.statusBarHeadsUpState) + val state by collectLastValue(underTest.statusBarHeadsUpState) + val status by collectLastValue(underTest.statusBarHeadsUpStatus) // WHEN a row is pinned headsUpRepository.setNotifications(fakeHeadsUpRowRepository("key 0", isPinned = true)) @@ -477,13 +487,15 @@ class HeadsUpNotificationInteractorTest : SysuiTestCase() { // AND the notifications are hidden keyguardViewStateRepository.areNotificationsFullyHidden.value = true - assertThat(statusBarHeadsUpState!!.isPinned).isFalse() + assertThat(state).isEqualTo(TopPinnedState.NothingPinned) + assertThat(status!!.isPinned).isFalse() } @Test fun statusBarHeadsUpState_onLockScreen_false() = testScope.runTest { - val statusBarHeadsUpState by collectLastValue(underTest.statusBarHeadsUpState) + val state by collectLastValue(underTest.statusBarHeadsUpState) + val status by collectLastValue(underTest.statusBarHeadsUpStatus) // WHEN a row is pinned headsUpRepository.setNotifications(fakeHeadsUpRowRepository("key 0", isPinned = true)) @@ -494,13 +506,15 @@ class HeadsUpNotificationInteractorTest : SysuiTestCase() { testSetup = true, ) - assertThat(statusBarHeadsUpState!!.isPinned).isFalse() + assertThat(state).isEqualTo(TopPinnedState.NothingPinned) + assertThat(status!!.isPinned).isFalse() } @Test fun statusBarHeadsUpState_onByPassLockScreen_true() = testScope.runTest { - val statusBarHeadsUpState by collectLastValue(underTest.statusBarHeadsUpState) + val state by collectLastValue(underTest.statusBarHeadsUpState) + val status by collectLastValue(underTest.statusBarHeadsUpStatus) // WHEN a row is pinned headsUpRepository.setNotifications(fakeHeadsUpRowRepository("key 0", isPinned = true)) @@ -513,13 +527,15 @@ class HeadsUpNotificationInteractorTest : SysuiTestCase() { // AND bypass is enabled faceAuthRepository.isBypassEnabled.value = true - assertThat(statusBarHeadsUpState!!.isPinned).isTrue() + assertThat(state).isInstanceOf(TopPinnedState.Pinned::class.java) + assertThat(status!!.isPinned).isTrue() } @Test fun statusBarHeadsUpState_onByPassLockScreen_withoutNotifications_false() = testScope.runTest { - val statusBarHeadsUpState by collectLastValue(underTest.statusBarHeadsUpState) + val state by collectLastValue(underTest.statusBarHeadsUpState) + val status by collectLastValue(underTest.statusBarHeadsUpStatus) // WHEN no pinned rows // AND the lock screen is shown @@ -530,7 +546,8 @@ class HeadsUpNotificationInteractorTest : SysuiTestCase() { // AND bypass is enabled faceAuthRepository.isBypassEnabled.value = true - assertThat(statusBarHeadsUpState!!.isPinned).isFalse() + assertThat(state).isEqualTo(TopPinnedState.NothingPinned) + assertThat(status!!.isPinned).isFalse() } private fun fakeHeadsUpRowRepository(key: String, isPinned: Boolean = false) = diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/ui/viewmodel/NotifChipsViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/ui/viewmodel/NotifChipsViewModel.kt index 2f6431b05c8b..ec3a5b271e35 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/ui/viewmodel/NotifChipsViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/chips/notification/ui/viewmodel/NotifChipsViewModel.kt @@ -27,6 +27,7 @@ import com.android.systemui.statusbar.chips.ui.model.ColorsModel import com.android.systemui.statusbar.chips.ui.model.OngoingActivityChipModel import com.android.systemui.statusbar.core.StatusBarConnectedDisplays import com.android.systemui.statusbar.notification.domain.interactor.HeadsUpNotificationInteractor +import com.android.systemui.statusbar.notification.domain.model.TopPinnedState import com.android.systemui.statusbar.notification.headsup.PinnedStatus import com.android.systemui.statusbar.notification.promoted.shared.model.PromotedNotificationContentModel import javax.inject.Inject @@ -60,7 +61,7 @@ constructor( /** Converts the notification to the [OngoingActivityChipModel] object. */ private fun NotificationChipModel.toActivityChipModel( - headsUpState: PinnedStatus + headsUpState: TopPinnedState ): OngoingActivityChipModel.Shown { StatusBarNotifChips.assertInNewMode() val icon = @@ -87,8 +88,12 @@ constructor( } } - if (headsUpState == PinnedStatus.PinnedByUser) { - // If the user tapped the chip to show the HUN, we want to just show the icon because + val isShowingHeadsUpFromChipTap = + headsUpState is TopPinnedState.Pinned && + headsUpState.status == PinnedStatus.PinnedByUser && + headsUpState.key == this.key + if (isShowingHeadsUpFromChipTap) { + // If the user tapped this chip to show the HUN, we want to just show the icon because // the HUN will show the rest of the information. return OngoingActivityChipModel.Shown.IconOnly(icon, colors, onClickListener) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/domain/interactor/HeadsUpNotificationInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/domain/interactor/HeadsUpNotificationInteractor.kt index 75c7d2d5be98..6140c92369b3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/domain/interactor/HeadsUpNotificationInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/domain/interactor/HeadsUpNotificationInteractor.kt @@ -25,6 +25,7 @@ import com.android.systemui.scene.shared.flag.SceneContainerFlag import com.android.systemui.shade.domain.interactor.ShadeInteractor import com.android.systemui.statusbar.notification.data.repository.HeadsUpRepository import com.android.systemui.statusbar.notification.data.repository.HeadsUpRowRepository +import com.android.systemui.statusbar.notification.domain.model.TopPinnedState import com.android.systemui.statusbar.notification.headsup.PinnedStatus import com.android.systemui.statusbar.notification.shared.HeadsUpRowKey import javax.inject.Inject @@ -98,21 +99,39 @@ constructor( } } - /** What [PinnedStatus] does the top row have? */ - private val topPinnedStatus: Flow<PinnedStatus> = + /** What [PinnedStatus] and key does the top row have? */ + private val topPinnedState: Flow<TopPinnedState> = headsUpRepository.activeHeadsUpRows.flatMapLatest { rows -> if (rows.isNotEmpty()) { - combine(rows.map { it.pinnedStatus }) { pinnedStatus -> - pinnedStatus.firstOrNull { it.isPinned } ?: PinnedStatus.NotPinned + // For each row, emits a (key, pinnedStatus) pair each time any row's + // `pinnedStatus` changes + val toCombine: List<Flow<Pair<String, PinnedStatus>>> = + rows.map { row -> row.pinnedStatus.map { status -> row.key to status } } + combine(toCombine) { pairs -> + val topPinnedRow: Pair<String, PinnedStatus>? = + pairs.firstOrNull { it.second.isPinned } + if (topPinnedRow != null) { + TopPinnedState.Pinned( + key = topPinnedRow.first, + status = topPinnedRow.second, + ) + } else { + TopPinnedState.NothingPinned + } } } else { - // if the set is empty, there are no flows to combine - flowOf(PinnedStatus.NotPinned) + flowOf(TopPinnedState.NothingPinned) } } /** Are there any pinned heads up rows to display? */ - val hasPinnedRows: Flow<Boolean> = topPinnedStatus.map { it.isPinned } + val hasPinnedRows: Flow<Boolean> = + topPinnedState.map { + when (it) { + is TopPinnedState.Pinned -> it.status.isPinned + is TopPinnedState.NothingPinned -> false + } + } val isHeadsUpOrAnimatingAway: Flow<Boolean> by lazy { if (SceneContainerFlag.isUnexpectedlyInLegacyMode()) { @@ -142,13 +161,25 @@ constructor( } } - /** Emits the pinned notification state as it relates to the status bar. */ - val statusBarHeadsUpState: Flow<PinnedStatus> = - combine(topPinnedStatus, canShowHeadsUp) { topPinnedStatus, canShowHeadsUp -> + /** + * Emits the pinned notification state as it relates to the status bar. Includes both the pinned + * status and key of the notification that's pinned (if there is a pinned notification). + */ + val statusBarHeadsUpState: Flow<TopPinnedState> = + combine(topPinnedState, canShowHeadsUp) { topPinnedState, canShowHeadsUp -> if (canShowHeadsUp) { - topPinnedStatus + topPinnedState } else { - PinnedStatus.NotPinned + TopPinnedState.NothingPinned + } + } + + /** Emits the pinned notification status as it relates to the status bar. */ + val statusBarHeadsUpStatus: Flow<PinnedStatus> = + statusBarHeadsUpState.map { + when (it) { + is TopPinnedState.Pinned -> it.status + is TopPinnedState.NothingPinned -> PinnedStatus.NotPinned } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/domain/model/TopPinnedState.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/domain/model/TopPinnedState.kt new file mode 100644 index 000000000000..51c448adf998 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/domain/model/TopPinnedState.kt @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.notification.domain.model + +import com.android.systemui.statusbar.notification.headsup.PinnedStatus + +/** A class representing the state of the top pinned row. */ +sealed interface TopPinnedState { + /** Nothing is pinned. */ + data object NothingPinned : TopPinnedState + + /** + * The top pinned row is a notification with the given key and status. + * + * @property status must have [PinnedStatus.isPinned] as true. + */ + data class Pinned(val key: String, val status: PinnedStatus) : TopPinnedState { + init { + check(status.isPinned) + } + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/HomeStatusBarViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/HomeStatusBarViewModel.kt index d731752ad5d5..d9d9a29ee2b6 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/HomeStatusBarViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/HomeStatusBarViewModel.kt @@ -295,11 +295,12 @@ constructor( override val shouldShowOperatorNameView: Flow<Boolean> = combine( shouldHomeStatusBarBeVisible, - headsUpNotificationInteractor.statusBarHeadsUpState, + headsUpNotificationInteractor.statusBarHeadsUpStatus, homeStatusBarInteractor.visibilityViaDisableFlags, homeStatusBarInteractor.shouldShowOperatorName, - ) { shouldStatusBarBeVisible, headsUpState, visibilityViaDisableFlags, shouldShowOperator -> - val hideForHeadsUp = headsUpState == PinnedStatus.PinnedBySystem + ) { shouldStatusBarBeVisible, headsUpStatus, visibilityViaDisableFlags, shouldShowOperator + -> + val hideForHeadsUp = headsUpStatus == PinnedStatus.PinnedBySystem shouldStatusBarBeVisible && !hideForHeadsUp && visibilityViaDisableFlags.isSystemInfoAllowed && @@ -309,10 +310,10 @@ constructor( override val isClockVisible: Flow<VisibilityModel> = combine( shouldHomeStatusBarBeVisible, - headsUpNotificationInteractor.statusBarHeadsUpState, + headsUpNotificationInteractor.statusBarHeadsUpStatus, homeStatusBarInteractor.visibilityViaDisableFlags, - ) { shouldStatusBarBeVisible, headsUpState, visibilityViaDisableFlags -> - val hideClockForHeadsUp = headsUpState == PinnedStatus.PinnedBySystem + ) { shouldStatusBarBeVisible, headsUpStatus, visibilityViaDisableFlags -> + val hideClockForHeadsUp = headsUpStatus == PinnedStatus.PinnedBySystem val showClock = shouldStatusBarBeVisible && visibilityViaDisableFlags.isClockAllowed && diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/ui/viewmodel/KeyguardStatusBarViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/ui/viewmodel/KeyguardStatusBarViewModel.kt index 6175ea190697..a98a9e0c16d2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/ui/viewmodel/KeyguardStatusBarViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/ui/viewmodel/KeyguardStatusBarViewModel.kt @@ -60,7 +60,7 @@ constructor( private val showingHeadsUpStatusBar: Flow<Boolean> = if (SceneContainerFlag.isEnabled) { - headsUpNotificationInteractor.statusBarHeadsUpState.map { it.isPinned } + headsUpNotificationInteractor.statusBarHeadsUpStatus.map { it.isPinned } } else { flowOf(false) } |