Model hiding the footer in the NotificationListViewModel
Sometimes, when opening the shade, the quick settings panel would jump
to a new position after the initial animation (see recording in bug).
This happens because with the refactored footer code, we set the footer
visibility to GONE (via StackScrollerDecorView#setVisible) as long as
the shade is closed, which means it was sometimes not being
counted when measuring the height of the NSSL for the animation (in
NPVC#calculatePanelHeightShade). In the old code, we were setting it
to INVISIBLE instead.
This fixes this bug by modeling this "hidden" state that is needed in
the StackScrollAlgorithm in the NotificationListViewModel. I had an
attempt to include it in the VisibilityChange object we've been using
so far for the footer visibility, but that makes things harder to read
since this hidden state is handled differently and without needing to
be animated.
I have an alternative solution for this at ag/26581241 but it's hackier,
so I believe this to be a better approach that preserves the
pre-refactor behavior.
Fix: 329432313
Test: open shade in various ways (different gestures, screen size,
orientation) to check that the bug doesn't happen anymore
Test: NotificationListViewModelTest
Flag: ACONFIG com.android.systemui.notifications_footer_view_refactor STAGING
Change-Id: I8d34e74fc1d3939176132850d1d962ec5c7bccd2
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/footer/ui/view/FooterView.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/footer/ui/view/FooterView.java
index f792898..adcbbfb 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/footer/ui/view/FooterView.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/footer/ui/view/FooterView.java
@@ -58,6 +58,7 @@
private FooterViewButton mClearAllButton;
private FooterViewButton mManageOrHistoryButton;
+ private boolean mShouldBeHidden;
private boolean mShowHistory;
// String cache, for performance reasons.
// Reading them from a Resources object can be quite slow sometimes.
@@ -110,6 +111,20 @@
setSecondaryVisible(visible, animate, onAnimationEnded);
}
+ /** See {@link this#setShouldBeHidden} below. */
+ public boolean shouldBeHidden() {
+ return mShouldBeHidden;
+ }
+
+ /**
+ * Whether this view's visibility should be set to INVISIBLE. Note that this is different from
+ * the {@link StackScrollerDecorView#setVisible} method, which in turn handles visibility
+ * transitions between VISIBLE and GONE.
+ */
+ public void setShouldBeHidden(boolean hide) {
+ mShouldBeHidden = hide;
+ }
+
@Override
public void dump(PrintWriter pwOriginal, String[] args) {
IndentingPrintWriter pw = DumpUtilsKt.asIndenting(pwOriginal);
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/StackScrollAlgorithm.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/StackScrollAlgorithm.java
index b42c07d..5eaccd9 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/StackScrollAlgorithm.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/StackScrollAlgorithm.java
@@ -594,15 +594,16 @@
);
if (view instanceof FooterView) {
if (FooterViewRefactor.isEnabled()) {
- final float footerEnd = algorithmState.mCurrentExpandedYPosition
- + view.getIntrinsicHeight();
- final boolean noSpaceForFooter = footerEnd > ambientState.getStackEndHeight();
- // TODO(b/293167744): May be able to keep only noSpaceForFooter here if we add an
- // emission when clearAllNotifications is called, and then use that in the footer
- // visibility flow.
- ((FooterView.FooterViewState) viewState).hideContent =
- noSpaceForFooter || (ambientState.isClearAllInProgress()
- && !hasNonClearableNotifs(algorithmState));
+ if (((FooterView) view).shouldBeHidden()) {
+ viewState.hidden = true;
+ } else {
+ final float footerEnd = algorithmState.mCurrentExpandedYPosition
+ + view.getIntrinsicHeight();
+ final boolean noSpaceForFooter = footerEnd > ambientState.getStackEndHeight();
+ ((FooterView.FooterViewState) viewState).hideContent =
+ noSpaceForFooter || (ambientState.isClearAllInProgress()
+ && !hasNonClearableNotifs(algorithmState));
+ }
} else {
final boolean shadeClosed = !ambientState.isShadeExpanded();
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewbinder/NotificationListViewBinder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewbinder/NotificationListViewBinder.kt
index 6b30393..ac7dc9b 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewbinder/NotificationListViewBinder.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewbinder/NotificationListViewBinder.kt
@@ -187,13 +187,14 @@
},
)
launch {
- viewModel.shouldShowFooterView.collect { animatedVisibility ->
+ viewModel.shouldIncludeFooterView.collect { animatedVisibility ->
footerView.setVisible(
/* visible = */ animatedVisibility.value,
/* animate = */ animatedVisibility.isAnimating,
)
}
}
+ launch { viewModel.shouldHideFooterView.collect { footerView.setShouldBeHidden(it) } }
disposableHandle.awaitCancellationThenDispose()
}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModel.kt
index 4744fcb..dac8c6b 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModel.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModel.kt
@@ -27,7 +27,6 @@
import com.android.systemui.statusbar.notification.stack.domain.interactor.NotificationStackInteractor
import com.android.systemui.statusbar.policy.domain.interactor.UserSetupInteractor
import com.android.systemui.statusbar.policy.domain.interactor.ZenModeInteractor
-import com.android.systemui.util.kotlin.combine
import com.android.systemui.util.kotlin.sample
import com.android.systemui.util.ui.AnimatableEvent
import com.android.systemui.util.ui.AnimatedValue
@@ -105,7 +104,32 @@
}
}
- val shouldShowFooterView: Flow<AnimatedValue<Boolean>> by lazy {
+ /**
+ * Whether the footer should not be visible for the user, even if it's present in the list (as
+ * per [shouldIncludeFooterView] below).
+ *
+ * This essentially corresponds to having the view set to INVISIBLE.
+ */
+ val shouldHideFooterView: Flow<Boolean> by lazy {
+ if (FooterViewRefactor.isUnexpectedlyInLegacyMode()) {
+ flowOf(false)
+ } else {
+ // When the shade is closed, the footer is still present in the list, but not visible.
+ // This prevents the footer from being shown when a HUN is present, while still allowing
+ // the footer to be counted as part of the shade for measurements.
+ shadeInteractor.shadeExpansion.map { it == 0f }.distinctUntilChanged()
+ }
+ }
+
+ /**
+ * Whether the footer should be part of the list or not, and whether the transition from one
+ * state to another should be animated. This essentially corresponds to transitioning the view
+ * visibility from VISIBLE to GONE and vice versa.
+ *
+ * Note that this value being true doesn't necessarily mean that the footer is visible. It could
+ * be hidden by another condition (see [shouldHideFooterView] above).
+ */
+ val shouldIncludeFooterView: Flow<AnimatedValue<Boolean>> by lazy {
if (FooterViewRefactor.isUnexpectedlyInLegacyMode()) {
flowOf(AnimatedValue.NotAnimating(false))
} else {
@@ -114,34 +138,30 @@
userSetupInteractor.isUserSetUp,
notificationStackInteractor.isShowingOnLockscreen,
shadeInteractor.isQsFullscreen,
- remoteInputInteractor.isRemoteInputActive,
- shadeInteractor.shadeExpansion.map { it == 0f }.distinctUntilChanged(),
+ remoteInputInteractor.isRemoteInputActive
) {
hasNotifications,
isUserSetUp,
isShowingOnLockscreen,
qsFullScreen,
- isRemoteInputActive,
- isShadeClosed ->
+ isRemoteInputActive ->
when {
- !hasNotifications -> VisibilityChange.HIDE_WITH_ANIMATION
+ !hasNotifications -> VisibilityChange.DISAPPEAR_WITH_ANIMATION
// Hide the footer until the user setup is complete, to prevent access
// to settings (b/193149550).
- !isUserSetUp -> VisibilityChange.HIDE_WITH_ANIMATION
+ !isUserSetUp -> VisibilityChange.DISAPPEAR_WITH_ANIMATION
// Do not show the footer if the lockscreen is visible (incl. AOD),
// except if the shade is opened on top. See also b/219680200.
// Do not animate, as that makes the footer appear briefly when
// transitioning between the shade and keyguard.
- isShowingOnLockscreen -> VisibilityChange.HIDE_WITHOUT_ANIMATION
+ isShowingOnLockscreen -> VisibilityChange.DISAPPEAR_WITHOUT_ANIMATION
// Do not show the footer if quick settings are fully expanded (except
// for the foldable split shade view). See b/201427195 && b/222699879.
- qsFullScreen -> VisibilityChange.HIDE_WITH_ANIMATION
+ qsFullScreen -> VisibilityChange.DISAPPEAR_WITH_ANIMATION
// Hide the footer if remote input is active (i.e. user is replying to a
// notification). See b/75984847.
- isRemoteInputActive -> VisibilityChange.HIDE_WITH_ANIMATION
- // Never show the footer if the shade is collapsed (e.g. when HUNing).
- isShadeClosed -> VisibilityChange.HIDE_WITHOUT_ANIMATION
- else -> VisibilityChange.SHOW_WITH_ANIMATION
+ isRemoteInputActive -> VisibilityChange.DISAPPEAR_WITH_ANIMATION
+ else -> VisibilityChange.APPEAR_WITH_ANIMATION
}
}
.flowOn(bgDispatcher)
@@ -174,9 +194,9 @@
}
enum class VisibilityChange(val visible: Boolean, val canAnimate: Boolean) {
- HIDE_WITHOUT_ANIMATION(visible = false, canAnimate = false),
- HIDE_WITH_ANIMATION(visible = false, canAnimate = true),
- SHOW_WITH_ANIMATION(visible = true, canAnimate = true)
+ DISAPPEAR_WITHOUT_ANIMATION(visible = false, canAnimate = false),
+ DISAPPEAR_WITH_ANIMATION(visible = false, canAnimate = true),
+ APPEAR_WITH_ANIMATION(visible = true, canAnimate = true)
}
// TODO(b/308591475): This should be tracked separately by the empty shade.
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModelTest.kt
index 0a18eb6..091db7d 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModelTest.kt
@@ -125,35 +125,35 @@
}
@Test
- fun testShouldShowEmptyShadeView_trueWhenNoNotifs() =
+ fun testShouldIncludeEmptyShadeView_trueWhenNoNotifs() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowEmptyShadeView)
+ val shouldInclude by collectLastValue(underTest.shouldShowEmptyShadeView)
// WHEN has no notifs
activeNotificationListRepository.setActiveNotifs(count = 0)
runCurrent()
// THEN empty shade is visible
- assertThat(shouldShow).isTrue()
+ assertThat(shouldInclude).isTrue()
}
@Test
- fun testShouldShowEmptyShadeView_falseWhenNotifs() =
+ fun testShouldIncludeEmptyShadeView_falseWhenNotifs() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowEmptyShadeView)
+ val shouldInclude by collectLastValue(underTest.shouldShowEmptyShadeView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
runCurrent()
// THEN empty shade is not visible
- assertThat(shouldShow).isFalse()
+ assertThat(shouldInclude).isFalse()
}
@Test
- fun testShouldShowEmptyShadeView_falseWhenQsExpandedDefault() =
+ fun testShouldIncludeEmptyShadeView_falseWhenQsExpandedDefault() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowEmptyShadeView)
+ val shouldInclude by collectLastValue(underTest.shouldShowEmptyShadeView)
// WHEN has no notifs
activeNotificationListRepository.setActiveNotifs(count = 0)
@@ -162,13 +162,13 @@
runCurrent()
// THEN empty shade is not visible
- assertThat(shouldShow).isFalse()
+ assertThat(shouldInclude).isFalse()
}
@Test
- fun testShouldShowEmptyShadeView_trueWhenQsExpandedInSplitShade() =
+ fun testShouldIncludeEmptyShadeView_trueWhenQsExpandedInSplitShade() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowEmptyShadeView)
+ val shouldInclude by collectLastValue(underTest.shouldShowEmptyShadeView)
// WHEN has no notifs
activeNotificationListRepository.setActiveNotifs(count = 0)
@@ -180,13 +180,13 @@
runCurrent()
// THEN empty shade is visible
- assertThat(shouldShow).isTrue()
+ assertThat(shouldInclude).isTrue()
}
@Test
- fun testShouldShowEmptyShadeView_trueWhenLockedShade() =
+ fun testShouldIncludeEmptyShadeView_trueWhenLockedShade() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowEmptyShadeView)
+ val shouldInclude by collectLastValue(underTest.shouldShowEmptyShadeView)
// WHEN has no notifs
activeNotificationListRepository.setActiveNotifs(count = 0)
@@ -195,13 +195,13 @@
runCurrent()
// THEN empty shade is visible
- assertThat(shouldShow).isTrue()
+ assertThat(shouldInclude).isTrue()
}
@Test
- fun testShouldShowEmptyShadeView_falseWhenKeyguard() =
+ fun testShouldIncludeEmptyShadeView_falseWhenKeyguard() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowEmptyShadeView)
+ val shouldInclude by collectLastValue(underTest.shouldShowEmptyShadeView)
// WHEN has no notifs
activeNotificationListRepository.setActiveNotifs(count = 0)
@@ -210,13 +210,13 @@
runCurrent()
// THEN empty shade is not visible
- assertThat(shouldShow).isFalse()
+ assertThat(shouldInclude).isFalse()
}
@Test
- fun testShouldShowEmptyShadeView_falseWhenStartingToSleep() =
+ fun testShouldIncludeEmptyShadeView_falseWhenStartingToSleep() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowEmptyShadeView)
+ val shouldInclude by collectLastValue(underTest.shouldShowEmptyShadeView)
// WHEN has no notifs
activeNotificationListRepository.setActiveNotifs(count = 0)
@@ -227,7 +227,7 @@
runCurrent()
// THEN empty shade is not visible
- assertThat(shouldShow).isFalse()
+ assertThat(shouldInclude).isFalse()
}
@Test
@@ -277,9 +277,9 @@
}
@Test
- fun testShouldShowFooterView_trueWhenShade() =
+ fun testShouldIncludeFooterView_trueWhenShade() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -289,13 +289,13 @@
runCurrent()
// THEN footer is visible
- assertThat(shouldShow?.value).isTrue()
+ assertThat(shouldInclude?.value).isTrue()
}
@Test
- fun testShouldShowFooterView_trueWhenLockedShade() =
+ fun testShouldIncludeFooterView_trueWhenLockedShade() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -305,13 +305,13 @@
runCurrent()
// THEN footer is visible
- assertThat(shouldShow?.value).isTrue()
+ assertThat(shouldInclude?.value).isTrue()
}
@Test
- fun testShouldShowFooterView_falseWhenKeyguard() =
+ fun testShouldIncludeFooterView_falseWhenKeyguard() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -320,13 +320,13 @@
runCurrent()
// THEN footer is not visible
- assertThat(shouldShow?.value).isFalse()
+ assertThat(shouldInclude?.value).isFalse()
}
@Test
- fun testShouldShowFooterView_falseWhenUserNotSetUp() =
+ fun testShouldIncludeFooterView_falseWhenUserNotSetUp() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -338,13 +338,13 @@
runCurrent()
// THEN footer is not visible
- assertThat(shouldShow?.value).isFalse()
+ assertThat(shouldInclude?.value).isFalse()
}
@Test
- fun testShouldShowFooterView_falseWhenStartingToSleep() =
+ fun testShouldIncludeFooterView_falseWhenStartingToSleep() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -356,13 +356,13 @@
runCurrent()
// THEN footer is not visible
- assertThat(shouldShow?.value).isFalse()
+ assertThat(shouldInclude?.value).isFalse()
}
@Test
- fun testShouldShowFooterView_falseWhenQsExpandedDefault() =
+ fun testShouldIncludeFooterView_falseWhenQsExpandedDefault() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -375,13 +375,13 @@
runCurrent()
// THEN footer is not visible
- assertThat(shouldShow?.value).isFalse()
+ assertThat(shouldInclude?.value).isFalse()
}
@Test
- fun testShouldShowFooterView_trueWhenQsExpandedSplitShade() =
+ fun testShouldIncludeFooterView_trueWhenQsExpandedSplitShade() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -396,13 +396,13 @@
runCurrent()
// THEN footer is visible
- assertThat(shouldShow?.value).isTrue()
+ assertThat(shouldInclude?.value).isTrue()
}
@Test
- fun testShouldShowFooterView_falseWhenRemoteInputActive() =
+ fun testShouldIncludeFooterView_falseWhenRemoteInputActive() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -414,29 +414,13 @@
runCurrent()
// THEN footer is not visible
- assertThat(shouldShow?.value).isFalse()
+ assertThat(shouldInclude?.value).isFalse()
}
@Test
- fun testShouldShowFooterView_falseWhenShadeIsClosed() =
+ fun testShouldIncludeFooterView_animatesWhenShade() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
-
- // WHEN has notifs
- activeNotificationListRepository.setActiveNotifs(count = 2)
- // AND shade is closed
- fakeKeyguardRepository.setStatusBarState(StatusBarState.SHADE)
- fakeShadeRepository.setLegacyShadeExpansion(0f)
- runCurrent()
-
- // THEN footer is not visible
- assertThat(shouldShow?.value).isFalse()
- }
-
- @Test
- fun testShouldShowFooterView_animatesWhenShade() =
- testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -446,13 +430,13 @@
runCurrent()
// THEN footer visibility animates
- assertThat(shouldShow?.isAnimating).isTrue()
+ assertThat(shouldInclude?.isAnimating).isTrue()
}
@Test
- fun testShouldShowFooterView_notAnimatingOnKeyguard() =
+ fun testShouldIncludeFooterView_notAnimatingOnKeyguard() =
testScope.runTest {
- val shouldShow by collectLastValue(underTest.shouldShowFooterView)
+ val shouldInclude by collectLastValue(underTest.shouldIncludeFooterView)
// WHEN has notifs
activeNotificationListRepository.setActiveNotifs(count = 2)
@@ -462,6 +446,34 @@
runCurrent()
// THEN footer visibility does not animate
- assertThat(shouldShow?.isAnimating).isFalse()
+ assertThat(shouldInclude?.isAnimating).isFalse()
+ }
+
+ @Test
+ fun testShouldHideFooterView_trueWhenShadeIsClosed() =
+ testScope.runTest {
+ val shouldHide by collectLastValue(underTest.shouldHideFooterView)
+
+ // WHEN shade is closed
+ fakeKeyguardRepository.setStatusBarState(StatusBarState.SHADE)
+ fakeShadeRepository.setLegacyShadeExpansion(0f)
+ runCurrent()
+
+ // THEN footer is hidden
+ assertThat(shouldHide).isTrue()
+ }
+
+ @Test
+ fun testShouldHideFooterView_falseWhenShadeIsOpen() =
+ testScope.runTest {
+ val shouldHide by collectLastValue(underTest.shouldHideFooterView)
+
+ // WHEN shade is open
+ fakeKeyguardRepository.setStatusBarState(StatusBarState.SHADE)
+ fakeShadeRepository.setLegacyShadeExpansion(1f)
+ runCurrent()
+
+ // THEN footer is hidden
+ assertThat(shouldHide).isFalse()
}
}