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

Commit fcf5804e authored by Alex Shabalin's avatar Alex Shabalin
Browse files

Prevent making unnecessary requestDeviceSuggestion() calls. 

This CL adds additional checks inside the scroll handler and triggers
panel's `onPanelFullyVisible` event when the panel becomes fully
visible and was not visible before.

To sum it up, make a `requestDeviceSuggestion()` call only when:
- The Media Controls carousel becomes visible.
- The card within the carousel becomes fully visible.

Fix: 434295012
Test: atest MediaCarouselControllerTest MediaCarouselScrollHandlerTest
Test: On a physical device
Flag: com.android.systemui.enable_suggested_device_ui
Change-Id: I49408eee27e0314b8b0260c935ee7444902b8c25
parent 109ce60c

packages/SystemUI/multivalentTests/src/com/android/systemui/media/controls/ui/view/MediaCarouselScrollHandlerTest.kt

+17 −4
Original line number Diff line number Diff line
@@ -67,7 +67,7 @@ class MediaCarouselScrollHandlerTest : SysuiTestCase() { 
    @Mock lateinit var seekBarUpdateListener: (visibleToUser: Boolean) -> Unit

    @Mock lateinit var closeGuts: (immediate: Boolean) -> Unit

    @Mock lateinit var falsingManager: FalsingManager

    @Mock lateinit var onCarouselVisibleToUser: () -> Unit

    @Mock lateinit var onVisibleCardChanged: () -> Unit

    @Mock lateinit var logger: MediaUiEventLogger

    @Mock lateinit var contentContainer: ViewGroup

    @Mock lateinit var settingsButton: View
@@ -97,7 +97,7 @@ class MediaCarouselScrollHandlerTest : SysuiTestCase() { 
                seekBarUpdateListener,

                closeGuts,

                falsingManager,

                onCarouselVisibleToUser,

                onVisibleCardChanged,

                logger,

            )

        mediaCarouselScrollHandler.playerWidthPlusPadding = carouselWidth
@@ -257,7 +257,7 @@ class MediaCarouselScrollHandlerTest : SysuiTestCase() { 
    }



    @Test

    fun testCarouselScrollToNewIndex_onCarouselVisibleToUser() {

    fun testCarouselScrollToNewIndex_exactScroll_onVisibleCardChanged() {

        setupMediaContainer(visibleIndex = 0)

        whenever(mediaCarousel.relativeScrollX).thenReturn(carouselWidth)

        mediaCarouselScrollHandler.visibleToUser = true
@@ -266,7 +266,20 @@ class MediaCarouselScrollHandlerTest : SysuiTestCase() { 


        captor.value.onScrollChange(null, 0, 0, 0, 0)



        verify(onCarouselVisibleToUser).invoke()

        verify(onVisibleCardChanged).invoke()

    }



    @Test

    fun testCarouselScrollToNewIndex_partialScroll_noCallbackInvoked() {

        setupMediaContainer(visibleIndex = 0)

        whenever(mediaCarousel.relativeScrollX).thenReturn(carouselWidth + 15)

        mediaCarouselScrollHandler.visibleToUser = true

        val captor = ArgumentCaptor.forClass(View.OnScrollChangeListener::class.java)

        verify(mediaCarousel).setOnScrollChangeListener(captor.capture())



        captor.value.onScrollChange(null, 0, 0, 0, 0)



        verify(onVisibleCardChanged, never()).invoke()

    }



    private fun setupMediaContainer(visibleIndex: Int, showsSettingsButton: Boolean = true) {

packages/SystemUI/src/com/android/systemui/media/controls/ui/controller/MediaCarouselController.kt

+38 −4
Original line number Diff line number Diff line
@@ -60,6 +60,7 @@ import com.android.systemui.keyguard.shared.model.TransitionState 
import com.android.systemui.lifecycle.repeatWhenAttached

import com.android.systemui.media.controls.domain.pipeline.MediaDataManager

import com.android.systemui.media.controls.shared.model.MediaData

import com.android.systemui.media.controls.ui.controller.MediaPlayerData.visiblePlayerKeys

import com.android.systemui.media.controls.ui.view.MediaCarouselScrollHandler

import com.android.systemui.media.controls.ui.view.MediaHostState

import com.android.systemui.media.controls.ui.view.MediaScrollView
@@ -155,6 +156,9 @@ constructor( 
    /** Are we currently disabling scrolling, only allowing the first media session to show */

    private var currentlyDisableScrolling: Boolean = false



    /** A key for the last player card that is completely visible */

    private var lastFullyVisiblePlayerKey: String? = null



    /**

     * The desired location where we'll be at the end of the transformation. Usually this matches

     * the end location, except when we're still waiting on a state update call.
@@ -336,7 +340,7 @@ constructor( 
                this::updateSeekbarListening,

                this::closeGuts,

                falsingManager,

                this::onCarouselVisibleToUser,

                this::onVisibleCardChanged,

                logger,

            )

        carouselLocale = context.resources.configuration.locales.get(0)
@@ -1052,18 +1056,48 @@ constructor( 
        layout(0, 0, width, height)

    }



    /** Triggered whenever carousel becomes visible, e.g. on swipe down the notification shade. */

    fun onCarouselVisibleToUser() {

        if (!enableSuggestedDeviceUi()) {

            return

        }

        onCardVisibilityChanged()

    }



    /** Triggered whenever carousel's scroll position changes, revealing a new card.  */

    fun onVisibleCardChanged() {

        if (!enableSuggestedDeviceUi()) {

            return

        }

        val newVisiblePlayerKey =

            visiblePlayerKeys().elementAtOrNull(mediaCarouselScrollHandler.visibleMediaIndex)?.key

        if (newVisiblePlayerKey != lastFullyVisiblePlayerKey) {

            lastFullyVisiblePlayerKey = newVisiblePlayerKey

            if (newVisiblePlayerKey != null) {

                onCardVisibilityChanged()

            }

        }

    }



    /**

     * Triggered whenever card becomes visible either due to the carousel being visible or the card

     * visibility changed within the carousel.

     */

    private fun onCardVisibilityChanged() {

        val isCarouselVisible = mediaCarouselScrollHandler.visibleToUser

        val visibleMediaIndex = mediaCarouselScrollHandler.visibleMediaIndex

        debugLogger.logCardVisibilityChanged(isCarouselVisible, visibleMediaIndex)



        if (

            !enableSuggestedDeviceUi() ||

                !mediaCarouselScrollHandler.visibleToUser ||

                !isCarouselVisible ||

                MediaPlayerData.mediaData().all { it.second.resumption }

        ) {

            return

        }

        val visibleMediaIndex = mediaCarouselScrollHandler.visibleMediaIndex

        if (MediaPlayerData.players().size > visibleMediaIndex) {

            val mediaControlPanel = MediaPlayerData.getMediaControlPanel(visibleMediaIndex)

            mediaControlPanel?.onSuggestionSpaceVisible()

            mediaControlPanel?.onPanelFullyVisible()

        }

    }

packages/SystemUI/src/com/android/systemui/media/controls/ui/controller/MediaCarouselControllerLogger.kt

+12 −0
Original line number Diff line number Diff line
@@ -109,6 +109,18 @@ constructor(@MediaCarouselControllerLog private val buffer: LogBuffer) { 
            { "media carousel($str1), width: $int1 height: $int2, location:$long1" },

        )

    }



    fun logCardVisibilityChanged(carouselVisible: Boolean, visibleMediaIndex: Int) {

        buffer.log(

            TAG,

            LogLevel.DEBUG,

            {

                bool1 = carouselVisible

                int1 = visibleMediaIndex

            },

            { "card visibility changed, isVisible: $bool1, index: $int1" },

        )

    }

}



private const val TAG = "MediaCarouselCtlrLog"

packages/SystemUI/src/com/android/systemui/media/controls/ui/controller/MediaControlPanel.java

+2 −2
Original line number Diff line number Diff line
@@ -606,9 +606,9 @@ public class MediaControlPanel { 
    }



    /**

     * Should be called when the space that holds device suggestions becomes visible to the user.

     * Called when the panel becomes fully visible.

     */

    public void onSuggestionSpaceVisible() {

    public void onPanelFullyVisible() {

        if (!Flags.enableSuggestedDeviceUi()) {

            return;

        }

packages/SystemUI/src/com/android/systemui/media/controls/ui/view/MediaCarouselScrollHandler.kt

+5 −2
Original line number Diff line number Diff line
@@ -64,7 +64,7 @@ class MediaCarouselScrollHandler( 
    private var seekBarUpdateListener: (visibleToUser: Boolean) -> Unit,

    private val closeGuts: (immediate: Boolean) -> Unit,

    private val falsingManager: FalsingManager,

    private val onCarouselVisibleToUser: () -> Unit,

    private val onVisibleCardChanged: () -> Unit,

    private val logger: MediaUiEventLogger,

) {

    /** Trace state logger for media carousel visibility */
@@ -548,7 +548,10 @@ class MediaCarouselScrollHandler( 
            val visible = (i == visibleMediaIndex) || ((i == (visibleMediaIndex + 1)) && scrolledIn)

            view.visibility = if (visible) View.VISIBLE else View.INVISIBLE

        }

        onCarouselVisibleToUser()

        if (!scrolledIn) {

            // Ignore events with a partial scroll, only proceed if the card is fully visible.

            onVisibleCardChanged()

        }

    }



    /**