From 145bbfdfbd94d28edc6bceb8e4efc54cbd730f6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6llner?= <chrisgollner@google.com>
Date: Tue, 22 Mar 2022 16:19:04 +0000
Subject: [PATCH] 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
---
 .../res/layout/media_session_view.xml         |  9 ++++
 .../res/values-sw720dp-land/dimens.xml        |  2 +-
 .../res/xml/media_session_expanded.xml        | 32 ++++--------
 .../systemui/media/MediaControlPanel.java     | 25 ++++++++--
 .../android/systemui/media/MediaViewHolder.kt |  3 ++
 .../systemui/media/MediaControlPanelTest.kt   | 50 ++++++++++++++++++-
 .../systemui/media/MediaViewHolderTest.kt     | 24 +++++++++
 7 files changed, 118 insertions(+), 27 deletions(-)
 create mode 100644 packages/SystemUI/tests/src/com/android/systemui/media/MediaViewHolderTest.kt

diff --git a/packages/SystemUI/res/layout/media_session_view.xml b/packages/SystemUI/res/layout/media_session_view.xml
index 6cf32151d8eab..111735623bc2f 100644
--- 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"
diff --git a/packages/SystemUI/res/values-sw720dp-land/dimens.xml b/packages/SystemUI/res/values-sw720dp-land/dimens.xml
index bdd7049365945..0ca59b13d2063 100644
--- 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>
diff --git a/packages/SystemUI/res/xml/media_session_expanded.xml b/packages/SystemUI/res/xml/media_session_expanded.xml
index 10da70447ec00..bec4e7ad540ea 100644
--- 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>
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
index 958c2441942c2..e1602dbb27a00 100644
--- 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,
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt b/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt
index 836b5cb3e2b14..e9c88861eeae3 100644
--- 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)
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt
index 1c7e3a4e90cfc..d0f2816f3bcd6 100644
--- 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)
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaViewHolderTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaViewHolderTest.kt
new file mode 100644
index 0000000000000..ee327937e0917
--- /dev/null
+++ 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)
+    }
+}