Skip to content

Commit

Permalink
Adapt focus interop code to Compose 1.7 changes
Browse files Browse the repository at this point in the history
Fixes JetBrains/compose-multiplatform#2944
Fixes JetBrains/compose-multiplatform#4919
Fixes https://youtrack.jetbrains.com/issue/COMPOSE-1212/Integration.-Check-changes-in-FocusOwnerImpl

After https://android-review.googlesource.com/c/platform/frameworks/support/+/2813125, there are some changes that affected our code for focus interop with ComposePanel/SwingPanel:
- the root node no longer focusable
- FocusOwnerImpl now has iterop callbacks and our own modification of this file (in jb-main, integration was reset) doesn't work anymore

## Testing
- added new tests
- ComposeFocusTest now pass

## Release notes
### Fixes - Multiple platforms
- Fix "ComposePanel. Focus moves to child after focusing/unfocusing the main window"
  • Loading branch information
igordmn committed Jun 10, 2024
1 parent b6db74c commit 942bbfc
Show file tree
Hide file tree
Showing 11 changed files with 485 additions and 338 deletions.
6 changes: 1 addition & 5 deletions compose/ui/ui/api/desktop/ui.api
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,7 @@ public final class androidx/compose/ui/ComposeScene {
public final fun getLayoutDirection ()Landroidx/compose/ui/unit/LayoutDirection;
public final fun getRoots ()Ljava/util/Set;
public final fun hasInvalidations ()Z
public final fun moveFocus-3ESFkO8 (I)Z
public final fun releaseFocus ()V
public final fun render (Lorg/jetbrains/skia/Canvas;J)V
public final fun requestFocus ()V
public final fun sendKeyEvent-ZmokQxo (Ljava/lang/Object;)Z
public final fun sendPointerEvent-BGSDPeU (IJJJILandroidx/compose/ui/input/pointer/PointerButtons;Landroidx/compose/ui/input/pointer/PointerKeyboardModifiers;Ljava/lang/Object;Landroidx/compose/ui/input/pointer/PointerButton;)V
public static synthetic fun sendPointerEvent-BGSDPeU$default (Landroidx/compose/ui/ComposeScene;IJJJILandroidx/compose/ui/input/pointer/PointerButtons;Landroidx/compose/ui/input/pointer/PointerKeyboardModifiers;Ljava/lang/Object;Landroidx/compose/ui/input/pointer/PointerButton;ILjava/lang/Object;)V
Expand Down Expand Up @@ -3501,8 +3498,7 @@ public final class androidx/compose/ui/scene/ComposeSceneContext$Companion {

public abstract interface class androidx/compose/ui/scene/ComposeSceneFocusManager : androidx/compose/ui/focus/FocusManager {
public abstract fun getFocusRect ()Landroidx/compose/ui/geometry/Rect;
public abstract fun releaseFocus ()V
public abstract fun requestFocus ()V
public abstract fun getHasFocus ()Z
}

public abstract interface class androidx/compose/ui/scene/ComposeSceneLayer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ class ComposePanel @ExperimentalComposeUiApi constructor(
skiaLayerAnalytics = skiaLayerAnalytics,
windowContainer = windowContainer
).apply {
focusManager.releaseFocus()
setBounds(0, 0, width, height)
contentComponent.isFocusable = isFocusable
contentComponent.isRequestFocusEnabled = isRequestFocusEnabled
Expand All @@ -207,19 +206,12 @@ class ComposePanel @ExperimentalComposeUiApi constructor(
_focusListeners.forEach(contentComponent::addFocusListener)
contentComponent.addFocusListener(object : FocusListener {
override fun focusGained(e: FocusEvent) {
// The focus can be switched from the child component inside SwingPanel.
// In that case, SwingPanel will take care of it.
if (!isParentOf(e.oppositeComponent)) {
focusManager.requestFocus()
if (!e.isTemporary && !e.isFocusGainedHandledBySwingPanel(this@ComposePanel)) {
when (e.cause) {
FocusEvent.Cause.TRAVERSAL_FORWARD -> {
focusManager.moveFocus(FocusDirection.Next)
}
FocusEvent.Cause.TRAVERSAL_BACKWARD -> {
focusManager.moveFocus(FocusDirection.Previous)
}
FocusEvent.Cause.UNKNOWN, FocusEvent.Cause.ACTIVATION -> {
focusManager.moveFocus(FocusDirection.Enter)
if (!focusManager.hasFocus) {
focusManager.takeFocus(FocusDirection.Next)
}
}
else -> Unit
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public fun <T : Component> SwingPanel(
DisposableEffect(Unit) {
val focusListener = object : FocusListener {
override fun focusGained(e: FocusEvent) {
if (interopComponent.container.isParentOf(e.oppositeComponent)) {
if (e.isFocusGainedHandledBySwingPanel(interopComponent.container)) {
when (e.cause) {
FocusEvent.Cause.TRAVERSAL_FORWARD -> focusSwitcher.moveForward()
FocusEvent.Cause.TRAVERSAL_BACKWARD -> focusSwitcher.moveBackward()
Expand Down Expand Up @@ -156,6 +156,19 @@ public fun <T : Component> SwingPanel(
}
}

/**
* true, if the event is handled by SwingPanel.
*
* The focus can be switched from the child component inside SwingPanel.
* In that case, SwingPanel will take care of it.
*
* The alternative that needs more investigation is to
* not use ComposePanel as next/previous focus element for SwingPanel children
* (see [SwingPanelContainer.focusComponent])
*/
internal fun FocusEvent.isFocusGainedHandledBySwingPanel(container: Container) =
container.isParentOf(oppositeComponent)

/**
* A container for [SwingPanel]'s component. Takes care about focus and clipping.
*
Expand Down Expand Up @@ -194,67 +207,77 @@ private class FocusSwitcher<T : Component>(
private val interopComponent: SwingInteropComponent<T>,
private val focusManager: FocusManager,
) {
private val backwardRequester = FocusRequester()
private val forwardRequester = FocusRequester()
private var isRequesting = false
private val backwardTracker = FocusTracker {
val container = interopComponent.container
val component = container.focusTraversalPolicy.getFirstComponent(container)
if (component != null) {
component.requestFocus(FocusEvent.Cause.TRAVERSAL_FORWARD)
} else {
moveForward()
}
}

fun moveBackward() {
try {
isRequesting = true
backwardRequester.requestFocus()
} finally {
isRequesting = false
private val forwardTracker = FocusTracker {
val component = interopComponent.container.focusTraversalPolicy.getLastComponent(interopComponent.container)
if (component != null) {
component.requestFocus(FocusEvent.Cause.TRAVERSAL_BACKWARD)
} else {
moveBackward()
}
}

fun moveBackward() {
backwardTracker.requestFocusWithoutEvent()
focusManager.moveFocus(FocusDirection.Previous)
}

fun moveForward() {
try {
isRequesting = true
forwardRequester.requestFocus()
} finally {
isRequesting = false
}
forwardTracker.requestFocusWithoutEvent()
focusManager.moveFocus(FocusDirection.Next)
}

@Composable
fun Content() {
EmptyLayout(
Modifier
.focusRequester(backwardRequester)
.onFocusEvent {
if (it.isFocused && !isRequesting) {
focusManager.clearFocus(force = true)

val container = interopComponent.container
val component = container.focusTraversalPolicy.getFirstComponent(container)
if (component != null) {
component.requestFocus(FocusEvent.Cause.TRAVERSAL_FORWARD)
} else {
moveForward()
}
}
}
.focusTarget()
)
EmptyLayout(
Modifier
.focusRequester(forwardRequester)
.onFocusEvent {
if (it.isFocused && !isRequesting) {
focusManager.clearFocus(force = true)

val component = interopComponent.container.focusTraversalPolicy.getLastComponent(interopComponent.container)
if (component != null) {
component.requestFocus(FocusEvent.Cause.TRAVERSAL_BACKWARD)
} else {
moveBackward()
}
EmptyLayout(backwardTracker.modifier)
EmptyLayout(forwardTracker.modifier)
}

/**
* A helper class that can help:
* - to prevent recursive focus events
* (a case when we focus the same element inside `onFocusEvent`)
* - to prevent triggering `onFocusEvent` while requesting focus somewhere else
*/
private class FocusTracker(
private val onNonRecursiveFocused: () -> Unit
) {
private val requester = FocusRequester()

private var isRequestingFocus = false
private var isHandlingFocus = false

fun requestFocusWithoutEvent() {
try {
isRequestingFocus = true
requester.requestFocus()
} finally {
isRequestingFocus = false
}
}

val modifier = Modifier
.focusRequester(requester)
.onFocusEvent {
if (!isRequestingFocus && !isHandlingFocus && it.isFocused) {
try {
isHandlingFocus = true
onNonRecursiveFocused()
} finally {
isHandlingFocus = false
}
}
.focusTarget()
)
}
.focusTarget()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import androidx.compose.ui.awt.AwtEventListener
import androidx.compose.ui.awt.AwtEventListeners
import androidx.compose.ui.awt.OnlyValidPrimaryMouseButtonFilter
import androidx.compose.ui.awt.SwingInteropContainer
import androidx.compose.ui.awt.isFocusGainedHandledBySwingPanel
import androidx.compose.ui.awt.runOnEDTThread
import androidx.compose.ui.focus.FocusDirection
import androidx.compose.ui.focus.FocusManager
Expand Down Expand Up @@ -68,6 +69,9 @@ import java.awt.Toolkit
import java.awt.event.ContainerEvent
import java.awt.event.ContainerListener
import java.awt.event.FocusEvent
import java.awt.event.FocusEvent.Cause.TRAVERSAL
import java.awt.event.FocusEvent.Cause.TRAVERSAL_BACKWARD
import java.awt.event.FocusEvent.Cause.TRAVERSAL_FORWARD
import java.awt.event.FocusListener
import java.awt.event.InputEvent
import java.awt.event.InputMethodEvent
Expand Down Expand Up @@ -215,8 +219,20 @@ internal class ComposeSceneMediator(
// We don't reset focus for Compose when the component loses focus temporary.
// Partially because we don't support restoring focus after clearing it.
// Focus can be lost temporary when another window or popup takes focus.
if (!e.isTemporary) {
scene.focusManager.requestFocus()
if (!e.isTemporary && !e.isFocusGainedHandledBySwingPanel(container)) {
when (e.cause) {
TRAVERSAL_BACKWARD -> {
if (!focusManager.takeFocus(FocusDirection.Previous)) {
platformContext.parentFocusManager.moveFocus(FocusDirection.Previous)
}
}
TRAVERSAL, TRAVERSAL_FORWARD -> {
if (!focusManager.takeFocus(FocusDirection.Next)) {
platformContext.parentFocusManager.moveFocus(FocusDirection.Next)
}
}
else -> Unit
}
}
}

Expand Down Expand Up @@ -651,7 +667,14 @@ internal class ComposeSceneMediator(
}
override val parentFocusManager: FocusManager = DesktopFocusManager()
override fun requestFocus(): Boolean {
return contentComponent.hasFocus() || contentComponent.requestFocusInWindow()
// Don't check hasFocus(), and don't check the returning result
// Swing returns "false" if the window isn't visible or isn't active,
// but the component will always receive the focus after activation.
//
// if we return false - we don't allow changing the focus, and it breaks requesting
// focus at start and in inactive mode
contentComponent.requestFocusInWindow()
return true
}

override val rootForTestListener
Expand Down
Loading

0 comments on commit 942bbfc

Please sign in to comment.