Merge "Fix focus transaction issue" into androidx-main
diff --git a/compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/FocusTransactionsTest.kt b/compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/FocusTransactionsTest.kt
index 5c098de..bd71cac 100644
--- a/compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/FocusTransactionsTest.kt
+++ b/compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/FocusTransactionsTest.kt
@@ -22,7 +22,6 @@
import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusRequester.Companion.Cancel
-import androidx.compose.ui.focus.FocusStateImpl.Active
import androidx.compose.ui.focus.FocusStateImpl.ActiveParent
import androidx.compose.ui.focus.FocusStateImpl.Inactive
import androidx.compose.ui.input.InputMode.Companion.Keyboard
@@ -92,6 +91,40 @@
}
}
+ @OptIn(ExperimentalComposeUiApi::class)
+ @Test
+ fun reentrantRequestFocus_byCallingRequestFocusWithinOnFocusChanged2() {
+ // Arrange.
+ val (item1, item2) = FocusRequester.createRefs()
+ var (item1Focused, item2Focused) = List(2) { false }
+ rule.setFocusableContent {
+ Box(
+ Modifier
+ .focusRequester(item1)
+ .onFocusChanged {
+ item1Focused = it.isFocused
+ if (item1Focused) item2.requestFocus()
+ }
+ .focusTarget()
+ )
+ Box(
+ Modifier
+ .focusRequester(item2)
+ .onFocusChanged { item2Focused = it.isFocused }
+ .focusTarget()
+ )
+ }
+
+ // Act.
+ rule.runOnIdle { item1.requestFocus() }
+
+ // Assert.
+ rule.runOnIdle {
+ assertThat(item1Focused).isFalse()
+ assertThat(item2Focused).isTrue()
+ }
+ }
+
@Test
fun cancelTakeFocus_fromOnFocusChanged() {
// Arrange.
@@ -129,10 +162,8 @@
// Assert.
rule.runOnIdle {
assertThat(focusState1).isEqualTo(Inactive)
- // TODO(b/312524818): When a focus transaction is cancelled, we should re-notify
- // all the focus event modifiers that were called in the previous transaction.
- assertThat(focusState2).isEqualTo(Active) // Should be Inactive.
- assertThat(focusState3).isEqualTo(Active) // Should be Inactive.
+ assertThat(focusState2).isEqualTo(Inactive)
+ assertThat(focusState3).isEqualTo(Inactive)
val root = view as AndroidComposeView
diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusOwnerImpl.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusOwnerImpl.kt
index 09e2c47..3666a3e 100644
--- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusOwnerImpl.kt
+++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusOwnerImpl.kt
@@ -132,11 +132,9 @@
* [FocusTargetNode] was found or if the focus search was cancelled.
*/
override fun takeFocus(focusDirection: FocusDirection, previouslyFocusedRect: Rect?): Boolean {
- return focusTransactionManager.withExistingTransaction {
- focusSearch(focusDirection, previouslyFocusedRect) {
- it.requestFocus(focusDirection) ?: false
- } ?: false
- }
+ return focusSearch(focusDirection, previouslyFocusedRect) {
+ it.requestFocus(focusDirection) ?: false
+ } ?: false
}
/**
diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactionManager.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactionManager.kt
index 59327a0..ab8d02b 100644
--- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactionManager.kt
+++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactionManager.kt
@@ -89,12 +89,13 @@
}
states.clear()
ongoingTransaction = false
+ cancellationListener.clear()
}
private fun cancelTransaction() {
- cancellationListener.forEach { it() }
- cancellationListener.clear()
states.clear()
ongoingTransaction = false
+ cancellationListener.forEach { it() }
+ cancellationListener.clear()
}
}
diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactions.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactions.kt
index 6078b91..24eaca0 100644
--- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactions.kt
+++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactions.kt
@@ -41,7 +41,9 @@
internal fun FocusTargetNode.requestFocus(): Boolean = requestFocus(Enter) ?: false
internal fun FocusTargetNode.requestFocus(focusDirection: FocusDirection): Boolean? {
- return requireTransactionManager().withNewTransaction {
+ return requireTransactionManager().withNewTransaction(
+ onCancelled = { if (node.isAttached) refreshFocusEventNodes() }
+ ) {
when (performCustomRequestFocus(focusDirection)) {
None -> performRequestFocus()
Redirected -> true
diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaBasedOwner.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaBasedOwner.skiko.kt
index dc83020..8cbcca3 100644
--- a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaBasedOwner.skiko.kt
+++ b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaBasedOwner.skiko.kt
@@ -206,10 +206,8 @@
init {
snapshotObserver.startObserving()
root.attach(this)
- focusOwner.focusTransactionManager.withNewTransaction {
- // TODO instead of taking focus here, call this when the owner gets focused.
- focusOwner.takeFocus(Enter, previouslyFocusedRect = null)
- }
+ // TODO instead of taking focus here, call this when the owner gets focused.
+ focusOwner.takeFocus(Enter, previouslyFocusedRect = null)
}
fun dispose() {
