From e4d7aaa6fd5027772046552b21beeafdcc1bc5ed Mon Sep 17 00:00:00 2001 From: Jordan Silva Date: Thu, 21 Sep 2023 11:33:47 +0100 Subject: [PATCH] Fix to prevent unsorted breakpoints returning wrong spec It orders the breakpoint list to guarantee that the smallest breakpoint is always returned for a given availableWidth or availableHeight. Fix: 301396419 Flag: ENABLE_RESPONSIVE_WORKSPACE Test: CalculatedWorkspaceSpecTest Change-Id: Ia545e84150027bd8daa8d0dde58ff6fc579c3b6a --- .../launcher3/responsive/HotseatSpecs.kt | 10 +++- .../launcher3/responsive/ResponsiveSpecs.kt | 8 ++- .../res/xml/valid_workspace_unsorted_file.xml | 58 +++++++++++++++++++ .../responsive/CalculatedWorkspaceSpecTest.kt | 39 +++++++++++++ 4 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 tests/res/xml/valid_workspace_unsorted_file.xml diff --git a/src/com/android/launcher3/responsive/HotseatSpecs.kt b/src/com/android/launcher3/responsive/HotseatSpecs.kt index d578b0874d..37a682f130 100644 --- a/src/com/android/launcher3/responsive/HotseatSpecs.kt +++ b/src/com/android/launcher3/responsive/HotseatSpecs.kt @@ -21,7 +21,15 @@ import android.util.Log import com.android.launcher3.R import com.android.launcher3.util.ResourceHelper -class HotseatSpecs(val widthSpecs: List, val heightSpecs: List) { +class HotseatSpecs(widthSpecs: List, heightSpecs: List) { + + val widthSpecs: List + val heightSpecs: List + + init { + this.widthSpecs = widthSpecs.sortedBy { it.maxAvailableSize } + this.heightSpecs = heightSpecs.sortedBy { it.maxAvailableSize } + } fun getCalculatedHeightSpec(availableHeight: Int): CalculatedHotseatSpec { val spec = heightSpecs.firstOrNull { availableHeight <= it.maxAvailableSize } diff --git a/src/com/android/launcher3/responsive/ResponsiveSpecs.kt b/src/com/android/launcher3/responsive/ResponsiveSpecs.kt index 72a0ea4f72..a43c44aa81 100644 --- a/src/com/android/launcher3/responsive/ResponsiveSpecs.kt +++ b/src/com/android/launcher3/responsive/ResponsiveSpecs.kt @@ -24,10 +24,9 @@ import android.util.Log * @param widthSpecs List of width responsive specifications * @param heightSpecs List of height responsive specifications */ -abstract class ResponsiveSpecs( - val widthSpecs: List, +abstract class ResponsiveSpecs(widthSpecs: List, heightSpecs: List) { + val widthSpecs: List val heightSpecs: List -) { init { check(widthSpecs.isNotEmpty() && heightSpecs.isNotEmpty()) { @@ -35,6 +34,9 @@ abstract class ResponsiveSpecs( "width list size = ${widthSpecs.size}; " + "height list size = ${heightSpecs.size}." } + + this.widthSpecs = widthSpecs.sortedBy { it.maxAvailableSize } + this.heightSpecs = heightSpecs.sortedBy { it.maxAvailableSize } } /** diff --git a/tests/res/xml/valid_workspace_unsorted_file.xml b/tests/res/xml/valid_workspace_unsorted_file.xml new file mode 100644 index 0000000000..1216c81a41 --- /dev/null +++ b/tests/res/xml/valid_workspace_unsorted_file.xml @@ -0,0 +1,58 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/src/com/android/launcher3/responsive/CalculatedWorkspaceSpecTest.kt b/tests/src/com/android/launcher3/responsive/CalculatedWorkspaceSpecTest.kt index 0af694e000..8f56c5f160 100644 --- a/tests/src/com/android/launcher3/responsive/CalculatedWorkspaceSpecTest.kt +++ b/tests/src/com/android/launcher3/responsive/CalculatedWorkspaceSpecTest.kt @@ -104,4 +104,43 @@ class CalculatedWorkspaceSpecTest : AbstractDeviceProfileTest() { assertThat(heightSpec.gutterPx).isEqualTo(54) assertThat(heightSpec.cellSizePx).isEqualTo(260) } + + /** + * This test tests: + * - (height spec) gets the correct breakpoint from the XML - use the first breakpoint + * - (height spec) do the correct calculations for remainder space and fixed size + * - (width spec) do the correct calculations for remainder space and fixed size + */ + @Test + fun smallPhone_returnsFirstBreakpointSpec_unsortedFile() { + val deviceSpec = deviceSpecs["phone"]!! + deviceSpec.densityDpi = 540 // larger display size + initializeVarsForPhone(deviceSpec) + + val availableWidth = deviceSpec.naturalSize.first + // Hotseat size is roughly 640px on a real device, + // it doesn't need to be precise on unit tests + val availableHeight = deviceSpec.naturalSize.second - deviceSpec.statusBarNaturalPx - 640 + + val workspaceSpecs = + WorkspaceSpecs.create( + TestResourceHelper(context!!, TestR.xml.valid_workspace_unsorted_file) + ) + val widthSpec = workspaceSpecs.getCalculatedWidthSpec(4, availableWidth) + val heightSpec = workspaceSpecs.getCalculatedHeightSpec(5, availableHeight) + + assertThat(widthSpec.availableSpace).isEqualTo(availableWidth) + assertThat(widthSpec.cells).isEqualTo(4) + assertThat(widthSpec.startPaddingPx).isEqualTo(74) + assertThat(widthSpec.endPaddingPx).isEqualTo(74) + assertThat(widthSpec.gutterPx).isEqualTo(54) + assertThat(widthSpec.cellSizePx).isEqualTo(193) + + assertThat(heightSpec.availableSpace).isEqualTo(availableHeight) + assertThat(heightSpec.cells).isEqualTo(5) + assertThat(heightSpec.startPaddingPx).isEqualTo(0) + assertThat(heightSpec.endPaddingPx).isEqualTo(108) + assertThat(heightSpec.gutterPx).isEqualTo(54) + assertThat(heightSpec.cellSizePx).isEqualTo(260) + } }