Merge "Merge cherrypicks of [2115574] into androidx-compose-release." into androidx-compose-release

4 files changed, 124 insertions, 60 deletions

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 f2526242c63..09d9cc45584 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
 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 @@ class LazyListFocusMoveTest(param: Param) {
         @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 @@ class LazyListFocusMoveTest(param: Param) {
 
         // 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 @@ class LazyListFocusMoveTest(param: Param) {
             }
         }
         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 @@ class LazyListFocusMoveTest(param: Param) {
 
         // 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 @@ class LazyListFocusMoveTest(param: Param) {
                 }
                 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 @@ class LazyListFocusMoveTest(param: Param) {
             }
         }
         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 @@ class LazyListFocusMoveTest(param: Param) {
 
         // 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 @@ class LazyListFocusMoveTest(param: Param) {
                 }
                 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 @@ class LazyListFocusMoveTest(param: Param) {
             }
         }
         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 @@ class LazyListFocusMoveTest(param: Param) {
 
         // 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 @@ class LazyListFocusMoveTest(param: Param) {
                 }
                 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 @@ class LazyListFocusMoveTest(param: Param) {
             }
         }
         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 @@ class LazyListFocusMoveTest(param: Param) {
 
         // 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 @@ class LazyListFocusMoveTest(param: Param) {
                 }
                 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 @@ class LazyListFocusMoveTest(param: Param) {
     }
 
     @Test
-    fun moveFocusToAmongItemsInNestedLazyLists() {
+    fun moveFocusAmongNestedLazyLists() {
         // Arrange.
         rule.setTestContent {
             lazyList(30.dp, lazyListState) {
@@ -338,14 +365,21 @@ class LazyListFocusMoveTest(param: Param) {
                 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 @@ class LazyListFocusMoveTest(param: Param) {
 
         // 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 @@ class LazyListFocusMoveTest(param: Param) {
                 }
                 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 @@ class LazyListFocusMoveTest(param: Param) {
         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 @@ class LazyListFocusMoveTest(param: Param) {
         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 4a11f9b8d5e..e1302ca0c6e 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 @@ fun ScrollableLazyRowFocusDemo() {
     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 3f1f98015b5..086592ce7b8 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 &lt;T> MutableVector&lt;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 &lt;T> MutableVector&lt;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 &lt;T, R> List&lt;T>.fastZipWithNext(transform: (T, T) -> R): List&lt;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 cd15209fbb0..bd668d44c65 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 @@ private fun FocusModifier.forwardFocusSearch(
 ): 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 @@ private fun FocusModifier.backwardFocusSearch(
 
             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 @@ private fun FocusModifier.backwardFocusSearch(
     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 @@ private fun FocusModifier.searchChildren(
     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.searchChildren(
 
 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.pickChildForBackwardSearch(
 
 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 @@ private inline fun <T> MutableVector<T>.forEachItemAfter(item: T, action: (T) ->
     }
 }
 
+@Suppress("BanInlineOptIn")
 @OptIn(ExperimentalContracts::class)
 private inline fun <T> MutableVector<T>.forEachItemBefore(item: T, action: (T) -> Unit) {
     contract { callsInPlace(action) }
@@ -159,3 +188,20 @@ private inline fun <T> MutableVector<T>.forEachItemBefore(item: T, action: (T) -
         }
     }
 }
+
+/**
+ * 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 })
+}