Merge "Add 1D Focus Search support for Lazylists" into androidx-main
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListFocusMoveTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListFocusMoveTest.kt
index f252624..09d9cc4 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListFocusMoveTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyListFocusMoveTest.kt
@@ -35,7 +35,9 @@
import androidx.compose.ui.focus.FocusDirection.Companion.Down
import androidx.compose.ui.focus.FocusDirection.Companion.In
import androidx.compose.ui.focus.FocusDirection.Companion.Left
+import androidx.compose.ui.focus.FocusDirection.Companion.Next
import androidx.compose.ui.focus.FocusDirection.Companion.Out
+import androidx.compose.ui.focus.FocusDirection.Companion.Previous
import androidx.compose.ui.focus.FocusDirection.Companion.Right
import androidx.compose.ui.focus.FocusDirection.Companion.Up
import androidx.compose.ui.focus.FocusManager
@@ -91,7 +93,7 @@
@JvmStatic
@Parameterized.Parameters(name = "{0}")
fun initParameters() = buildList {
- for (focusDirection in listOf(Left, Right, Up, Down)) {
+ for (focusDirection in listOf(Previous, Next, Left, Right, Up, Down, In, Out)) {
for (reverseLayout in listOf(true, false)) {
for (layoutDirection in listOf(Ltr, Rtl)) {
add(Param(focusDirection, reverseLayout, layoutDirection))
@@ -120,24 +122,20 @@
// Assert.
rule.runOnIdle {
- assertThat(success).isTrue()
+ assertThat(success).apply { if (focusDirection == In) isFalse() else isTrue() }
when (focusDirection) {
Left -> when (layoutDirection) {
Ltr -> assertThat(isFocused[if (reverseLayout) 2 else 0]).isTrue()
- Rtl -> {
- // Disabling this case due to b/230758535
- // assertThat(isFocused[if (reverseLayout) 0 else 2]).isTrue()
- }
+ Rtl -> assertThat(isFocused[if (reverseLayout) 0 else 2]).isTrue()
}
Right -> when (layoutDirection) {
Ltr -> assertThat(isFocused[if (reverseLayout) 0 else 2]).isTrue()
- Rtl -> {
- // Disabling this case due to b/230758535
- // assertThat(isFocused[if (reverseLayout) 2 else 0]).isTrue()
- }
+ Rtl -> assertThat(isFocused[if (reverseLayout) 2 else 0]).isTrue()
}
Up -> assertThat(isFocused[if (reverseLayout) 2 else 0]).isTrue()
Down -> assertThat(isFocused[if (reverseLayout) 0 else 2]).isTrue()
+ Previous -> assertThat(isFocused[0]).isTrue()
+ Next -> assertThat(isFocused[2]).isTrue()
In -> assertThat(isFocused[1]).isTrue()
Out -> assertThat(isLazyListFocused).isTrue()
else -> unsupportedDirection()
@@ -156,11 +154,16 @@
}
}
rule.runOnIdle {
- // Scroll so that the focused item is in the middle,
- // then move focus to the last visible item.
+ // Scroll so that the focused item is in the middle.
runBlocking { lazyListState.scrollToItem(4) }
+
+ // Move focus to the last visible item.
initiallyFocused.requestFocus()
- focusManager.moveFocus(focusDirection)
+ when (focusDirection) {
+ Left, Right, Up, Down, Previous, Next -> focusManager.moveFocus(focusDirection)
+ In, Out -> { /* Do nothing */ }
+ else -> unsupportedDirection()
+ }
}
// Act.
@@ -170,7 +173,7 @@
// Assert.
rule.runOnIdle {
- assertThat(success).isTrue()
+ assertThat(success).apply { if (focusDirection == In) isFalse() else isTrue() }
when (focusDirection) {
Left -> when (layoutDirection) {
Ltr -> assertThat(isFocused[if (reverseLayout) 7 else 3]).isTrue()
@@ -182,6 +185,8 @@
}
Up -> assertThat(isFocused[if (reverseLayout) 7 else 3]).isTrue()
Down -> assertThat(isFocused[if (reverseLayout) 3 else 7]).isTrue()
+ Previous -> assertThat(isFocused[3]).isTrue()
+ Next -> assertThat(isFocused[7]).isTrue()
In -> assertThat(isFocused[5]).isTrue()
Out -> assertThat(isLazyListFocused).isTrue()
else -> unsupportedDirection()
@@ -204,11 +209,16 @@
}
}
rule.runOnIdle {
- // Scroll so that the focused item is in the middle,
- // then move focus to the last visible item.
+ // Scroll so that the focused item is in the middle.
runBlocking { lazyListState.scrollToItem(105) }
initiallyFocused.requestFocus()
- focusManager.moveFocus(focusDirection)
+
+ // Move focus to the last visible item.
+ when (focusDirection) {
+ Left, Right, Up, Down, Previous, Next -> focusManager.moveFocus(focusDirection)
+ In, Out -> { /* Do nothing */ }
+ else -> unsupportedDirection()
+ }
}
// Act.
@@ -218,7 +228,7 @@
// Assert.
rule.runOnIdle {
- assertThat(success).isTrue()
+ assertThat(success).apply { if (focusDirection == In) isFalse() else isTrue() }
when (focusDirection) {
Left -> when (layoutDirection) {
Ltr -> assertThat(isFocused[if (reverseLayout) 208 else 4]).isTrue()
@@ -230,6 +240,8 @@
}
Up -> assertThat(isFocused[if (reverseLayout) 208 else 4]).isTrue()
Down -> assertThat(isFocused[if (reverseLayout) 4 else 208]).isTrue()
+ Previous -> assertThat(isFocused[4]).isTrue()
+ Next -> assertThat(isFocused[208]).isTrue()
In -> assertThat(isFocused[106]).isTrue()
Out -> assertThat(isLazyListFocused).isTrue()
else -> unsupportedDirection()
@@ -250,11 +262,16 @@
}
}
rule.runOnIdle {
- // Scroll so that the focused item is in the middle,
- // then move focus to the last visible item.
+ // Scroll so that the focused item is in the middle.
runBlocking { lazyListState.scrollToItem(1) }
initiallyFocused.requestFocus()
- focusManager.moveFocus(focusDirection)
+
+ // Move focus to the last visible item.
+ when (focusDirection) {
+ Left, Right, Up, Down, Previous, Next -> focusManager.moveFocus(focusDirection)
+ In, Out -> { /* Do nothing */ }
+ else -> unsupportedDirection()
+ }
}
// Act.
@@ -264,7 +281,7 @@
// Assert.
rule.runOnIdle {
- assertThat(success).isTrue()
+ assertThat(success).apply { if (focusDirection == In) isFalse() else isTrue() }
when (focusDirection) {
Left -> when (layoutDirection) {
Ltr -> assertThat(isFocused[if (reverseLayout) 8 else 0]).isTrue()
@@ -276,6 +293,8 @@
}
Up -> assertThat(isFocused[if (reverseLayout) 8 else 0]).isTrue()
Down -> assertThat(isFocused[if (reverseLayout) 2 else 6]).isTrue()
+ Previous -> assertThat(isFocused[2]).isTrue()
+ Next -> assertThat(isFocused[6]).isTrue()
In -> assertThat(isFocused[4]).isTrue()
Out -> assertThat(isLazyListFocused).isTrue()
else -> unsupportedDirection()
@@ -296,11 +315,17 @@
}
}
rule.runOnIdle {
- // Scroll so that the focused item is in the middle,
- // then move focus to the last visible item.
+ // Scroll so that the focused item is in the middle.
runBlocking { lazyListState.scrollToItem(1) }
initiallyFocused.requestFocus()
- focusManager.moveFocus(focusDirection)
+
+ // Move focus to the last visible item.
+ when (focusDirection) {
+ Left, Right, Up, Down -> focusManager.moveFocus(focusDirection)
+ Previous, Next -> repeat(3) { focusManager.moveFocus(focusDirection) }
+ In, Out -> { /* Do nothing */ }
+ else -> unsupportedDirection()
+ }
}
// Act.
@@ -310,7 +335,7 @@
// Assert.
rule.runOnIdle {
- assertThat(success).isTrue()
+ assertThat(success).apply { if (focusDirection == In) isFalse() else isTrue() }
when (focusDirection) {
Left -> when (layoutDirection) {
Ltr -> assertThat(isFocused[if (reverseLayout) 8 else 0]).isTrue()
@@ -322,7 +347,9 @@
}
Up -> assertThat(isFocused[if (reverseLayout) 8 else 0]).isTrue()
Down -> assertThat(isFocused[if (reverseLayout) 0 else 8]).isTrue()
- In -> assertThat(isFocused[6]).isTrue()
+ Previous -> assertThat(isFocused[0]).isTrue()
+ Next -> assertThat(isFocused[8]).isTrue()
+ In -> assertThat(isFocused[4]).isTrue()
Out -> assertThat(isLazyListFocused).isTrue()
else -> unsupportedDirection()
}
@@ -330,7 +357,7 @@
}
@Test
- fun moveFocusToAmongItemsInNestedLazyLists() {
+ fun moveFocusAmongNestedLazyLists() {
// Arrange.
rule.setTestContent {
lazyList(30.dp, lazyListState) {
@@ -338,14 +365,21 @@
item { lazyListCrossAxis(30.dp) { items(3) { FocusableBox(it + 3) } } }
item { FocusableBox(6, initiallyFocused) }
item { lazyListCrossAxis(30.dp) { items(3) { FocusableBox(it + 7) } } }
- item { lazyListCrossAxis(30.dp) { items(3) { FocusableBox(it + 10) } } } }
+ item { lazyListCrossAxis(30.dp) { items(3) { FocusableBox(it + 10) } } }
+ }
}
rule.runOnIdle {
- // Scroll so that the focused item is in the middle,
- // then move focus to the last visible item.
+ // Scroll so that the focused item is in the middle.
runBlocking { lazyListState.scrollToItem(1) }
initiallyFocused.requestFocus()
- focusManager.moveFocus(focusDirection)
+
+ // Move focus to the last visible item.
+ when (focusDirection) {
+ Left, Right, Up, Down -> focusManager.moveFocus(focusDirection)
+ Previous, Next -> repeat(3) { focusManager.moveFocus(focusDirection) }
+ In, Out -> { /* Do nothing */ }
+ else -> unsupportedDirection()
+ }
}
// Act.
@@ -355,7 +389,7 @@
// Assert.
rule.runOnIdle {
- assertThat(success).isTrue()
+ assertThat(success).apply { if (focusDirection == In) isFalse() else isTrue() }
when (focusDirection) {
Left -> when (layoutDirection) {
Ltr -> assertThat(isFocused[if (reverseLayout) 12 else 0]).isTrue()
@@ -367,6 +401,8 @@
}
Up -> assertThat(isFocused[if (reverseLayout) 12 else 0]).isTrue()
Down -> assertThat(isFocused[if (reverseLayout) 2 else 10]).isTrue()
+ Previous -> assertThat(isFocused[2]).isTrue()
+ Next -> assertThat(isFocused[10]).isTrue()
In -> assertThat(isFocused[6]).isTrue()
Out -> assertThat(isLazyListFocused).isTrue()
else -> unsupportedDirection()
@@ -402,7 +438,7 @@
content: LazyListScope.() -> Unit
) {
when (focusDirection) {
- Left, Right, In, Out -> LazyRow(
+ Left, Right, In, Out, Next, Previous -> LazyRow(
modifier = Modifier
.size(size)
.onFocusChanged { isLazyListFocused = it.isFocused }
@@ -431,7 +467,7 @@
content: LazyListScope.() -> Unit
) {
when (focusDirection) {
- Left, Right, In, Out -> LazyColumn(
+ Left, Right, In, Out, Next, Previous -> LazyColumn(
modifier = Modifier.size(size),
state = state,
reverseLayout = reverseLayout,
diff --git a/compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/focus/ScrollableLazyRowFocusDemo.kt b/compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/focus/ScrollableLazyRowFocusDemo.kt
index 4a11f9b..e1302ca 100644
--- a/compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/focus/ScrollableLazyRowFocusDemo.kt
+++ b/compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/focus/ScrollableLazyRowFocusDemo.kt
@@ -44,7 +44,7 @@
val state = rememberLazyListState()
var reverseLayout by remember { mutableStateOf(false) }
Column {
- Text("Use the DPad to move focus in the Row")
+ Text("Use the DPad or Tab keys to move focus in the Row")
LazyRow(state = state, reverseLayout = reverseLayout) {
items(count = 10) { FocusableBox(it.toString()) }
items(count = 10) { NonFocusableBox((it + 10).toString()) }
diff --git a/compose/ui/ui/lint-baseline.xml b/compose/ui/ui/lint-baseline.xml
index 3f1f980..086592c 100644
--- a/compose/ui/ui/lint-baseline.xml
+++ b/compose/ui/ui/lint-baseline.xml
@@ -121,24 +121,6 @@
<issue
id="BanInlineOptIn"
message="Inline functions cannot opt into experimental APIs."
- errorLine1="private inline fun <T> MutableVector<T>.forEachItemAfter(item: T, action: (T) -> Unit) {"
- errorLine2=" ~~~~~~~~~~~~~~~~">
- <location
- file="src/commonMain/kotlin/androidx/compose/ui/focus/OneDimensionalFocusSearch.kt"/>
- </issue>
-
- <issue
- id="BanInlineOptIn"
- message="Inline functions cannot opt into experimental APIs."
- errorLine1="private inline fun <T> MutableVector<T>.forEachItemBefore(item: T, action: (T) -> Unit) {"
- errorLine2=" ~~~~~~~~~~~~~~~~~">
- <location
- file="src/commonMain/kotlin/androidx/compose/ui/focus/OneDimensionalFocusSearch.kt"/>
- </issue>
-
- <issue
- id="BanInlineOptIn"
- message="Inline functions cannot opt into experimental APIs."
errorLine1="internal inline fun <T, R> List<T>.fastZipWithNext(transform: (T, T) -> R): List<R> {"
errorLine2=" ~~~~~~~~~~~~~~~">
<location
diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/OneDimensionalFocusSearch.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/OneDimensionalFocusSearch.kt
index cd15209..bd668d4 100644
--- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/OneDimensionalFocusSearch.kt
+++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/OneDimensionalFocusSearch.kt
@@ -45,7 +45,8 @@
): Boolean = when (focusState) {
ActiveParent, DeactivatedParent -> {
val focusedChild = focusedChild ?: error(NoActiveChild)
- focusedChild.forwardFocusSearch(onFound) || searchChildren(focusedChild, Next, onFound)
+ focusedChild.forwardFocusSearch(onFound) ||
+ generateAndSearchChildren(focusedChild, Next, onFound)
}
Active, Captured, Deactivated -> pickChildForForwardSearch(onFound)
Inactive -> onFound.invoke(this)
@@ -65,11 +66,11 @@
DeactivatedParent -> focusedChild.backwardFocusSearch(onFound) ||
// Since this item is deactivated, just skip it and search among its siblings.
- searchChildren(focusedChild, Previous, onFound)
+ generateAndSearchChildren(focusedChild, Previous, onFound)
// Since this item "is focused", it means we already visited all its children.
// So just search among its siblings.
- Active, Captured -> searchChildren(focusedChild, Previous, onFound)
+ Active, Captured -> generateAndSearchChildren(focusedChild, Previous, onFound)
Deactivated, Inactive -> error(NoActiveChild)
}
@@ -86,6 +87,28 @@
Inactive -> pickChildForBackwardSearch(onFound) || onFound.invoke(this)
}
+// Search among your children for the next child.
+// If the next child is not found, generate more children by requesting a beyondBoundsLayout.
+private fun FocusModifier.generateAndSearchChildren(
+ focusedItem: FocusModifier,
+ direction: FocusDirection,
+ onFound: (FocusModifier) -> Boolean
+): Boolean {
+ // Search among the currently available children.
+ if (searchChildren(focusedItem, direction, onFound)) {
+ return true
+ }
+
+ // Generate more items until searchChildren() finds a result.
+ return searchBeyondBounds(direction) {
+ // Search among the added children. (The search continues as long as we return null).
+ searchChildren(focusedItem, direction, onFound).takeIf { found ->
+ // Stop searching when we find a result or if we don't have any more content.
+ found || !hasMoreContent
+ }
+ } ?: false
+}
+
// Search for the next sibling that should be granted focus.
private fun FocusModifier.searchChildren(
focusedItem: FocusModifier,
@@ -95,7 +118,7 @@
check(focusState == ActiveParent || focusState == DeactivatedParent) {
"This function should only be used within a parent that has focus."
}
-
+ children.sort()
when (direction) {
Next -> children.forEachItemAfter(focusedItem) { child ->
if (child.isEligibleForFocusSearch && child.forwardFocusSearch(onFound)) return true
@@ -117,13 +140,17 @@
private fun FocusModifier.pickChildForForwardSearch(
onFound: (FocusModifier) -> Boolean
-): Boolean = children.any { it.forwardFocusSearch(onFound) }
+): Boolean {
+ children.sort()
+ return children.any { it.isEligibleForFocusSearch && it.forwardFocusSearch(onFound) }
+}
private fun FocusModifier.pickChildForBackwardSearch(
onFound: (FocusModifier) -> Boolean
): Boolean {
+ children.sort()
children.forEachReversed {
- if (it.backwardFocusSearch(onFound)) {
+ if (it.isEligibleForFocusSearch && it.backwardFocusSearch(onFound)) {
return true
}
}
@@ -132,6 +159,7 @@
private fun FocusModifier.isRoot() = parent == null
+@Suppress("BanInlineOptIn")
@OptIn(ExperimentalContracts::class)
private inline fun <T> MutableVector<T>.forEachItemAfter(item: T, action: (T) -> Unit) {
contract { callsInPlace(action) }
@@ -146,6 +174,7 @@
}
}
+@Suppress("BanInlineOptIn")
@OptIn(ExperimentalContracts::class)
private inline fun <T> MutableVector<T>.forEachItemBefore(item: T, action: (T) -> Unit) {
contract { callsInPlace(action) }
@@ -159,3 +188,20 @@
}
}
}
+
+/**
+ * Sort the focus modifiers. in place order
+ *
+ * We want to visit the nodes in placement order instead of composition order.
+ * This is because components like LazyList reuse nodes without re-composing them, but it always
+ * re-places nodes that are reused.
+ *
+ * Instead of sorting the items, we could just look for the next largest place order index in linear
+ * time. However if the next item is deactivated, not eligible for focus search or none of its
+ * children are focusable we would have to backtrack and find the item with the next largest place
+ * order index. This would be more expensive than sorting the items. In addition to this, sorting
+ * the items makes the next focus search more efficient.
+ */
+private fun MutableVector<FocusModifier>.sort() {
+ sortWith(compareBy { it.layoutNodeWrapper?.layoutNode?.placeOrder })
+}
