Media layout changes for tablets

- Changes height on tablets to 251dp instead 184dp - Aligns media actions + seek bar to bottom - Aligns title/artist + play/pause button above media actions Fixes: 221823467 Test: Unit tests + Manually Change-Id: I8b850253bd1aa9ed0bc3648326582d2c986b42ca
2022-03-22 16:19:04 +00:00
parent fa5aefba20
commit 145bbfdfbd
7 changed files with 118 additions and 27 deletions
--- a/packages/SystemUI/res/layout/media_session_view.xml
+++ b/packages/SystemUI/res/layout/media_session_view.xml
@@ -172,6 +172,15 @@
        app:layout_constraintStart_toStartOf="parent"
        />

+    <androidx.constraintlayout.widget.Barrier
+        android:id="@+id/media_action_barrier_top"
+        android:layout_width="match_parent"
+        android:layout_height="0dp"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:barrierDirection="top"
+        app:constraint_referenced_ids="actionPrev,media_progress_bar,actionNext,action0,action1,action2,action3,action4"
+        />
+
    <!-- Button visibility will be controlled in code -->
    <ImageButton
        android:id="@+id/actionPrev"
--- a/packages/SystemUI/res/values-sw720dp-land/dimens.xml
+++ b/packages/SystemUI/res/values-sw720dp-land/dimens.xml
@@ -26,5 +26,5 @@
    <dimen name="split_shade_header_height">56dp</dimen>
    <dimen name="status_bar_header_height_keyguard">56dp</dimen>

-    <dimen name="qs_media_session_height_expanded">184dp</dimen>
+    <dimen name="qs_media_session_height_expanded">251dp</dimen>
 </resources>
--- a/packages/SystemUI/res/xml/media_session_expanded.xml
+++ b/packages/SystemUI/res/xml/media_session_expanded.xml
@@ -31,25 +31,24 @@
        android:id="@+id/header_title"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
-        android:layout_marginTop="20dp"
        android:layout_marginStart="@dimen/qs_media_padding"
        android:layout_marginEnd="@dimen/qs_media_padding"
        app:layout_constraintEnd_toStartOf="@id/actionPlayPause"
        app:layout_constrainedWidth="true"
-        app:layout_constraintTop_toBottomOf="@id/icon"
        app:layout_constraintStart_toStartOf="parent"
+        app:layout_constraintBottom_toTopOf="@id/header_artist"
        app:layout_constraintHorizontal_bias="0" />
    <Constraint
        android:id="@+id/header_artist"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_marginEnd="@dimen/qs_media_padding"
+        android:layout_marginBottom="@dimen/qs_media_padding"
        android:layout_marginTop="0dp"
        app:layout_constrainedWidth="true"
        app:layout_constraintEnd_toStartOf="@id/actionPlayPause"
-        app:layout_constraintBottom_toTopOf="@id/media_action_barrier"
-        app:layout_constraintTop_toBottomOf="@id/header_title"
        app:layout_constraintStart_toStartOf="@id/header_title"
+        app:layout_constraintBottom_toTopOf="@id/media_action_barrier_top"
        app:layout_constraintVertical_bias="0"
        app:layout_constraintHorizontal_bias="0" />

@@ -59,10 +58,9 @@
        android:layout_height="48dp"
        android:layout_marginStart="@dimen/qs_media_padding"
        android:layout_marginEnd="@dimen/qs_media_padding"
-        android:layout_marginBottom="0dp"
+        android:layout_marginBottom="@dimen/qs_media_padding"
        app:layout_constraintEnd_toEndOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/media_seamless"
-        app:layout_constraintBottom_toBottomOf="@id/header_artist" />
+        app:layout_constraintBottom_toTopOf="@id/media_action_barrier_top" />

    <!--
    The bottom row of action buttons should remain in the same order when RTL, so their constraints
@@ -76,7 +74,6 @@
        app:layout_constraintLeft_toLeftOf="parent"
        app:layout_constraintRight_toLeftOf="@id/media_progress_bar"
        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/header_artist"
        app:layout_constraintHorizontal_chainStyle="spread" />

    <Constraint
@@ -86,7 +83,6 @@
        app:layout_constraintLeft_toRightOf="@id/actionPrev"
        app:layout_constraintRight_toLeftOf="@id/actionNext"
        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/header_artist"
        app:layout_constraintHorizontal_weight="1" />

    <Constraint
@@ -95,8 +91,7 @@
        android:layout_height="48dp"
        app:layout_constraintLeft_toRightOf="@id/media_progress_bar"
        app:layout_constraintRight_toLeftOf="@id/action0"
-        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/header_artist" />
+        app:layout_constraintBottom_toBottomOf="parent" />

    <Constraint
        android:id="@+id/action0"
@@ -104,8 +99,7 @@
        android:layout_height="48dp"
        app:layout_constraintLeft_toRightOf="@id/actionNext"
        app:layout_constraintRight_toLeftOf="@id/action1"
-        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/header_artist" />
+        app:layout_constraintBottom_toBottomOf="parent" />

    <Constraint
        android:id="@+id/action1"
@@ -113,8 +107,7 @@
        android:layout_height="48dp"
        app:layout_constraintLeft_toRightOf="@id/action0"
        app:layout_constraintRight_toLeftOf="@id/action2"
-        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/header_artist" />
+        app:layout_constraintBottom_toBottomOf="parent" />

    <Constraint
        android:id="@+id/action2"
@@ -122,8 +115,7 @@
        android:layout_height="48dp"
        app:layout_constraintLeft_toRightOf="@id/action1"
        app:layout_constraintRight_toLeftOf="@id/action3"
-        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/header_artist" />
+        app:layout_constraintBottom_toBottomOf="parent"/>

    <Constraint
        android:id="@+id/action3"
@@ -131,8 +123,7 @@
        android:layout_height="48dp"
        app:layout_constraintLeft_toRightOf="@id/action2"
        app:layout_constraintRight_toLeftOf="@id/action4"
-        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/header_artist" />
+        app:layout_constraintBottom_toBottomOf="parent"/>

    <Constraint
        android:id="@+id/action4"
@@ -140,6 +131,5 @@
        android:layout_height="48dp"
        app:layout_constraintLeft_toRightOf="@id/action3"
        app:layout_constraintRight_toRightOf="parent"
-        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/header_artist" />
+        app:layout_constraintBottom_toBottomOf="parent" />
 </ConstraintSet>
--- a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
@@ -606,12 +606,29 @@ public class MediaControlPanel {
                setSemanticButton(genericButtons[i], null,  collapsedSet, expandedSet, false);
            }
        }
+        expandedSet.setVisibility(R.id.media_progress_bar, getSeekBarVisibility());
+        expandedSet.setAlpha(R.id.media_progress_bar, mSeekBarViewModel.getEnabled() ? 1.0f : 0.0f);
+    }

-        // If disabled, set progress bar to INVISIBLE instead of GONE so layout weights still work
+    private int getSeekBarVisibility() {
        boolean seekbarEnabled = mSeekBarViewModel.getEnabled();
-        expandedSet.setVisibility(R.id.media_progress_bar,
-                seekbarEnabled ? ConstraintSet.VISIBLE : ConstraintSet.INVISIBLE);
-        expandedSet.setAlpha(R.id.media_progress_bar, seekbarEnabled ? 1.0f : 0.0f);
+        if (seekbarEnabled) {
+            return ConstraintSet.VISIBLE;
+        }
+        // If disabled and "neighbours" are visible, set progress bar to INVISIBLE instead of GONE
+        // so layout weights still work.
+        return areAnyExpandedBottomActionsVisible() ? ConstraintSet.INVISIBLE : ConstraintSet.GONE;
+    }
+
+    private boolean areAnyExpandedBottomActionsVisible() {
+        ConstraintSet expandedSet = mMediaViewController.getExpandedLayout();
+        int[] referencedIds = mMediaViewHolder.getActionsTopBarrier().getReferencedIds();
+        for (int id : referencedIds) {
+            if (expandedSet.getVisibility(id) == ConstraintSet.VISIBLE) {
+                return true;
+            }
+        }
+        return false;
    }

    private void setSemanticButton(final ImageButton button, MediaAction mediaAction,
--- a/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt
@@ -24,6 +24,7 @@ import android.widget.ImageButton
 import android.widget.ImageView
 import android.widget.SeekBar
 import android.widget.TextView
+import androidx.constraintlayout.widget.Barrier
 import com.android.systemui.R
 import com.android.systemui.util.animation.TransitionLayout

@@ -70,6 +71,8 @@ class MediaViewHolder constructor(itemView: View) {
    val action3 = itemView.requireViewById<ImageButton>(R.id.action3)
    val action4 = itemView.requireViewById<ImageButton>(R.id.action4)

+    val actionsTopBarrier = itemView.requireViewById<Barrier>(R.id.media_action_barrier_top)
+
    init {
        (player.background as IlluminationDrawable).let {
            it.registerLightSource(seamless)
--- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt
@@ -16,6 +16,7 @@

 package com.android.systemui.media

+import org.mockito.Mockito.`when` as whenever
 import android.content.Intent
 import android.graphics.Color
 import android.graphics.drawable.GradientDrawable
@@ -35,6 +36,7 @@ import android.widget.ImageButton
 import android.widget.ImageView
 import android.widget.SeekBar
 import android.widget.TextView
+import androidx.constraintlayout.widget.Barrier
 import androidx.constraintlayout.widget.ConstraintSet
 import androidx.lifecycle.LiveData
 import androidx.test.filters.SmallTest
@@ -63,7 +65,6 @@ import org.mockito.Mockito.never
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
 import org.mockito.junit.MockitoJUnit
-import org.mockito.Mockito.`when` as whenever

 private const val KEY = "TEST_KEY"
 private const val APP = "APP"
@@ -120,6 +121,7 @@ public class MediaControlPanelTest : SysuiTestCase() {
    private lateinit var actionPlayPause: ImageButton
    private lateinit var actionNext: ImageButton
    private lateinit var actionPrev: ImageButton
+    private lateinit var actionsTopBarrier: Barrier
    @Mock private lateinit var longPressText: TextView
    @Mock private lateinit var handler: Handler
    private lateinit var settings: ImageButton
@@ -176,6 +178,21 @@ public class MediaControlPanelTest : SysuiTestCase() {
        actionPrev = ImageButton(context).also { it.setId(R.id.actionPrev) }
        actionNext = ImageButton(context).also { it.setId(R.id.actionNext) }

+        actionsTopBarrier =
+            Barrier(context).also {
+                it.id = R.id.media_action_barrier_top
+                it.referencedIds =
+                    intArrayOf(
+                        actionPrev.id,
+                        seekBar.id,
+                        actionNext.id,
+                        action0.id,
+                        action1.id,
+                        action2.id,
+                        action3.id,
+                        action4.id)
+            }
+
        initMediaViewHolderMocks()

        // Create media session
@@ -254,6 +271,8 @@ public class MediaControlPanelTest : SysuiTestCase() {
        whenever(viewHolder.cancelText).thenReturn(cancelText)
        whenever(viewHolder.dismiss).thenReturn(dismiss)
        whenever(viewHolder.dismissText).thenReturn(dismissText)
+
+        whenever(viewHolder.actionsTopBarrier).thenReturn(actionsTopBarrier)
    }

    @After
@@ -315,6 +334,35 @@ public class MediaControlPanelTest : SysuiTestCase() {
        verify(expandedSet).setVisibility(R.id.action4, ConstraintSet.GONE)
    }

+    @Test
+    fun bind_seekBarDisabled_seekBarVisibilityIsSetToInvisible() {
+        whenever(seekBarViewModel.getEnabled()).thenReturn(false)
+
+        val icon = Icon.createWithResource(context, android.R.drawable.ic_media_play)
+        val semanticActions = MediaButton(
+            playOrPause = MediaAction(icon, Runnable {}, "play"),
+            nextOrCustom = MediaAction(icon, Runnable {}, "next")
+        )
+        val state = mediaData.copy(semanticActions = semanticActions)
+
+        player.attachPlayer(viewHolder)
+        player.bindPlayer(state, PACKAGE)
+
+        verify(expandedSet).setVisibility(R.id.media_progress_bar, ConstraintSet.INVISIBLE)
+    }
+
+    @Test
+    fun bind_seekBarDisabled_noActions_seekBarVisibilityIsSetToGone() {
+        whenever(seekBarViewModel.getEnabled()).thenReturn(false)
+
+        val state = mediaData.copy(semanticActions = MediaButton())
+
+        player.attachPlayer(viewHolder)
+        player.bindPlayer(state, PACKAGE)
+
+        verify(expandedSet).setVisibility(R.id.media_progress_bar, ConstraintSet.INVISIBLE)
+    }
+
    @Test
    fun bindNotificationActions() {
        val icon = Icon.createWithResource(context, android.R.drawable.ic_media_play)
--- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaViewHolderTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaViewHolderTest.kt
@@ -0,0 +1,24 @@
+package com.android.systemui.media
+
+import android.testing.AndroidTestingRunner
+import android.testing.TestableLooper
+import android.view.LayoutInflater
+import android.widget.FrameLayout
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@SmallTest
+@RunWith(AndroidTestingRunner::class)
+@TestableLooper.RunWithLooper
+class MediaViewHolderTest : SysuiTestCase() {
+
+    @Test
+    fun create_succeeds() {
+        val inflater = LayoutInflater.from(context)
+        val parent = FrameLayout(context)
+
+        MediaViewHolder.create(inflater, parent)
+    }
+}