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
diff --git a/src/com/android/launcher3/responsive/HotseatSpecs.kt b/src/com/android/launcher3/responsive/HotseatSpecs.kt
index d578b08..37a682f 100644
--- a/src/com/android/launcher3/responsive/HotseatSpecs.kt
+++ b/src/com/android/launcher3/responsive/HotseatSpecs.kt
@@ -21,7 +21,15 @@
import com.android.launcher3.R
import com.android.launcher3.util.ResourceHelper
-class HotseatSpecs(val widthSpecs: List<HotseatSpec>, val heightSpecs: List<HotseatSpec>) {
+class HotseatSpecs(widthSpecs: List<HotseatSpec>, heightSpecs: List<HotseatSpec>) {
+
+ val widthSpecs: List<HotseatSpec>
+ val heightSpecs: List<HotseatSpec>
+
+ 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 72a0ea4..a43c44a 100644
--- a/src/com/android/launcher3/responsive/ResponsiveSpecs.kt
+++ b/src/com/android/launcher3/responsive/ResponsiveSpecs.kt
@@ -24,10 +24,9 @@
* @param widthSpecs List of width responsive specifications
* @param heightSpecs List of height responsive specifications
*/
-abstract class ResponsiveSpecs<T : ResponsiveSpec>(
- val widthSpecs: List<T>,
+abstract class ResponsiveSpecs<T : ResponsiveSpec>(widthSpecs: List<T>, heightSpecs: List<T>) {
+ val widthSpecs: List<T>
val heightSpecs: List<T>
-) {
init {
check(widthSpecs.isNotEmpty() && heightSpecs.isNotEmpty()) {
@@ -35,6 +34,9 @@
"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 0000000..1216c81
--- /dev/null
+++ b/tests/res/xml/valid_workspace_unsorted_file.xml
@@ -0,0 +1,58 @@
+<?xml version="1.0" encoding="utf-8"?><!--
+ ~ Copyright (C) 2023 The Android Open Source Project
+ ~
+ ~ Licensed under the Apache License, Version 2.0 (the "License");
+ ~ you may not use this file except in compliance with the License.
+ ~ You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+
+<workspaceSpecs xmlns:launcher="http://schemas.android.com/apk/res-auto">
+
+ <workspaceSpec
+ launcher:specType="height"
+ launcher:maxAvailableSize="9999dp">
+ <startPadding launcher:fixedSize="8dp" />
+ <endPadding launcher:ofRemainderSpace="1" />
+ <gutter launcher:fixedSize="16dp" />
+ <cellSize launcher:fixedSize="104dp" />
+ </workspaceSpec>
+
+ <!-- 584 grid height -->
+ <workspaceSpec
+ launcher:specType="height"
+ launcher:maxAvailableSize="584dp">
+ <startPadding launcher:fixedSize="0dp" />
+ <endPadding launcher:fixedSize="32dp" />
+ <gutter launcher:fixedSize="16dp" />
+ <cellSize launcher:ofAvailableSpace="0.15808" />
+ </workspaceSpec>
+
+ <!-- 584 grid height + 28 remainder space -->
+ <workspaceSpec
+ launcher:specType="height"
+ launcher:maxAvailableSize="612dp">
+ <startPadding launcher:fixedSize="0dp" />
+ <endPadding launcher:ofRemainderSpace="1" />
+ <gutter launcher:fixedSize="16dp" />
+ <cellSize launcher:fixedSize="104dp" />
+ </workspaceSpec>
+
+ <!-- Width spec is always the same -->
+ <workspaceSpec
+ launcher:specType="width"
+ launcher:maxAvailableSize="9999dp">
+ <startPadding launcher:fixedSize="22dp" />
+ <endPadding launcher:fixedSize="22dp" />
+ <gutter launcher:fixedSize="16dp" />
+ <cellSize launcher:ofRemainderSpace="0.25" />
+ </workspaceSpec>
+
+</workspaceSpecs>
diff --git a/tests/src/com/android/launcher3/responsive/CalculatedWorkspaceSpecTest.kt b/tests/src/com/android/launcher3/responsive/CalculatedWorkspaceSpecTest.kt
index 0af694e..8f56c5f 100644
--- a/tests/src/com/android/launcher3/responsive/CalculatedWorkspaceSpecTest.kt
+++ b/tests/src/com/android/launcher3/responsive/CalculatedWorkspaceSpecTest.kt
@@ -104,4 +104,43 @@
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)
+ }
}