From 3cae3974c5bd527ff82aefd505fde6412d60b0b8 Mon Sep 17 00:00:00 2001 From: Roy Chou Date: Fri, 14 Jul 2023 04:10:06 +0000 Subject: [PATCH] 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)