[Focus] In AndroidView requestFocus for an composable outside of the AndroidView will be treat it as move to next(similar to tab). [376142752]

Fixed

Bug

Status Update

No update yet.

Description

jr...@google.com

created issue #1

Oct 29, 2024 01:06AM

Jetpack Compose version: Latest
Jetpack Compose component used: AndroidView, Focus
Android Studio Build: uild #AI-233.14808.21.2331.11574862
Kotlin version: Latest

Steps to Reproduce or Code Sample to Reproduce:
1. See cl/690811317 MainActivity.kt
2. Create several buttons under AndroidView. And several buttons outside of AndroidView in compose
3. Add an onKeyEvent on AndroidView that request focus for the buttons in compose.
3. Press the key that set in the onKeyEvent. The focus will move to the next AndroidView button instead of buttons incompose.

Stack trace (if applicable):

Comments

be...@google.com <be...@google.com> #2Oct 29, 2024 01:43PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ra...@google.com <ra...@google.com> #3Oct 29, 2024 11:24PM

Reassigned to mo...@google.com.

I was able to reproduce this using this snippet:

import android.widget.Button as ButtonView

@Composable
fun TestBox(modifier: Modifier = Modifier) {
    val requester = remember { FocusRequester() }
    Row(modifier) {
        Column(modifier = Modifier.focusGroup()) {
            Button({}, modifier = Modifier.focusRequester(requester)) { Text("Button 1") }
            Button({}) { Text("Button 2") }
        }
        Column {
            Button({}) { Text("Button 3") }
            AndroidView(
                factory = { context ->
                    LinearLayout(context).apply {
                        orientation = LinearLayout.VERTICAL
                        addView(ButtonView(context).apply {
                            text = "Android Button 4"
                            setOnClickListener { requester.requestFocus() }
                        })
                        addView(ButtonView(context).apply { text = "Android Button 5" })
                        addView(ButtonView(context).apply { text = "Android Button 6" })
                    }
                }
            )
        }
    }
}

Step 1: Use the arrow keys to navigate to Buttton 4.

Step 2: Press enter (Trigger requester.requestFocus()).

Step 3: Observer that focus moves to Button 5 insteade of Button 1.

It looks like the onExit of Modifier.focusInteropModifier() intercepts the focus exit and performs a requestFocus(FOCUS_DOWN) instead:

https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/viewinterop/FocusGroupNode.android.kt;drc=3537510cdeb3d7c3e108ec3f6732c7435571771d;l=94

George, can you help debug this further?

mo...@google.com <mo...@google.com> Nov 7, 2024 11:28PM

Accepted by mo...@google.com.

ap...@google.com <ap...@google.com> #4Dec 11, 2024 04:48PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main
Author: George Mount <mount@google.com>
Link: https://android-review.googlesource.com/3386976

Fix several focus issues

Expand for full commit details

Fix several focus issues 
 
Fixes: 369256395 
Fixes: 378570682 
Fixes: 376142752 
 
Relnote: "Fixed several focus-related issues, including 
crashes when IME tries to focus on ComposeView without 
focusable items, focus change within child AndroidViews, 
and focus request leaving an AndroidView." 
 
Allow focus to leave AndroidView using requestFocus() 
When requestFocus() is called to move focus from an 
Android View within Compose to a compose focus requester, 
the ComposeView should be focused and the focus target 
should have focus. 
 
This CL moves the functionality for moving focus 
between child Views to the key handler, moveFocus(), 
and requestFocus(). 
 
Test: new test, manual test 
Change-Id: Ia03c3c04dc9fd9d55d8fcae6b15e15cb2384f467

Files:

M compose/ui/ui/api/current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/FocusViewInteropTest.kt
M compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/focus/OwnerFocusTest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt
A compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/FocusFinderCompat.android.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/viewinterop/AndroidViewHolder.android.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/viewinterop/FocusGroupNode.android.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/ComposeUiFlags.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusOwnerImpl.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusTransactions.kt

Hash: 1467e40d3c1253ebb6e3b54f9bfdcdc93ef53da7
Date: Mon Dec 02 18:21:31 2024

pr...@google.com <pr...@google.com> #5Jan 15, 2025 06:31PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.ui:ui:1.8.0-alpha08
androidx.compose.ui:ui-android:1.8.0-alpha08
androidx.compose.ui:ui-jvmstubs:1.8.0-alpha08
androidx.compose.ui:ui-linuxx64stubs:1.8.0-alpha08

Issue 376142752