Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First focusable Compose component doesn't get focus in IDE toolwindow #3888

Closed
rock3r opened this issue Oct 31, 2023 · 11 comments · Fixed by JetBrains/compose-multiplatform-core#1352
Labels
bug Something isn't working desktop focus management

Comments

@rock3r
Copy link
Contributor

rock3r commented Oct 31, 2023

Describe the bug
When working in the IDE, if I have a ToolWindow with a ComposePanel as its tab content, when the tab is shown, the focus is not passed to the first focusable Composable, even though the ComposePanel does get focused.

Affected platforms
Select one of the platforms below:

  • Desktop

Versions

  • Kotlin version*: 1.8.20
  • Compose Multiplatform version*: 1.5.10
  • OS version(s)* (required for Desktop and iOS issues): macOS Sonoma, potentially others
  • OS architecture (x86 or arm64): arm64 (probably irrelevant)
  • JDK (for desktop issues): JBR 17

To Reproduce
Create an IJ plugin using Jewel. Add a toolwindow and add Compose content to it.

val composePanel = ComposePanel()

composePanel.setContent {
    MyTheme {
        var text by remember { mutableStateOf("") }
        TextField(text, { text = it })
    }
}
val wrapper = Wrapper(composePanel) // com.intellij.ui.components.panels.Wrapper
val tabContent = contentManager.factory.createContent(wrapper, "My tab", false)
tabContent.isCloseable = isCloseable
contentManager.addContent(tabContent)

Expected behavior
The first composable component should get focused when the composition starts.

Screenshots
image

Additional context
@Walingar can provide it

@m-sasha
Copy link
Contributor

m-sasha commented May 9, 2024

Does it work if you request focus on the TextField?

val focusRequester = remember { FocusRequester() }
TextField(
    ...
    modifier = Modifier.focusRequester(focusRequester)
)
LaunchedEffect(Unit) {
    focusRequester.requestFocus()
}

@rock3r
Copy link
Contributor Author

rock3r commented May 9, 2024

Yep, that's the current workaround in the Jewel IDE sample. It isn't good enough tho; you can either only ever do it the first time a tab is shown (with a Launched effect(Unit)), or every time (with a SideEffect). But in the former case, this means the next time the tab is shown it won't get focus in the composition, and in the latter it will reset focus to a given component, which isn't necessarily the one that was last focused.

@m-sasha
Copy link
Contributor

m-sasha commented May 9, 2024

I don't think this is an issue specific to ComposePanel - a tabbed pane in pure Compose would have the same problem.
So the question is in general about Compose - how to save/restore focus when switching between tabs/screens. I would encourage you to ask for assistance in the #compose channel in the kotlinlang Slack.

@rock3r
Copy link
Contributor Author

rock3r commented May 9, 2024

The problem I reported here is that when the ComposePanel gets focused (it is never destroyed when not visible, to be clear), it doesn't pass the focus inwards to its content.

This is the issue, and not how to save/restore focus, which is not necessary in this case. It is not a "holding it wrong" problem, as far as I can tell. Nikolay was aware of this issue before moving out of the team and may have further information about the internals, but he did validate this as a ComposePanel issue.

@m-sasha
Copy link
Contributor

m-sasha commented May 9, 2024

The problem I reported here is that when the ComposePanel gets focused (it is never destroyed when not visible, to be clear), it doesn't pass the focus inwards to its content.

But it does (see code below); it's just that Compose does not automatically focus anything. If you open a regular Compose application (without ComposePanel) with a TextField, and don't manually request focus for it, it will not be focused.

Here is a ComposePanel with a textfield that requests focus for itself when shown:

fun main() = SwingUtilities.invokeLater {
    val frame = JFrame()
    val button = JButton("Show Compose Panel")
    button.addActionListener {
        val composePanel = ComposePanel().apply {
            setContent {
                val focusRequester = remember { FocusRequester() }
                var text by remember { mutableStateOf("") }
                TextField(
                    value = text,
                    onValueChange = { text = it },
                    modifier = Modifier.focusRequester(focusRequester)
                )
                LaunchedEffect(Unit) {
                    focusRequester.requestFocus()
                }
            }
        }
        composePanel.size = Dimension(400, 200)
        composePanel.location = Point(0, 100)
        frame.contentPane.add(composePanel, BorderLayout.CENTER)
    }
    frame.contentPane.add(button, BorderLayout.NORTH)
    frame.size = Dimension(400, 300)
    frame.isVisible = true
}

@rock3r
Copy link
Contributor Author

rock3r commented May 9, 2024

Maybe I am not explaining myself clearly enough, so let me try again.

First things first: I think it is reasonable to expect that when a ComposePanel gets focus for the first time, and it contains focusable composables, it will pass the focus inwards to the first focusable composable. This is because:

  1. It makes no sense for the ComposePanel itself to hold the focus (just like a JPanel wouldn't, it would pass it down to its first focusable JComponent)
  2. This is how containers that have focusable children generally work in UI frameworks, including Swing and Compose

With that out of way, let's look at what's happening here. I have created the most minimal reproducer I can think of: a JTabbedPane with a ComposePanel (no external dependencies), and a JPanel, each having three buttons. You can find the reproducer code here: https://github.com/rock3r/cfd-repros/blob/main/src/main/kotlin/TabbedUI.kt

Screen recording

  1. The ComposePanel is the initially visible tab content. The first focusable/clickable composable does not get the focus.
  2. The JPanel is the second tab's content. When I switch to it, its first component is focused.
  3. Switching back to the Compose tab, the composition has lost the focus again. I would expect it to get the focus back when switching to it, just like the Swing version does.

@rock3r
Copy link
Contributor Author

rock3r commented May 9, 2024

Again, to be extra clear: I am ok with whatever is the first focusable component in the ComposePanel getting the focus. I am not asking to save which composable specifically has the focus, and restoring it (although there is no sane way to do that with the current focus APIs, that is a different discussion and not one that is important here)

@m-sasha
Copy link
Contributor

m-sasha commented May 9, 2024

First things first: I think it is reasonable to expect that when a ComposePanel gets focus for the first time, and it contains focusable composables, it will pass the focus inwards to the first focusable composable. This is because:

  1. It makes no sense for the ComposePanel itself to hold the focus (just like a JPanel wouldn't, it would pass it down to its first focusable JComponent)
  2. This is how containers that have focusable children generally work in UI frameworks, including Swing and Compose

This is true in Swing, but isn't true in Compose. I agree it's reasonable to expect, but it's just not true.

However, it appears that ComposePanel is making some effort to behave like a Swing component would. It sometimes succeeds, but not always.

When I run the reproducer you linked to (in jb-main), the Compose "First button" does initially get focus. What makes it happen is a focus listener in ComposePanel, which calls focusManager.moveFocus(FocusDirection.Next) when it receives a FocusEvent.Cause.TRAVERSAL_FORWARD event.

When I switch to the Swing tab and back, however, the "First button" is no longer focused. This is because the cause of the focus event in this case is, for some reason, FocusEvent.Cause.UNKNOWN, and the focus listener doesn't do anything on that. If I add

    when (e.cause) {
        ...
        FocusEvent.Cause.UNKNOWN -> {
            focusManager.moveFocus(FocusDirection.Enter)
        }
    }

the "First button" receives focus correctly again.

It appears that FocusEvent.Cause.UNKNOWN is also what is sent when we just do ComposePanel.requestFocus(), which also fixes, for example, this use case:

fun main() = SwingUtilities.invokeLater {
    val frame = JFrame()
    val button = JButton("Show Compose Panel")
    var composePanel: ComposePanel? = null

    fun addComposePanel() {
        val panel = ComposePanel()
        panel.setContent {
            var text by remember { mutableStateOf("") }
            TextField(
                value = text,
                onValueChange = { text = it },
            )
        }
        panel.size = Dimension(400, 200)
        panel.location = Point(0, 100)
        frame.contentPane.add(panel, BorderLayout.CENTER)
        composePanel = panel
    }

    button.addActionListener {
        if (composePanel == null)
            addComposePanel()
        else
            composePanel!!.isVisible = !composePanel!!.isVisible
        button.setText(if (composePanel!!.isVisible) "Hide ComposePanel" else "Show ComposePanel")
        if (composePanel!!.isVisible)
            composePanel!!.requestFocus()
    }
    frame.contentPane.add(button, BorderLayout.NORTH)
    frame.size = Dimension(400, 300)
    frame.isVisible = true
}

I don't exactly know why UNKNOWN is sent, or what other causes may be sent, but it appears that this particular change fixes this particular case, so we can try it.

@rock3r
Copy link
Contributor Author

rock3r commented May 9, 2024

Thanks for the in-depth analysis, Sasha. I am not sure why the cause is set to UNKNOWN, but that's probably because it's a programmatic change (the JTabbedPane does it on tab change, I reckon).

It'd be great to at least sort out that case.

@m-sasha
Copy link
Contributor

m-sasha commented May 9, 2024

By the way, in your reproducer, onFocusChanged comes after clickable (focusable, really). If you want it to actually be called, it needs to come before.

@rock3r
Copy link
Contributor Author

rock3r commented May 9, 2024

Ah, yep — just a leftover from a previous iteration that also had focusable. Thanks for pointing it out tho — it's an easy mistake to make.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working desktop focus management
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants