Skip to content

Redispatch unconsumed mouse wheel events (#2425) #2438

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 2, 2025
Merged

Redispatch unconsumed mouse wheel events (#2425) #2438

merged 1 commit into from
Oct 2, 2025
+215 −17

Conversation

@m-sasha
Copy link
Member

@m-sasha m-sasha commented Oct 2, 2025 •
edited by kropp
Loading

This is a cherry-pick (with some adaptations) of #2425

Redispatch mouse wheel events which the Compose scene did not consume.

Difference with Swing behavior

This fix introduces a slight difference with how Swing handles nested mouse-wheel scrolling. In Swing, there is essentially no nested scrolling. In a scenario with a JScrollPane inside another JScrollPane, only the inner one can be scrolled when the mouse is over it. With this PR, a ComposePanel with a scrollable element within a JScrollPane will scroll until it reaches the start/end and then the outer JScrollPane will scroll.
This behavior seems to be an improvement, so we thought it was ok to have this difference.

Danger, Will Robinson

Note that this includes a potentially dangerous hack of whose consequences I'm not yet sure. If it proves too problematic, we will need to take a different approach (documented in the source).
The hack is that in order to allow AWT to find the correct target for the redispatched event, we temporarily unregister ComposeSceneMediator's mouse listeners.

Feature flag

I've put the behavior behind ComposeFeatureFlags.redispatchUnconsumedMouseWheelEvents. The flag will be:

  • false by default in 1.9.1
  • true by default in 1.10
  • Removed in 1.11, if no major problems are discovered.
    In this PR, it's true, as it merges into jb-main. In the cherry-pick to 1.9.1, we should change it to false. When it's approved I will add a ticket to remove it for 1.11

Other mouse events

Note that this PR only deals with mouse wheel events, but at least in the case of a lightweight skia layer (i.e. SkiaSwingLayer), we should probably do the same for other mouse events.

Fixes https://youtrack.jetbrains.com/issue/CMP-4601

Testing

Added unit tests, and also tested manually with

import androidx.compose.foundation.*
import androidx.compose.foundation.layout.*
import androidx.compose.material.*
import androidx.compose.runtime.Composable
import androidx.compose.ui.*
import androidx.compose.ui.awt.*
import androidx.compose.ui.graphics.*
import androidx.compose.ui.unit.*
import java.awt.*
import javax.swing.*

fun main() = SwingUtilities.invokeLater {
    val frame = JFrame("CfD repro")
    frame.defaultCloseOperation = JFrame.EXIT_ON_CLOSE
    frame.minimumSize = Dimension(500, 400)
    System.setProperty("compose.swing.render.on.graphics", "true")

    val mainPanel =
        JPanel(null).apply {
            val container =
                JPanel(null).apply {
                    layout = BoxLayout(this, BoxLayout.Y_AXIS)
                    addChildren()
                }
            val scrollPane = JScrollPane(container)
            scrollPane.setBounds(0, 0, 500, 500)
            add(scrollPane)
        }

    frame.contentPane.add(mainPanel, BorderLayout.CENTER)
    frame.isVisible = true
}

private fun JPanel.addChildren() {
    repeat(10) {
        repeat(5) {
            add(JLabel("<html>" + "Very long text here ".repeat(10) + "</html>"))
            add(Box.createVerticalStrut(10))
        }

        add(
            ComposePanel().apply {
                setContent {
                    ComposeBox()
                }
            }
        )
        add(Box.createVerticalStrut(10))
    }
}

@Composable
fun ComposeBox() {
    Box(
        modifier = Modifier
            .height(100.dp)
            .background(color = Color(180, 180, 180))
            .padding(10.dp)
    ) {
        val stateVertical = rememberScrollState(0)

        Box(
            modifier = Modifier
                .verticalScroll(stateVertical)
                .padding(end = 12.dp, bottom = 12.dp)
        ) {
            Column {
                for (item in 0..10) {
                    TextBox("Item #$item")
                }
            }
        }
    }
}

@Composable
fun TextBox(text: String = "Item") {
    Box(
        modifier = Modifier.height(32.dp)
            .width(400.dp)
            .background(color = Color(0, 0, 0, 20))
            .padding(start = 10.dp),
        contentAlignment = Alignment.CenterStart
    ) {
        Text(text = text)
    }
}

This should be tested by QA

Release Notes

Fixes - Desktop

  • ComposePanel can now re-dispatch unconsumed mouse wheel events, allowing scrollable components beneath to be scrolled. To enable this behavior, set the system property "compose.swing.redispatchMouseWheelEvents" to "true".

@m-sasha m-sasha requested a review from sekater October 2, 2025 12:11
@sekater sekater requested a review from igordmn October 2, 2025 12:18
@igordmn
Copy link
Collaborator

igordmn commented Oct 2, 2025 •
edited
Loading

Please:

  • change the release notes (remove the link, and copy with changing from the PR)
  • create the branch m-sasha/fix-mousewheel-event-propagation-1.9 in the teamcity config from release/1.9 to fix PR checks

@m-sasha
Copy link
Member Author

m-sasha commented Oct 2, 2025

Please:

  • change the release notes (remove the link, and copy with changing from the PR)

Hmm, this must've been done automatically. When I created the PR, I copied the text manually and changed the release notes. Anyway, changed them again now.

  • create the branch m-sasha/fix-mousewheel-event-propagation-1.9 in the teamcity config from release/1.9 to fix PR checks

Not sure how or why it's not working as-is.

@m-sasha m-sasha force-pushed the m-sasha/fix-mousewheel-event-propagation-1.9 branch from dd678b5 to cbc3c43 Compare October 2, 2025 12:32
@igordmn
Copy link
Collaborator

igordmn commented Oct 2, 2025

why it's not working as-is.

it is broken, we are fixing it, it is a workaround

Not sure how

Clone https://jetbrains.team/p/ui/repositories/compose-teamcity-config, create a branch m-sasha/fix-mousewheel-event-propagation-1.9, push. if you haven't registered your SSH key, register in Space -> Your Profile -> Preferences -> Git Keys

@m-sasha
Copy link
Member Author

m-sasha commented Oct 2, 2025

Clone https://jetbrains.team/p/ui/repositories/compose-teamcity-config, create a branch m-sasha/fix-mousewheel-event-propagation-1.9, push. if you haven't registered your SSH key, register in Space -> Your Profile -> Preferences -> Git Keys

Did that. What now?

@igordmn
Copy link
Collaborator

igordmn commented Oct 2, 2025 •
edited
Loading

Did that

Also, reset it to release/1.9 state, force push, then rerun all checks on GitHub

@sekater sekater merged commit 9665e6b into release/1.9 Oct 2, 2025
10 of 11 checks passed
@sekater sekater deleted the m-sasha/fix-mousewheel-event-propagation-1.9 branch October 2, 2025 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@igordmn igordmn igordmn approved these changes

@sekater sekater sekater approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@m-sasha @igordmn @sekater