Skip to content

Conversation

m-sasha
Copy link
Member

@m-sasha m-sasha commented Aug 15, 2025

Size SwingPanel according to its min/pref/max sizes.

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

Screen.Recording.2025-08-15.at.15.12.41.mp4

Testing

Added a unit test and tested manually with

fun main() {
    var text = "I'm a Swing JLabel"
    val label = JLabel(text)

    singleWindowApplication {
        Box(
            contentAlignment = Alignment.Center,
            modifier = Modifier
                .fillMaxSize()
                .background(Color.Red)
        ) {
            SwingPanel(
                factory = { label },
                modifier = Modifier
//                    .size(200.dp, 200.dp)
            )
        }

        LaunchedEffect(Unit) {
            delay(1000)
            while (true) {
                repeat(20) {
                    delay(200)
                    text = "#${text}#"
                    label.text = text
                }
                repeat(20) {
                    delay(200)
                    text = text.substring(1, text.length - 1)
                    label.text = text
                }
            }
        }
    }
}

Release Notes

Fixes - Desktop

  • SwingPanel no longer requires to be manually sized to a fixed value; it will size according to its content's min/pref/max sizes.

@m-sasha m-sasha requested a review from igordmn August 15, 2025 12:31
@m-sasha m-sasha force-pushed the m-sasha/size-swingpanel-according-to-its-dimensions branch from 2f12380 to 296317d Compare August 16, 2025 09:04
@m-sasha m-sasha requested a review from igordmn August 17, 2025 13:42
@m-sasha m-sasha force-pushed the m-sasha/size-swingpanel-according-to-its-dimensions branch 2 times, most recently from 0d1fc37 to 668dca5 Compare August 18, 2025 09:49
@m-sasha m-sasha requested a review from MatkovIvan August 18, 2025 09:49
private val MaxSupportedRadix = 36
private const val MaxSupportedRadix = 36

private class AwtContentMeasurePolicy(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move it into SwingInteropViewHolder or separate file

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the advantage of doing that?

Under the same reasoning you would move SwingInteropViewGroup into SwingInteropViewHolder.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not component directly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to ask this in the next conversation down below?

I could ask the component directly, but it seems a bit more correct to follow the hierarchy. Maybe tomorrow we'll want SwingPanels to have a border for debugging purposes or something.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's basically the same consern as with assigning the size directly - it's not supposed to be the same, and I'm wonder if this might break it in some cases

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to ask this in the next conversation down below?

Yep, sorry

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under the same reasoning you would move SwingInteropViewGroup into SwingInteropViewHolder.

As far as I remember @igordmn wanded to keep them file-private, but it's no longer the case

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It delegates directly to the child, so I don't see how it can break.

Also, it's not actually possible to ask the component directly. When SwingInteropViewHolder calls its super constructor, it needs to pass it measurePolicy, but at this point the component (typedInteropView) doesn't exist yet.

Copy link
Member Author

@m-sasha m-sasha Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it's not possible to create AwtContentMeasurePolicy unless it would be an inner class of SwingInteropViewHolder, or it would need to take a getter () -> Component rather than the Component itself. Seems too much...

@@ -149,9 +161,72 @@ internal class SwingInteropViewGroup(
}
isFocusCycleRoot = true
}

override fun getPreferredSize(): Dimension? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to override getPreferredSize/getMinimumSize/getMaximumSize if we control sizing manually now? Does it really related to the fix of the issue?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AwtContentMeasurePolicy queries SwingInteropViewGroup for its min/pref/max size.

@m-sasha m-sasha force-pushed the m-sasha/size-swingpanel-according-to-its-dimensions branch from 668dca5 to e327909 Compare August 18, 2025 10:49
Copy link
Member

@MatkovIvan MatkovIvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main blocker from previous iteration was resolved - not blocking anymore.

The main review should still on @igordmn

@m-sasha m-sasha force-pushed the m-sasha/size-swingpanel-according-to-its-dimensions branch from e327909 to 93dac40 Compare August 18, 2025 19:50
@m-sasha m-sasha merged commit 8f370ea into jb-main Aug 20, 2025
10 checks passed
@m-sasha m-sasha deleted the m-sasha/size-swingpanel-according-to-its-dimensions branch August 20, 2025 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants