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

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

Make FooterViewModel children hold flows.

Instead of the parent view holding a flow of the child view, it makes
more sense for the child view to hold a flow of the visibility instead.
That way we're not triggering updates unnecessarily.

The footer view already knows to ignore duplicate updates for fields,
but if we can avoid triggering a downstream listener that's a small win
for performance. Plus it avoids holding constant values in a flow
unnecessarily.

Bug: 293167744
Test: manual + FooterViewModelTest
Flag: ACONFIG com.systemui.notifications_footer_refactor_flag DEVELOPMENT

Change-Id: Ic512b2fbdeb1d99c4aeeab1500221ae17d3e1087
parent 7f3c67b1
Loading
Loading
Loading
Loading
+17 −13
Original line number Diff line number Diff line
@@ -34,34 +34,38 @@ object FooterViewBinder {
        viewModel: FooterViewModel,
        clearAllNotifications: View.OnClickListener,
    ): DisposableHandle {
        // Listen for changes when the view is attached.
        // Bind the resource IDs
        footer.setMessageString(viewModel.message.messageId)
        footer.setMessageIcon(viewModel.message.iconId)
        footer.setClearAllButtonText(viewModel.clearAllButton.labelId)
        footer.setClearAllButtonDescription(viewModel.clearAllButton.accessibilityDescriptionId)

        // Bind the click listeners
        footer.setClearAllButtonClickListener(clearAllNotifications)

        // Listen for visibility changes when the view is attached.
        return footer.repeatWhenAttached {
            lifecycleScope.launch {
                viewModel.clearAllButton.collect { button ->
                    if (button.isVisible.isAnimating) {
                viewModel.clearAllButton.isVisible.collect { isVisible ->
                    if (isVisible.isAnimating) {
                        footer.setClearAllButtonVisible(
                            button.isVisible.value,
                            isVisible.value,
                            /* animate = */ true,
                        ) { _ ->
                            button.isVisible.stopAnimating()
                            isVisible.stopAnimating()
                        }
                    } else {
                        footer.setClearAllButtonVisible(
                            button.isVisible.value,
                            isVisible.value,
                            /* animate = */ false,
                        )
                    }
                    footer.setClearAllButtonText(button.labelId)
                    footer.setClearAllButtonDescription(button.accessibilityDescriptionId)
                    footer.setClearAllButtonClickListener(clearAllNotifications)
                }
            }

            lifecycleScope.launch {
                viewModel.message.collect { message ->
                    footer.setFooterLabelVisible(message.visible)
                    footer.setMessageString(message.messageId)
                    footer.setMessageIcon(message.iconId)
                viewModel.message.isVisible.collect { visible ->
                    footer.setFooterLabelVisible(visible)
                }
            }
        }
+2 −1
Original line number Diff line number Diff line
@@ -18,9 +18,10 @@ package com.android.systemui.statusbar.notification.footer.ui.viewmodel

import android.annotation.StringRes
import com.android.systemui.util.ui.AnimatedValue
import kotlinx.coroutines.flow.Flow

data class FooterButtonViewModel(
    @StringRes val labelId: Int,
    @StringRes val accessibilityDescriptionId: Int,
    val isVisible: AnimatedValue<Boolean>,
    val isVisible: Flow<AnimatedValue<Boolean>>,
)
+2 −1
Original line number Diff line number Diff line
@@ -18,10 +18,11 @@ package com.android.systemui.statusbar.notification.footer.ui.viewmodel

import android.annotation.DrawableRes
import android.annotation.StringRes
import kotlinx.coroutines.flow.StateFlow

/** A ViewModel for the string message that can be shown in the footer. */
data class FooterMessageViewModel(
    @StringRes val messageId: Int,
    @DrawableRes val iconId: Int,
    val visible: Boolean,
    val isVisible: StateFlow<Boolean>,
)
+25 −31
Original line number Diff line number Diff line
@@ -30,9 +30,7 @@ import dagger.Module
import dagger.Provides
import java.util.Optional
import javax.inject.Provider
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onStart

/** ViewModel for [FooterView]. */
@@ -41,7 +39,11 @@ class FooterViewModel(
    seenNotificationsInteractor: SeenNotificationsInteractor,
    shadeInteractor: ShadeInteractor,
) {
    val clearAllButton: Flow<FooterButtonViewModel> =
    val clearAllButton: FooterButtonViewModel =
        FooterButtonViewModel(
            labelId = R.string.clear_all_notifications_text,
            accessibilityDescriptionId = R.string.accessibility_clear_all,
            isVisible =
                activeNotificationsInteractor.hasClearableNotifications
                    .sample(
                        combine(
@@ -54,24 +56,16 @@ class FooterViewModel(
                        val shouldAnimate = isShadeFullyExpanded && animationsEnabled
                        AnimatableEvent(hasClearableNotifications, shouldAnimate)
                    }
            .toAnimatedValueFlow()
            .map { visible ->
                FooterButtonViewModel(
                    labelId = R.string.clear_all_notifications_text,
                    accessibilityDescriptionId = R.string.accessibility_clear_all,
                    isVisible = visible,
                    .toAnimatedValueFlow(),
        )
            }

    val message: Flow<FooterMessageViewModel> =
        seenNotificationsInteractor.hasFilteredOutSeenNotifications.map { hasFilteredOutNotifs ->
    val message: FooterMessageViewModel =
        FooterMessageViewModel(
            messageId = R.string.unlock_to_see_notif_text,
            iconId = R.drawable.ic_friction_lock_closed,
                visible = hasFilteredOutNotifs,
            isVisible = seenNotificationsInteractor.hasFilteredOutSeenNotifications,
        )
}
}

@Module
object FooterViewModelModule {
+12 −12
Original line number Diff line number Diff line
@@ -116,27 +116,27 @@ class FooterViewModelTest : SysuiTestCase() {
    @Test
    fun testMessageVisible_whenFilteredNotifications() =
        testComponent.runTest {
            val message by collectLastValue(footerViewModel.message)
            val visible by collectLastValue(footerViewModel.message.isVisible)

            activeNotificationListRepository.hasFilteredOutSeenNotifications.value = true

            assertThat(message?.visible).isTrue()
            assertThat(visible).isTrue()
        }

    @Test
    fun testMessageVisible_whenNoFilteredNotifications() =
        testComponent.runTest {
            val message by collectLastValue(footerViewModel.message)
            val visible by collectLastValue(footerViewModel.message.isVisible)

            activeNotificationListRepository.hasFilteredOutSeenNotifications.value = false

            assertThat(message?.visible).isFalse()
            assertThat(visible).isFalse()
        }

    @Test
    fun testClearAllButtonVisible_whenHasClearableNotifs() =
        testComponent.runTest {
            val button by collectLastValue(footerViewModel.clearAllButton)
            val visible by collectLastValue(footerViewModel.clearAllButton.isVisible)

            activeNotificationListRepository.notifStats.value =
                NotifStats(
@@ -148,13 +148,13 @@ class FooterViewModelTest : SysuiTestCase() {
                )
            runCurrent()

            assertThat(button?.isVisible?.value).isTrue()
            assertThat(visible?.value).isTrue()
        }

    @Test
    fun testClearAllButtonVisible_whenHasNoClearableNotifs() =
        testComponent.runTest {
            val button by collectLastValue(footerViewModel.clearAllButton)
            val visible by collectLastValue(footerViewModel.clearAllButton.isVisible)

            activeNotificationListRepository.notifStats.value =
                NotifStats(
@@ -166,13 +166,13 @@ class FooterViewModelTest : SysuiTestCase() {
                )
            runCurrent()

            assertThat(button?.isVisible?.value).isFalse()
            assertThat(visible?.value).isFalse()
        }

    @Test
    fun testClearAllButtonAnimating_whenShadeExpandedAndTouchable() =
        testComponent.runTest {
            val button by collectLastValue(footerViewModel.clearAllButton)
            val visible by collectLastValue(footerViewModel.clearAllButton.isVisible)
            runCurrent()

            // WHEN shade is expanded
@@ -200,13 +200,13 @@ class FooterViewModelTest : SysuiTestCase() {
            runCurrent()

            // THEN button visibility should animate
            assertThat(button?.isVisible?.isAnimating).isTrue()
            assertThat(visible?.isAnimating).isTrue()
        }

    @Test
    fun testClearAllButtonAnimating_whenShadeNotExpanded() =
        testComponent.runTest {
            val button by collectLastValue(footerViewModel.clearAllButton)
            val visible by collectLastValue(footerViewModel.clearAllButton.isVisible)
            runCurrent()

            // WHEN shade is collapsed
@@ -234,6 +234,6 @@ class FooterViewModelTest : SysuiTestCase() {
            runCurrent()

            // THEN button visibility should not animate
            assertThat(button?.isVisible?.isAnimating).isFalse()
            assertThat(visible?.isAnimating).isFalse()
        }
}