Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d688bb02 authored by Ioana Alexandru's avatar Ioana Alexandru
Browse files

Properly hide footer buttons when label visible 

Previously, this was handled in FooterView#setFooterLabelVisible
directly. Pre-footer refactor, every time the footer was updated
(including button visibility), setFooterLabelVisible would be called at
the end and essentially override button visibilities if needed. In the
new code, button visibility would be updated independently, which could
lead to buttons showing up over the label.

Instead, we should model this in the FooterViewModel and make sure the
buttons are never visible when the label is visible.

Fix: 332438711
Test: FooterViewModelTest + manual check that clear all button doesn't
appear on tangor over "unlock to see notifications" text
Flag: ACONFIG com.android.systemui.notifications_footer_view_refactor TEAMFOOD

Change-Id: I78c2db21b3d1b7dd0301065f61b85186c214d4d8
parent ef54218e

packages/SystemUI/src/com/android/systemui/statusbar/notification/footer/ui/view/FooterView.java

+21 −7
Original line number Diff line number Diff line
@@ -102,6 +102,11 @@ public class FooterView extends StackScrollerDecorView { 
        setClearAllButtonVisible(visible, animate, /* onAnimationEnded = */ null);

    }



    /** Set the visibility of the "Manage"/"History" button to {@code visible}. */

    public void setManageOrHistoryButtonVisible(boolean visible) {

        mManageOrHistoryButton.setVisibility(visible ? View.VISIBLE : View.GONE);

    }



    /**

     * Set the visibility of the "Clear all" button to {@code visible}. Animate the change if

     * {@code animate} is true.
@@ -274,6 +279,14 @@ public class FooterView extends StackScrollerDecorView { 


    /** Show a message instead of the footer buttons. */

    public void setFooterLabelVisible(boolean isVisible) {

        // In the refactored code, hiding the buttons is handled in the FooterViewModel

        if (FooterViewRefactor.isEnabled()) {

            if (isVisible) {

                mSeenNotifsFooterTextView.setVisibility(View.VISIBLE);

            } else {

                mSeenNotifsFooterTextView.setVisibility(View.GONE);

            }

        } else {

            if (isVisible) {

                mManageOrHistoryButton.setVisibility(View.GONE);

                mClearAllButton.setVisibility(View.GONE);
@@ -284,6 +297,7 @@ public class FooterView extends StackScrollerDecorView { 
                mSeenNotifsFooterTextView.setVisibility(View.GONE);

            }

        }

    }



    /** Set onClickListener for the manage/history button. */

    public void setManageButtonClickListener(OnClickListener listener) {

packages/SystemUI/src/com/android/systemui/statusbar/notification/footer/ui/viewbinder/FooterViewBinder.kt

+6 −2
Original line number Diff line number Diff line
@@ -141,8 +141,12 @@ object FooterViewBinder { 
            }

        }



        // NOTE: The manage/history button is always visible as long as the footer is visible, no

        //  need to update the visibility here.

        launch {

            viewModel.manageOrHistoryButton.isVisible.collect { isVisible ->

                // NOTE: This visibility change is never animated.

                footer.setManageOrHistoryButtonVisible(isVisible.value)

            }

        }

    }



    private suspend fun bindMessage(

packages/SystemUI/src/com/android/systemui/statusbar/notification/footer/ui/viewmodel/FooterViewModel.kt

+31 −11
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ import java.util.Optional 
import javax.inject.Provider

import kotlinx.coroutines.flow.Flow

import kotlinx.coroutines.flow.combine

import kotlinx.coroutines.flow.distinctUntilChanged

import kotlinx.coroutines.flow.flowOf

import kotlinx.coroutines.flow.map

import kotlinx.coroutines.flow.onStart
@@ -45,12 +46,33 @@ class FooterViewModel( 
    seenNotificationsInteractor: SeenNotificationsInteractor,

    shadeInteractor: ShadeInteractor,

) {

    /** A message to show instead of the footer buttons. */

    val message: FooterMessageViewModel =

        FooterMessageViewModel(

            messageId = R.string.unlock_to_see_notif_text,

            iconId = R.drawable.ic_friction_lock_closed,

            isVisible = seenNotificationsInteractor.hasFilteredOutSeenNotifications,

        )



    private val clearAllButtonVisible =

        activeNotificationsInteractor.hasClearableNotifications

            .combine(message.isVisible) { hasClearableNotifications, isMessageVisible ->

                if (isMessageVisible) {

                    // If the message is visible, the button never is

                    false

                } else {

                    hasClearableNotifications

                }

            }

            .distinctUntilChanged()



    /** The button for clearing notifications. */

    val clearAllButton: FooterButtonViewModel =

        FooterButtonViewModel(

            labelId = flowOf(R.string.clear_all_notifications_text),

            accessibilityDescriptionId = flowOf(R.string.accessibility_clear_all),

            isVisible =

                activeNotificationsInteractor.hasClearableNotifications

                clearAllButtonVisible

                    .sample(

                        // TODO(b/322167853): This check is currently duplicated in

                        //  NotificationListViewModel, but instead it should be a field in
@@ -61,9 +83,9 @@ class FooterViewModel( 
                                ::Pair

                            )

                            .onStart { emit(Pair(false, false)) }

                    ) { hasClearableNotifications, (isShadeFullyExpanded, animationsEnabled) ->

                    ) { clearAllButtonVisible, (isShadeFullyExpanded, animationsEnabled) ->

                        val shouldAnimate = isShadeFullyExpanded && animationsEnabled

                        AnimatableEvent(hasClearableNotifications, shouldAnimate)

                        AnimatableEvent(clearAllButtonVisible, shouldAnimate)

                    }

                    .toAnimatedValueFlow(),

        )
@@ -77,18 +99,16 @@ class FooterViewModel( 
            else R.string.manage_notifications_text

        }



    /** The button for managing notification settings or opening notification history. */

    val manageOrHistoryButton: FooterButtonViewModel =

        FooterButtonViewModel(

            labelId = manageOrHistoryButtonText,

            accessibilityDescriptionId = manageOrHistoryButtonText,

            isVisible = flowOf(AnimatedValue.NotAnimating(true)),

        )



    val message: FooterMessageViewModel =

        FooterMessageViewModel(

            messageId = R.string.unlock_to_see_notif_text,

            iconId = R.drawable.ic_friction_lock_closed,

            isVisible = seenNotificationsInteractor.hasFilteredOutSeenNotifications,

            isVisible =

                // Hide the manage button if the message is visible

                message.isVisible.map { messageVisible ->

                    AnimatedValue.NotAnimating(!messageVisible)

                },

        )

}

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/footer/ui/view/FooterViewTest.java

+0 −4
Original line number Diff line number Diff line
@@ -299,8 +299,6 @@ public class FooterViewTest extends SysuiTestCase { 
    @Test

    public void testSetFooterLabelVisible() {

        mView.setFooterLabelVisible(true);

        assertThat(mView.findViewById(R.id.manage_text).getVisibility()).isEqualTo(View.GONE);

        assertThat(mView.findSecondaryView().getVisibility()).isEqualTo(View.GONE);

        assertThat(mView.findViewById(R.id.unlock_prompt_footer).getVisibility())

                .isEqualTo(View.VISIBLE);

    }
@@ -308,8 +306,6 @@ public class FooterViewTest extends SysuiTestCase { 
    @Test

    public void testSetFooterLabelInvisible() {

        mView.setFooterLabelVisible(false);

        assertThat(mView.findViewById(R.id.manage_text).getVisibility()).isEqualTo(View.VISIBLE);

        assertThat(mView.findSecondaryView().getVisibility()).isEqualTo(View.VISIBLE);

        assertThat(mView.findViewById(R.id.unlock_prompt_footer).getVisibility())

                .isEqualTo(View.GONE);

    }

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/footer/ui/viewmodel/FooterViewModelTest.kt

+47 −8
Original line number Diff line number Diff line
@@ -66,7 +66,7 @@ class FooterViewModelTest : SysuiTestCase() { 
    val underTest = kosmos.footerViewModel



    @Test

    fun testMessageVisible_whenFilteredNotifications() =

    fun messageVisible_whenFilteredNotifications() =

        testScope.runTest {

            val visible by collectLastValue(underTest.message.isVisible)
@@ -76,7 +76,7 @@ class FooterViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun testMessageVisible_whenNoFilteredNotifications() =

    fun messageVisible_whenNoFilteredNotifications() =

        testScope.runTest {

            val visible by collectLastValue(underTest.message.isVisible)
@@ -86,7 +86,7 @@ class FooterViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun testClearAllButtonVisible_whenHasClearableNotifs() =

    fun clearAllButtonVisible_whenHasClearableNotifs() =

        testScope.runTest {

            val visible by collectLastValue(underTest.clearAllButton.isVisible)
@@ -104,7 +104,7 @@ class FooterViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun testClearAllButtonVisible_whenHasNoClearableNotifs() =

    fun clearAllButtonVisible_whenHasNoClearableNotifs() =

        testScope.runTest {

            val visible by collectLastValue(underTest.clearAllButton.isVisible)
@@ -122,7 +122,26 @@ class FooterViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun testClearAllButtonAnimating_whenShadeExpandedAndTouchable() =

    fun clearAllButtonVisible_whenMessageVisible() =

        testScope.runTest {

            val visible by collectLastValue(underTest.clearAllButton.isVisible)



            activeNotificationListRepository.notifStats.value =

                NotifStats(

                    numActiveNotifs = 2,

                    hasNonClearableAlertingNotifs = false,

                    hasClearableAlertingNotifs = true,

                    hasNonClearableSilentNotifs = false,

                    hasClearableSilentNotifs = true,

                )

            activeNotificationListRepository.hasFilteredOutSeenNotifications.value = true

            runCurrent()



            assertThat(visible?.value).isFalse()

        }



    @Test

    fun clearAllButtonAnimating_whenShadeExpandedAndTouchable() =

        testScope.runTest {

            val visible by collectLastValue(underTest.clearAllButton.isVisible)

            runCurrent()
@@ -156,7 +175,7 @@ class FooterViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun testClearAllButtonAnimating_whenShadeNotExpanded() =

    fun clearAllButtonAnimating_whenShadeNotExpanded() =

        testScope.runTest {

            val visible by collectLastValue(underTest.clearAllButton.isVisible)

            runCurrent()
@@ -190,7 +209,7 @@ class FooterViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun testManageButton_whenHistoryDisabled() =

    fun manageButton_whenHistoryDisabled() =

        testScope.runTest {

            val buttonLabel by collectLastValue(underTest.manageOrHistoryButton.labelId)

            runCurrent()
@@ -203,7 +222,7 @@ class FooterViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun testHistoryButton_whenHistoryEnabled() =

    fun historyButton_whenHistoryEnabled() =

        testScope.runTest {

            val buttonLabel by collectLastValue(underTest.manageOrHistoryButton.labelId)

            runCurrent()
@@ -214,4 +233,24 @@ class FooterViewModelTest : SysuiTestCase() { 
            // THEN label is "History"

            assertThat(buttonLabel).isEqualTo(R.string.manage_notifications_history_text)

        }



    @Test

    fun manageButtonVisible_whenMessageVisible() =

        testScope.runTest {

            val visible by collectLastValue(underTest.manageOrHistoryButton.isVisible)



            activeNotificationListRepository.hasFilteredOutSeenNotifications.value = true



            assertThat(visible?.value).isFalse()

        }



    @Test

    fun manageButtonVisible_whenMessageNotVisible() =

        testScope.runTest {

            val visible by collectLastValue(underTest.manageOrHistoryButton.isVisible)



            activeNotificationListRepository.hasFilteredOutSeenNotifications.value = false



            assertThat(visible?.value).isTrue()

        }

}