fix QS header for new ConstraintLayout version

With new ConstraintLayout versions layout_constrainedWidth was causing the view to assume match_parent width. As MotionScene supports importing external layouts, we're not loading them at runtime anymore. Also, the transition for large_screen_header_constraint is not used anymore, as initial and final constraint were the same. Bug: 259933125 Test: CombinedShadeHeaderConstraintsTest and manual Change-Id: I3961d9f4311c96b5d0663e1700fca78be5e6544b
2022-11-21 23:47:03 +00:00
parent 9bb27a054d
commit 124b2a3b56
6 changed files with 59 additions and 57 deletions
--- a/packages/SystemUI/res/xml/combined_qs_header_scene.xml
+++ b/packages/SystemUI/res/xml/combined_qs_header_scene.xml
@@ -124,20 +124,9 @@
        </KeyFrameSet>
    </Transition>

-    <Transition
-        android:id="@+id/large_screen_header_transition"
-        app:constraintSetStart="@id/large_screen_header_constraint"
-        app:constraintSetEnd="@id/large_screen_header_constraint"/>
+    <Include app:constraintSet="@xml/large_screen_shade_header"/>

-    <!--
-        Placeholder ConstraintSet. They are populated in the controller for this class.
-        This is needed because there's no easy way to just refer to a `ConstraintSet` file. The
-        options are either a layout file or inline the ConstraintSets.
-     -->
-    <ConstraintSet android:id="@id/qqs_header_constraint"/>
-
-    <ConstraintSet android:id="@id/qs_header_constraint"/>
-
-    <ConstraintSet android:id="@id/large_screen_header_constraint" />
+    <Include app:constraintSet="@xml/qs_header"/>

+    <Include app:constraintSet="@xml/qqs_header"/>
 </MotionScene>
--- a/packages/SystemUI/res/xml/qqs_header.xml
+++ b/packages/SystemUI/res/xml/qqs_header.xml
@@ -59,7 +59,6 @@
        <Layout
            android:layout_width="wrap_content"
            android:layout_height="@dimen/new_qs_header_non_clickable_element_height"
-            app:layout_constrainedWidth="true"
            app:layout_constraintHeight_min="@dimen/new_qs_header_non_clickable_element_height"
            app:layout_constraintStart_toEndOf="@id/date"
            app:layout_constraintEnd_toStartOf="@id/batteryRemainingIcon"
@@ -75,7 +74,6 @@
        <Layout
            android:layout_width="wrap_content"
            android:layout_height="@dimen/new_qs_header_non_clickable_element_height"
-            app:layout_constrainedWidth="true"
            app:layout_constraintHeight_min="@dimen/new_qs_header_non_clickable_element_height"
            app:layout_constraintStart_toEndOf="@id/statusIcons"
            app:layout_constraintEnd_toEndOf="@id/end_guide"
@@ -112,5 +110,4 @@
            app:layout_constraintHorizontal_bias="1"
        />
    </Constraint>
-
 </ConstraintSet>
--- a/packages/SystemUI/res/xml/qs_header.xml
+++ b/packages/SystemUI/res/xml/qs_header.xml
@@ -56,6 +56,7 @@
        <Layout
            android:layout_width="wrap_content"
            android:layout_height="@dimen/new_qs_header_non_clickable_element_height"
+            app:layout_constrainedWidth="true"
            app:layout_constraintStart_toStartOf="parent"
            app:layout_constraintEnd_toStartOf="@id/space"
            app:layout_constraintBottom_toBottomOf="parent"
@@ -88,7 +89,6 @@
        <Layout
            android:layout_width="wrap_content"
            android:layout_height="@dimen/new_qs_header_non_clickable_element_height"
-            app:layout_constrainedWidth="true"
            app:layout_constraintStart_toEndOf="@id/space"
            app:layout_constraintEnd_toStartOf="@id/batteryRemainingIcon"
            app:layout_constraintTop_toTopOf="@id/date"
--- a/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt
+++ b/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt
@@ -43,7 +43,6 @@ import com.android.systemui.qs.carrier.QSCarrierGroup
 import com.android.systemui.qs.carrier.QSCarrierGroupController
 import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.HEADER_TRANSITION_ID
 import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.LARGE_SCREEN_HEADER_CONSTRAINT
-import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.LARGE_SCREEN_HEADER_TRANSITION_ID
 import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.QQS_HEADER_CONSTRAINT
 import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.QS_HEADER_CONSTRAINT
 import com.android.systemui.statusbar.phone.StatusBarContentInsetsProvider
@@ -69,11 +68,9 @@ import javax.inject.Named
 * expansion of the headers in small screen portrait.
 *
 * [header] will be a [MotionLayout] if [Flags.COMBINED_QS_HEADERS] is enabled. In this case, the
- * [MotionLayout] has 2 transitions:
+ * [MotionLayout] has one transitions:
 * * [HEADER_TRANSITION_ID]: [QQS_HEADER_CONSTRAINT] <-> [QS_HEADER_CONSTRAINT] for portrait
 *   handheld device configuration.
- * * [LARGE_SCREEN_HEADER_TRANSITION_ID]: [LARGE_SCREEN_HEADER_CONSTRAINT] (to itself) for all
- *   other configurations
 */
@CentralSurfacesScope
 class LargeScreenShadeHeaderController @Inject constructor(
@@ -99,8 +96,6 @@ class LargeScreenShadeHeaderController @Inject constructor(
        @VisibleForTesting
        internal val HEADER_TRANSITION_ID = R.id.header_transition
        @VisibleForTesting
-        internal val LARGE_SCREEN_HEADER_TRANSITION_ID = R.id.large_screen_header_transition
-        @VisibleForTesting
        internal val QQS_HEADER_CONSTRAINT = R.id.qqs_header_constraint
        @VisibleForTesting
        internal val QS_HEADER_CONSTRAINT = R.id.qs_header_constraint
@@ -115,10 +110,6 @@ class LargeScreenShadeHeaderController @Inject constructor(
        }
    }

-    init {
-        loadConstraints()
-    }
-
    private val combinedHeaders = featureFlags.isEnabled(Flags.COMBINED_QS_HEADERS)

    private lateinit var iconManager: StatusBarIconController.TintedIconManager
@@ -330,11 +321,11 @@ class LargeScreenShadeHeaderController @Inject constructor(
        if (header is MotionLayout) {
            // Use resources.getXml instead of passing the resource id due to bug b/205018300
            header.getConstraintSet(QQS_HEADER_CONSTRAINT)
-                .load(context, resources.getXml(R.xml.qqs_header))
+                    .load(context, resources.getXml(R.xml.qqs_header))
            header.getConstraintSet(QS_HEADER_CONSTRAINT)
-                .load(context, resources.getXml(R.xml.qs_header))
+                    .load(context, resources.getXml(R.xml.qs_header))
            header.getConstraintSet(LARGE_SCREEN_HEADER_CONSTRAINT)
-                .load(context, resources.getXml(R.xml.large_screen_shade_header))
+                    .load(context, resources.getXml(R.xml.large_screen_shade_header))
        }
    }

@@ -423,7 +414,6 @@ class LargeScreenShadeHeaderController @Inject constructor(
        }
        header as MotionLayout
        if (largeScreenActive) {
-            header.setTransition(LARGE_SCREEN_HEADER_TRANSITION_ID)
            header.getConstraintSet(LARGE_SCREEN_HEADER_CONSTRAINT).applyTo(header)
        } else {
            header.setTransition(HEADER_TRANSITION_ID)
--- a/packages/SystemUI/tests/src/com/android/systemui/shade/CombinedShadeHeaderConstraintsTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shade/CombinedShadeHeaderConstraintsTest.kt
@@ -23,9 +23,11 @@ import androidx.constraintlayout.widget.ConstraintSet.START
 import androidx.test.filters.SmallTest
 import com.android.systemui.R
 import com.android.systemui.SysuiTestCase
+import com.google.common.truth.Expect
 import com.google.common.truth.Truth.assertThat
 import com.google.common.truth.Truth.assertWithMessage
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith

@@ -37,6 +39,9 @@ class CombinedShadeHeaderConstraintsTest : SysuiTestCase() {
    private lateinit var qsConstraint: ConstraintSet
    private lateinit var largeScreenConstraint: ConstraintSet

+    @get:Rule
+    val expect: Expect = Expect.create()
+
    @Before
    fun setUp() {
        qqsConstraint = ConstraintSet().apply {
@@ -343,6 +348,32 @@ class CombinedShadeHeaderConstraintsTest : SysuiTestCase() {
        }
    }

+    @Test
+    fun testCheckViewsDontChangeSizeBetweenAnimationConstraints() {
+        val views = mapOf(
+                R.id.clock to "clock",
+                R.id.date to "date",
+                R.id.statusIcons to "icons",
+                R.id.privacy_container to "privacy",
+                R.id.carrier_group to "carriers",
+                R.id.batteryRemainingIcon to "battery",
+        )
+        views.forEach { (id, name) ->
+            expect.withMessage("$name changes height")
+                    .that(qqsConstraint.getConstraint(id).layout.mHeight.fromConstraint())
+                    .isEqualTo(qsConstraint.getConstraint(id).layout.mHeight.fromConstraint())
+            expect.withMessage("$name changes width")
+                    .that(qqsConstraint.getConstraint(id).layout.mWidth.fromConstraint())
+                    .isEqualTo(qsConstraint.getConstraint(id).layout.mWidth.fromConstraint())
+        }
+    }
+
+    private fun Int.fromConstraint() = when (this) {
+        -1 -> "MATCH_PARENT"
+        -2 -> "WRAP_CONTENT"
+        else -> toString()
+    }
+
    @Test
    fun testEmptyCutoutDateIconsAreConstrainedWidth() {
        CombinedShadeHeadersConstraintManagerImpl.emptyCutoutConstraints()()
--- a/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerCombinedTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerCombinedTest.kt
@@ -44,7 +44,6 @@ import com.android.systemui.qs.carrier.QSCarrierGroup
 import com.android.systemui.qs.carrier.QSCarrierGroupController
 import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.HEADER_TRANSITION_ID
 import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.LARGE_SCREEN_HEADER_CONSTRAINT
-import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.LARGE_SCREEN_HEADER_TRANSITION_ID
 import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.QQS_HEADER_CONSTRAINT
 import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.QS_HEADER_CONSTRAINT
 import com.android.systemui.statusbar.phone.StatusBarContentInsetsProvider
@@ -73,6 +72,7 @@ import org.mockito.Mockito.anyInt
 import org.mockito.Mockito.clearInvocations
 import org.mockito.Mockito.inOrder
 import org.mockito.Mockito.never
+import org.mockito.Mockito.reset
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.`when` as whenever
 import org.mockito.junit.MockitoJUnit
@@ -203,20 +203,6 @@ class LargeScreenShadeHeaderControllerCombinedTest : SysuiTestCase() {
            context.getString(com.android.internal.R.string.status_bar_mobile))
    }

-    @Test
-    fun testCorrectConstraints() {
-        val captor = ArgumentCaptor.forClass(XmlResourceParser::class.java)
-
-        verify(qqsConstraints).load(eq(context), capture(captor))
-        assertThat(captor.value.getResId()).isEqualTo(R.xml.qqs_header)
-
-        verify(qsConstraints).load(eq(context), capture(captor))
-        assertThat(captor.value.getResId()).isEqualTo(R.xml.qs_header)
-
-        verify(largeScreenConstraints).load(eq(context), capture(captor))
-        assertThat(captor.value.getResId()).isEqualTo(R.xml.large_screen_shade_header)
-    }
-
    @Test
    fun testControllersCreatedAndInitialized() {
        verify(variableDateViewController).init()
@@ -278,16 +264,6 @@ class LargeScreenShadeHeaderControllerCombinedTest : SysuiTestCase() {
        assertThat(viewVisibility).isEqualTo(View.INVISIBLE)
    }

-    @Test
-    fun testLargeScreenActive_true() {
-        controller.largeScreenActive = false // Make sure there's a change
-        clearInvocations(view)
-
-        controller.largeScreenActive = true
-
-        verify(view).setTransition(LARGE_SCREEN_HEADER_TRANSITION_ID)
-    }
-
    @Test
    fun testLargeScreenActive_false() {
        controller.largeScreenActive = true // Make sure there's a change
@@ -673,6 +649,25 @@ class LargeScreenShadeHeaderControllerCombinedTest : SysuiTestCase() {
        verify(clock).pivotY = height.toFloat() / 2
    }

+    @Test
+    fun onDensityOrFontScaleChanged_reloadConstraints() {
+        // After density or font scale change, constraints need to be reloaded to reflect new
+        // dimensions.
+        reset(qqsConstraints)
+        reset(qsConstraints)
+        reset(largeScreenConstraints)
+
+        configurationController.notifyDensityOrFontScaleChanged()
+
+        val captor = ArgumentCaptor.forClass(XmlResourceParser::class.java)
+        verify(qqsConstraints).load(eq(context), capture(captor))
+        assertThat(captor.value.getResId()).isEqualTo(R.xml.qqs_header)
+        verify(qsConstraints).load(eq(context), capture(captor))
+        assertThat(captor.value.getResId()).isEqualTo(R.xml.qs_header)
+        verify(largeScreenConstraints).load(eq(context), capture(captor))
+        assertThat(captor.value.getResId()).isEqualTo(R.xml.large_screen_shade_header)
+    }
+
    private fun View.executeLayoutChange(
            left: Int,
            top: Int,