From 460561d92e02f90110f15a8c23eebf211ad8e0cf Mon Sep 17 00:00:00 2001 From: Michael Mikhail Date: Wed, 31 Aug 2022 14:27:44 +0000 Subject: [PATCH] Reorder media player cards Checking that the updated media player has data that isPlaying control set to true and that of the current visible media player is set to false before reordering. Also, checking added media player is active. So we make sure we scroll to the current playing media player card. Media players will need to be reordered later, if these conditions don't meet. Bug: 234639220 Test: atest MediaCarouselControllerTest Change-Id: I242c7d800a11d0525f9b4c525fe4885cbffd60d1 --- .../systemui/media/MediaCarouselController.kt | 24 ++++++- .../media/MediaCarouselControllerTest.kt | 71 ++++++++++++++++++- 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt index 4c6aa7b4cad9b..cc77ed15b5f0f 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt @@ -458,6 +458,7 @@ class MediaCarouselController @Inject constructor( val existingPlayer = MediaPlayerData.getMediaPlayer(key) val curVisibleMediaKey = MediaPlayerData.playerKeys() .elementAtOrNull(mediaCarouselScrollHandler.visibleMediaIndex) + val isCurVisibleMediaPlaying = MediaPlayerData.getMediaData(curVisibleMediaKey)?.isPlaying if (existingPlayer == null) { val newPlayer = mediaControlPanelFactory.get() newPlayer.attachPlayer(MediaViewHolder.create( @@ -472,13 +473,23 @@ class MediaCarouselController @Inject constructor( key, data, newPlayer, systemClock, isSsReactivated, debugLogger ) updatePlayerToState(newPlayer, noAnimation = true) - reorderAllPlayers(curVisibleMediaKey) + if (data.active) { + reorderAllPlayers(curVisibleMediaKey) + } else { + needsReordering = true + } } else { existingPlayer.bindPlayer(data, key) MediaPlayerData.addMediaPlayer( key, data, existingPlayer, systemClock, isSsReactivated, debugLogger ) - if (isReorderingAllowed || shouldScrollToActivePlayer) { + // Check the playing status of both current visible and new media players + // To make sure we scroll to the active playing media card. + if (isReorderingAllowed || + shouldScrollToActivePlayer && + data.isPlaying == true && + isCurVisibleMediaPlaying == false + ) { reorderAllPlayers(curVisibleMediaKey) } else { needsReordering = true @@ -1035,6 +1046,15 @@ internal object MediaPlayerData { } } + fun getMediaData(mediaSortKey: MediaSortKey?): MediaData? { + mediaData.forEach { (key, value) -> + if (value == mediaSortKey) { + return mediaData[key]?.data + } + } + return null + } + fun getMediaPlayer(key: String): MediaControlPanel? { return mediaData.get(key)?.let { mediaPlayers.get(it) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt index 1bc197232da6a..5a50a9f644876 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt @@ -30,6 +30,7 @@ import com.android.systemui.statusbar.notification.collection.provider.VisualSta import com.android.systemui.statusbar.policy.ConfigurationController import com.android.systemui.util.animation.TransitionLayout import com.android.systemui.util.concurrency.DelayableExecutor +import com.android.systemui.util.mockito.capture import com.android.systemui.util.mockito.eq import com.android.systemui.util.time.FakeSystemClock import javax.inject.Provider @@ -38,6 +39,8 @@ import junit.framework.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor +import org.mockito.Captor import org.mockito.Mock import org.mockito.Mockito.verify import org.mockito.Mockito.verifyNoMoreInteractions @@ -71,6 +74,10 @@ class MediaCarouselControllerTest : SysuiTestCase() { @Mock lateinit var player: TransitionLayout @Mock lateinit var recommendationViewHolder: RecommendationViewHolder @Mock lateinit var recommendations: TransitionLayout + @Mock lateinit var mediaPlayer: MediaControlPanel + @Mock lateinit var mediaViewController: MediaViewController + @Mock lateinit var smartspaceMediaData: SmartspaceMediaData + @Captor lateinit var listener: ArgumentCaptor private val clock = FakeSystemClock() private lateinit var mediaCarouselController: MediaCarouselController @@ -94,7 +101,10 @@ class MediaCarouselControllerTest : SysuiTestCase() { logger, debugLogger ) - + verify(mediaDataManager).addListener(capture(listener)) + whenever(mediaControlPanelFactory.get()).thenReturn(mediaPlayer) + whenever(mediaPlayer.mediaViewController).thenReturn(mediaViewController) + whenever(mediaDataManager.smartspaceMediaData).thenReturn(smartspaceMediaData) MediaPlayerData.clear() } @@ -305,4 +315,63 @@ class MediaCarouselControllerTest : SysuiTestCase() { verifyNoMoreInteractions(mediaViewHolder) verify(recommendationViewHolder.recommendations).bottom = 75 } + + fun testMediaLoaded_ScrollToActivePlayer() { + listener.value.onMediaDataLoaded("playing local", + null, + DATA.copy(active = true, isPlaying = true, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false) + ) + listener.value.onMediaDataLoaded("paused local", + null, + DATA.copy(active = true, isPlaying = false, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false)) + // adding a media recommendation card. + MediaPlayerData.addMediaRecommendation(SMARTSPACE_KEY, EMPTY_SMARTSPACE_MEDIA_DATA, panel, + false, clock) + mediaCarouselController.shouldScrollToActivePlayer = true + // switching between media players. + listener.value.onMediaDataLoaded("playing local", + "playing local", + DATA.copy(active = true, isPlaying = false, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = true) + ) + listener.value.onMediaDataLoaded("paused local", + "paused local", + DATA.copy(active = true, isPlaying = true, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false)) + + assertEquals( + MediaPlayerData.getMediaPlayerIndex("paused local"), + mediaCarouselController.mediaCarouselScrollHandler.visibleMediaIndex + ) + } + + @Test + fun testMediaLoadedFromRecommendationCard_ScrollToActivePlayer() { + MediaPlayerData.addMediaRecommendation(SMARTSPACE_KEY, EMPTY_SMARTSPACE_MEDIA_DATA, panel, + false, clock) + listener.value.onMediaDataLoaded("playing local", + null, + DATA.copy(active = true, isPlaying = true, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false) + ) + + var playerIndex = MediaPlayerData.getMediaPlayerIndex("playing local") + assertEquals( + playerIndex, + mediaCarouselController.mediaCarouselScrollHandler.visibleMediaIndex + ) + assertEquals( playerIndex, 0) + + // Replaying the same media player one more time. + // And check that the card stays in its position. + listener.value.onMediaDataLoaded("playing local", + null, + DATA.copy(active = true, isPlaying = true, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false) + ) + playerIndex = MediaPlayerData.getMediaPlayerIndex("playing local") + assertEquals(playerIndex, 0) + } }