From 3cae3974c5bd527ff82aefd505fde6412d60b0b8 Mon Sep 17 00:00:00 2001 From: Roy Chou Date: Fri, 14 Jul 2023 04:10:06 +0000 Subject: [PATCH 1/5] fix(non linear font scaling): date textview is cut off in qs header Originally the qs header height is fixed to 120dp, so when larger font scaling the header height might not be enough to accommodate the subviews with given constraints, then some of the constraints are broken and the date textview bottom is cut off. Therefore, in NotificationsQSContainerController, we calculate the needed height for the subviews as the header height to prevent the ui broken. Bug: 291168649 Test: manually - attached screenshot in bug atest com.android.systemui.shade Change-Id: I99373152104ba1b378115fa12fea3ba3d7448e14 --- .../NotificationsQSContainerController.kt | 23 +++++++++-- ...icationsQSContainerControllerLegacyTest.kt | 41 ++++++++++++++++--- .../NotificationsQSContainerControllerTest.kt | 41 ++++++++++++++++--- 3 files changed, 89 insertions(+), 16 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/shade/NotificationsQSContainerController.kt b/packages/SystemUI/src/com/android/systemui/shade/NotificationsQSContainerController.kt index 5c1dd5670d8af..9412542239657 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/NotificationsQSContainerController.kt +++ b/packages/SystemUI/src/com/android/systemui/shade/NotificationsQSContainerController.kt @@ -155,10 +155,8 @@ class NotificationsQSContainerController @Inject constructor( largeScreenShadeHeaderActive = LargeScreenUtils.shouldUseLargeScreenShadeHeader(resources) notificationsBottomMargin = resources.getDimensionPixelSize( R.dimen.notification_panel_margin_bottom) - largeScreenShadeHeaderHeight = - resources.getDimensionPixelSize(R.dimen.large_screen_shade_header_height) - shadeHeaderHeight = - resources.getDimensionPixelSize(R.dimen.qs_header_height) + largeScreenShadeHeaderHeight = calculateLargeShadeHeaderHeight() + shadeHeaderHeight = calculateShadeHeaderHeight() panelMarginHorizontal = resources.getDimensionPixelSize( R.dimen.notification_panel_margin_horizontal) topMargin = if (largeScreenShadeHeaderActive) { @@ -182,6 +180,23 @@ class NotificationsQSContainerController @Inject constructor( } } + private fun calculateLargeShadeHeaderHeight(): Int { + return resources.getDimensionPixelSize(R.dimen.large_screen_shade_header_height) + } + + private fun calculateShadeHeaderHeight(): Int { + val minHeight = resources.getDimensionPixelSize(R.dimen.qs_header_height) + + // Following the constraints in xml/qs_header, the total needed height would be the sum of + // 1. privacy_container height (R.dimen.large_screen_shade_header_min_height) + // 2. carrier_group height (R.dimen.large_screen_shade_header_min_height) + // 3. date height (R.dimen.new_qs_header_non_clickable_element_height) + val estimatedHeight = + 2 * resources.getDimensionPixelSize(R.dimen.large_screen_shade_header_min_height) + + resources.getDimensionPixelSize(R.dimen.new_qs_header_non_clickable_element_height) + return estimatedHeight.coerceAtLeast(minHeight) + } + override fun setCustomizerAnimating(animating: Boolean) { if (isQSCustomizerAnimating != animating) { isQSCustomizerAnimating = animating diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationsQSContainerControllerLegacyTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationsQSContainerControllerLegacyTest.kt index 2bc112d68ae2a..112a09bcfe620 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationsQSContainerControllerLegacyTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationsQSContainerControllerLegacyTest.kt @@ -144,27 +144,52 @@ class NotificationsQSContainerControllerLegacyTest : SysuiTestCase() { @Test fun testSmallScreen_updateResources_splitShadeHeightIsSet() { overrideResource(R.bool.config_use_large_screen_shade_header, false) - overrideResource(R.dimen.qs_header_height, 1) - overrideResource(R.dimen.large_screen_shade_header_height, 2) + overrideResource(R.dimen.qs_header_height, 10) + overrideResource(R.dimen.large_screen_shade_header_height, 20) + + // ensure the estimated height (would be 3 here) wouldn't impact this test case + overrideResource(R.dimen.large_screen_shade_header_min_height, 1) + overrideResource(R.dimen.new_qs_header_non_clickable_element_height, 1) underTest.updateResources() val captor = ArgumentCaptor.forClass(ConstraintSet::class.java) verify(view).applyConstraints(capture(captor)) - assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(1) + assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(10) } @Test fun testLargeScreen_updateResources_splitShadeHeightIsSet() { overrideResource(R.bool.config_use_large_screen_shade_header, true) - overrideResource(R.dimen.qs_header_height, 1) - overrideResource(R.dimen.large_screen_shade_header_height, 2) + overrideResource(R.dimen.qs_header_height, 10) + overrideResource(R.dimen.large_screen_shade_header_height, 20) + + // ensure the estimated height (would be 3 here) wouldn't impact this test case + overrideResource(R.dimen.large_screen_shade_header_min_height, 1) + overrideResource(R.dimen.new_qs_header_non_clickable_element_height, 1) underTest.updateResources() val captor = ArgumentCaptor.forClass(ConstraintSet::class.java) verify(view).applyConstraints(capture(captor)) - assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(2) + assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(20) + } + + @Test + fun testSmallScreen_estimatedHeightIsLargerThanDimenValue_shadeHeightIsSetToEstimatedHeight() { + overrideResource(R.bool.config_use_large_screen_shade_header, false) + overrideResource(R.dimen.qs_header_height, 10) + overrideResource(R.dimen.large_screen_shade_header_height, 20) + + // make the estimated height (would be 15 here) larger than qs_header_height + overrideResource(R.dimen.large_screen_shade_header_min_height, 5) + overrideResource(R.dimen.new_qs_header_non_clickable_element_height, 5) + + underTest.updateResources() + + val captor = ArgumentCaptor.forClass(ConstraintSet::class.java) + verify(view).applyConstraints(capture(captor)) + assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(15) } @Test @@ -388,6 +413,10 @@ class NotificationsQSContainerControllerLegacyTest : SysuiTestCase() { val largeScreenHeaderHeight = 100 overrideResource(R.dimen.large_screen_shade_header_height, largeScreenHeaderHeight) + // ensure the estimated height (would be 30 here) wouldn't impact this test case + overrideResource(R.dimen.large_screen_shade_header_min_height, 10) + overrideResource(R.dimen.new_qs_header_non_clickable_element_height, 10) + underTest.updateResources() assertThat(getConstraintSetLayout(R.id.qs_frame).topMargin) diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationsQSContainerControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationsQSContainerControllerTest.kt index a5048187b1b42..8d3c4b21aa26e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationsQSContainerControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationsQSContainerControllerTest.kt @@ -143,27 +143,52 @@ class NotificationsQSContainerControllerTest : SysuiTestCase() { @Test fun testSmallScreen_updateResources_splitShadeHeightIsSet() { overrideResource(R.bool.config_use_large_screen_shade_header, false) - overrideResource(R.dimen.qs_header_height, 1) - overrideResource(R.dimen.large_screen_shade_header_height, 2) + overrideResource(R.dimen.qs_header_height, 10) + overrideResource(R.dimen.large_screen_shade_header_height, 20) + + // ensure the estimated height (would be 3 here) wouldn't impact this test case + overrideResource(R.dimen.large_screen_shade_header_min_height, 1) + overrideResource(R.dimen.new_qs_header_non_clickable_element_height, 1) underTest.updateResources() val captor = ArgumentCaptor.forClass(ConstraintSet::class.java) verify(view).applyConstraints(capture(captor)) - assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(1) + assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(10) } @Test fun testLargeScreen_updateResources_splitShadeHeightIsSet() { overrideResource(R.bool.config_use_large_screen_shade_header, true) - overrideResource(R.dimen.qs_header_height, 1) - overrideResource(R.dimen.large_screen_shade_header_height, 2) + overrideResource(R.dimen.qs_header_height, 10) + overrideResource(R.dimen.large_screen_shade_header_height, 20) + + // ensure the estimated height (would be 3 here) wouldn't impact this test case + overrideResource(R.dimen.large_screen_shade_header_min_height, 1) + overrideResource(R.dimen.new_qs_header_non_clickable_element_height, 1) underTest.updateResources() val captor = ArgumentCaptor.forClass(ConstraintSet::class.java) verify(view).applyConstraints(capture(captor)) - assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(2) + assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(20) + } + + @Test + fun testSmallScreen_estimatedHeightIsLargerThanDimenValue_shadeHeightIsSetToEstimatedHeight() { + overrideResource(R.bool.config_use_large_screen_shade_header, false) + overrideResource(R.dimen.qs_header_height, 10) + overrideResource(R.dimen.large_screen_shade_header_height, 20) + + // make the estimated height (would be 15 here) larger than qs_header_height + overrideResource(R.dimen.large_screen_shade_header_min_height, 5) + overrideResource(R.dimen.new_qs_header_non_clickable_element_height, 5) + + underTest.updateResources() + + val captor = ArgumentCaptor.forClass(ConstraintSet::class.java) + verify(view).applyConstraints(capture(captor)) + assertThat(captor.value.getHeight(R.id.split_shade_status_bar)).isEqualTo(15) } @Test @@ -376,6 +401,10 @@ class NotificationsQSContainerControllerTest : SysuiTestCase() { val largeScreenHeaderHeight = 100 overrideResource(R.dimen.large_screen_shade_header_height, largeScreenHeaderHeight) + // ensure the estimated height (would be 30 here) wouldn't impact this test case + overrideResource(R.dimen.large_screen_shade_header_min_height, 10) + overrideResource(R.dimen.new_qs_header_non_clickable_element_height, 10) + underTest.updateResources() assertThat(getConstraintSetLayout(R.id.qs_frame).topMargin) From 9d75ae662a0f9ab9bd3aa675cefcf70bbbc5da17 Mon Sep 17 00:00:00 2001 From: Roy Chou Date: Fri, 14 Jul 2023 04:31:31 +0000 Subject: [PATCH 2/5] fix(non linear font scaling): date textview might be covered by system icons in qs header As b/291168601, when the sum of date textview width and system icons container width is larger than the qs header width, date textview would be covered by system icons. Therefore, in xml/qs_header we add the constraint between the date textview and the system icons to prevent the ui broken. Bug: 291168601 Test: manually - attached screenshots in bug atest CombinedShadeHeaderConstraintsTest Change-Id: Ifd13da3bd3e7893bc79b0114fce972b039261364 --- packages/SystemUI/res/xml/qs_header.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/SystemUI/res/xml/qs_header.xml b/packages/SystemUI/res/xml/qs_header.xml index 7b4282f049b83..427fd87c640ad 100644 --- a/packages/SystemUI/res/xml/qs_header.xml +++ b/packages/SystemUI/res/xml/qs_header.xml @@ -83,6 +83,7 @@ android:layout_width="0dp" android:layout_height="@dimen/new_qs_header_non_clickable_element_height" app:layout_constraintWidth_default="wrap" + app:layout_constraintStart_toEndOf="@id/date" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintTop_toTopOf="@id/date" app:layout_constraintBottom_toBottomOf="@id/date" From 4f9682e60cdffc289b55c62ffa6f6bfff6ac93b4 Mon Sep 17 00:00:00 2001 From: Roy Chou Date: Fri, 14 Jul 2023 05:56:44 +0000 Subject: [PATCH 3/5] fix(non linear font scaling): clock and battery textview might be cut off in status bar when large font scaling Originally system icons in status bar have fixed top and bottom 8dp paddings, when larger font scaling the space might not enough for enlarged textview then the percentage textview is cut off, as in b/291168760. Therefore, as a quick solution we let the status_bar system_icons layout height be a sp dimen value like combined_qs_header and set the top/bottom paddings in system_icons to 0dp. It's to ensure the system_icons layout height is larger than the status icons height. In the future cl, we would also ensure the status bar height should be enough for accommodating the system_icons. Bug: 291168760 Test: manually - attached screenshot in bug Change-Id: I070bf6d5faa858acfa4927a9590c4575dc91f2fe --- packages/SystemUI/res/layout/status_bar.xml | 9 +++++++-- packages/SystemUI/res/values/dimens.xml | 7 +++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/SystemUI/res/layout/status_bar.xml b/packages/SystemUI/res/layout/status_bar.xml index 0ab921f4d555f..5132e57e2786f 100644 --- a/packages/SystemUI/res/layout/status_bar.xml +++ b/packages/SystemUI/res/layout/status_bar.xml @@ -67,6 +67,7 @@ android:id="@+id/status_bar_start_side_content" android:layout_width="wrap_content" android:layout_height="wrap_content" + android:layout_gravity="center_vertical|start" android:clipChildren="false"> @@ -88,7 +89,8 @@ - + diff --git a/packages/SystemUI/res/values/dimens.xml b/packages/SystemUI/res/values/dimens.xml index de8287e5e8cbf..3d4e8cd567f54 100644 --- a/packages/SystemUI/res/values/dimens.xml +++ b/packages/SystemUI/res/values/dimens.xml @@ -121,6 +121,9 @@ 0dp 6dp + + @dimen/status_bar_icon_size_sp + @*android:dimen/status_bar_icon_size_sp @@ -343,8 +346,8 @@ 11dp 0dp - 8dp - 8dp + 0dp + 0dp 0sp From 9d1274bda504a92d6b583f9ac9572075bd767bfa Mon Sep 17 00:00:00 2001 From: Roy Chou Date: Fri, 14 Jul 2023 06:07:28 +0000 Subject: [PATCH 4/5] fix(non linear font scaling): battery icon in qqs status bar might be cut off As the screenshot in b/291168770, when qqs status bar contains too much system icons and the container can not display all icons, the battery icon view might be cut off or even disappear. Since originally the battery icon view has higher priority and should stick to the right side of the container, we change the system icons container layout to ensure the battery icon view is placed properly, then using the remain space for the other system icons displaying. Bug: 291168770 Test: manually - attached screenshot in bug Change-Id: Id36f6cb0aa336dbbb4ca45c24879be3e4bc1e876 --- packages/SystemUI/res/layout/combined_qs_header.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/res/layout/combined_qs_header.xml b/packages/SystemUI/res/layout/combined_qs_header.xml index f3a6bbeaaf0e0..12f13e9a21385 100644 --- a/packages/SystemUI/res/layout/combined_qs_header.xml +++ b/packages/SystemUI/res/layout/combined_qs_header.xml @@ -133,7 +133,8 @@ frame when animating QS <-> QQS transition From d21590e080fcd553f2be91295f3506f77e2760e5 Mon Sep 17 00:00:00 2001 From: Roy Chou Date: Fri, 21 Jul 2023 07:25:54 +0000 Subject: [PATCH 5/5] fix(non linear font scaling): status bar mobile/wifi icons size not correct The mobile/wifi signal icon sizes are mis-set to 13sp, originally they should match the status_bar_system_icon_size. Therefore, we correct the dimen value so that when default font scaling the icon sizes whould be same as the original size. Besides, The mobile type/roaming icon sizes are mis-set to same as mobile signal icon size, while originally they're WRAP_CONTENT so should be same as the drawable size. Therefore, We adjust the type/roaming icon sizes to match the corresponding drawable icon size. It's to ensure then the icons is still scalable and the sizes are same as original when default font scaling. Bug: 291899494 Bug: 293196179 Test: manually - attach video/screenshots Change-Id: Ia601968173c71687e515b8d81f2e7980e48568ed --- .../status_bar_mobile_signal_group_inner.xml | 10 ++++++---- packages/SystemUI/res/values/dimens.xml | 16 ++++++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/SystemUI/res-keyguard/layout/status_bar_mobile_signal_group_inner.xml b/packages/SystemUI/res-keyguard/layout/status_bar_mobile_signal_group_inner.xml index 8f1323db299d2..a1e2dc36278b5 100644 --- a/packages/SystemUI/res-keyguard/layout/status_bar_mobile_signal_group_inner.xml +++ b/packages/SystemUI/res-keyguard/layout/status_bar_mobile_signal_group_inner.xml @@ -54,7 +54,7 @@ 17sp - - 13sp + + 15sp 17sp - - 13sp + + 15sp + + 16sp + + 8sp 4sp