From 54ab1dfb36832319e84aa24a4dd1adacfe7c5814 Mon Sep 17 00:00:00 2001 From: Anton Potapov Date: Fri, 13 Jan 2023 20:15:25 +0000 Subject: [PATCH] Fix large screen QS and QQS lookup. Specially when privacy chip is shown. There were several problems that were leading to the wrong behavior: 1) We didn't use special constraint set for large devices 2) Thats why the layout itself was a bit outdated and unoptimal 3) We didn't immediately change state after the transition is setup up. This led to carrier text missing 4) We set the transition after the listeners. This didn't cause any problems but it's an error prone approach when listeners are immediately notified with the current state Test: manual + auto Fixes: 260134176 Change-Id: Id7d641d04a8086651af5e82a6ec3b6eaa92a0b5c --- .../res/layout/ongoing_privacy_chip.xml | 1 + .../res/values-sw600dp-land/dimens.xml | 1 + .../res/xml/large_screen_shade_header.xml | 95 ++++++------------- ...mbinedShadeHeadersConstraintManagerImpl.kt | 3 +- .../shade/LargeScreenShadeHeaderController.kt | 17 ++-- .../CombinedShadeHeaderConstraintsTest.kt | 21 ++-- 6 files changed, 58 insertions(+), 80 deletions(-) diff --git a/packages/SystemUI/res/layout/ongoing_privacy_chip.xml b/packages/SystemUI/res/layout/ongoing_privacy_chip.xml index 5aa608084510c..d1a2cf4c24b21 100644 --- a/packages/SystemUI/res/layout/ongoing_privacy_chip.xml +++ b/packages/SystemUI/res/layout/ongoing_privacy_chip.xml @@ -25,6 +25,7 @@ android:focusable="true" android:clipChildren="false" android:clipToPadding="false" + android:paddingStart="8dp" > 16dp + 24dp 24dp 24dp - - + - + app:layout_constraintStart_toEndOf="@id/begin_guide" + app:layout_constraintTop_toTopOf="parent" /> + - + + app:layout_constraintTop_toTopOf="parent" /> + - + - + app:layout_constraintEnd_toStartOf="@id/statusIcons" + app:layout_constraintStart_toEndOf="@id/date" + app:layout_constraintTop_toTopOf="parent" + app:layout_constraintWidth_default="wrap" + app:layout_constraintWidth_min="48dp" /> + - + - + app:layout_constraintEnd_toStartOf="@id/batteryRemainingIcon" + app:layout_constraintTop_toTopOf="parent" + app:layout_constraintEnd_toEndOf="@id/carrier_group"/> + - + - + app:layout_constraintEnd_toStartOf="@id/privacy_container" + app:layout_constraintTop_toTopOf="parent" /> + - + + app:layout_constraintBottom_toBottomOf="parent" + app:layout_constraintEnd_toStartOf="@id/end_guide" + app:layout_constraintTop_toTopOf="parent" /> - \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/shade/CombinedShadeHeadersConstraintManagerImpl.kt b/packages/SystemUI/src/com/android/systemui/shade/CombinedShadeHeadersConstraintManagerImpl.kt index 5011227ad2ccc..b3d31f2986d17 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/CombinedShadeHeadersConstraintManagerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/shade/CombinedShadeHeadersConstraintManagerImpl.kt @@ -69,7 +69,8 @@ object CombinedShadeHeadersConstraintManagerImpl : CombinedShadeHeadersConstrain } return ConstraintsChanges( qqsConstraintsChanges = change, - qsConstraintsChanges = change + qsConstraintsChanges = change, + largeScreenConstraintsChanges = change, ) } diff --git a/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt b/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt index e406be1ea0a3d..88676371bd6b4 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt +++ b/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt @@ -113,7 +113,7 @@ class LargeScreenShadeHeaderController @Inject constructor( QQS_HEADER_CONSTRAINT -> "QQS Header" QS_HEADER_CONSTRAINT -> "QS Header" LARGE_SCREEN_HEADER_CONSTRAINT -> "Large Screen Header" - else -> "Unknown state" + else -> "Unknown state $this" } } @@ -296,6 +296,9 @@ class LargeScreenShadeHeaderController @Inject constructor( override fun onViewAttached() { privacyIconsController.chipVisibilityListener = chipVisibilityListener + updateVisibility() + updateTransition() + if (header is MotionLayout) { header.setOnApplyWindowInsetsListener(insetListener) clock.addOnLayoutChangeListener { v, _, _, _, _, _, _, _, _ -> @@ -308,9 +311,6 @@ class LargeScreenShadeHeaderController @Inject constructor( dumpManager.registerDumpable(this) configurationController.addCallback(configurationControllerListener) demoModeController.addCallback(demoModeReceiver) - - updateVisibility() - updateTransition() } override fun onViewDetached() { @@ -436,15 +436,14 @@ class LargeScreenShadeHeaderController @Inject constructor( header as MotionLayout if (largeScreenActive) { logInstantEvent("Large screen constraints set") - header.setTransition(HEADER_TRANSITION_ID) - header.transitionToStart() + header.setTransition(LARGE_SCREEN_HEADER_TRANSITION_ID) } else { logInstantEvent("Small screen constraints set") header.setTransition(HEADER_TRANSITION_ID) - header.transitionToStart() - updatePosition() - updateScrollY() } + header.jumpToState(header.startState) + updatePosition() + updateScrollY() } private fun updatePosition() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/CombinedShadeHeaderConstraintsTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/CombinedShadeHeaderConstraintsTest.kt index f802a5e09228f..ed9baf5b1c9f5 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/CombinedShadeHeaderConstraintsTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/CombinedShadeHeaderConstraintsTest.kt @@ -109,11 +109,12 @@ class CombinedShadeHeaderConstraintsTest : SysuiTestCase() { @Test fun testEdgeElementsAlignedWithEdge_largeScreen() { with(largeScreenConstraint) { - assertThat(getConstraint(R.id.clock).layout.startToStart).isEqualTo(PARENT_ID) - assertThat(getConstraint(R.id.clock).layout.horizontalBias).isEqualTo(0f) + assertThat(getConstraint(R.id.clock).layout.startToEnd).isEqualTo(R.id.begin_guide) + assertThat(getConstraint(R.id.clock).layout.horizontalBias).isEqualTo(0.5f) - assertThat(getConstraint(R.id.privacy_container).layout.endToEnd).isEqualTo(PARENT_ID) - assertThat(getConstraint(R.id.privacy_container).layout.horizontalBias).isEqualTo(1f) + assertThat(getConstraint(R.id.privacy_container).layout.endToStart) + .isEqualTo(R.id.end_guide) + assertThat(getConstraint(R.id.privacy_container).layout.horizontalBias).isEqualTo(0.5f) } } @@ -219,7 +220,12 @@ class CombinedShadeHeaderConstraintsTest : SysuiTestCase() { .isEqualTo(cutoutEnd - padding) } - assertThat(changes.largeScreenConstraintsChanges).isNull() + with(largeScreenConstraint) { + assertThat(getConstraint(R.id.begin_guide).layout.guideBegin) + .isEqualTo(cutoutStart - padding) + assertThat(getConstraint(R.id.end_guide).layout.guideEnd) + .isEqualTo(cutoutEnd - padding) + } } @Test @@ -246,7 +252,10 @@ class CombinedShadeHeaderConstraintsTest : SysuiTestCase() { assertThat(getConstraint(R.id.end_guide).layout.guideEnd).isEqualTo(0) } - assertThat(changes.largeScreenConstraintsChanges).isNull() + with(largeScreenConstraint) { + assertThat(getConstraint(R.id.begin_guide).layout.guideBegin).isEqualTo(0) + assertThat(getConstraint(R.id.end_guide).layout.guideEnd).isEqualTo(0) + } } @Test