JetBrains / jewel

An implementation of the IntelliJ look and feels in Compose for Desktop
Apache License 2.0
733 stars 39 forks source link

Compose 1.7 breaks ComposePanel #504

Open francisconoriega opened 3 months ago

francisconoriega commented 3 months ago

I found a pretty odd bug in compose 1.7 that doesn't repro in 1.6.11:

When using a Modifier.fillMaxWidth, fillMaxHeight or fillMaxSize, the bounds of the panel are broken whenever you change tabs or click on another panel's icon. Resizing the ComposePanel seems to fix things again.

Weirdly enough, for AS it only seems to happen if you also have a bottom panel open.

This issue seems to be more in AS and/or Compose 1.7 itself, but I wonder if this means jewel should go back to 1.6.11 so it doesn't have to force users into the faulty 1.7

Here's a minimalist example I manage to get that repros the issue:

internal class JewelDemoToolWindowFactory : ToolWindowFactory, DumbAware {
    override fun createToolWindowContent(project: Project, toolWindow: ToolWindow) {
        val composePanel = ComposePanel()
        composePanel.setContent {
            Column(modifier = Modifier.border(2.dp, Color.Magenta)) {
                Row(
                    modifier = Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.End
                ) {
                    Box(Modifier.size(50.dp).background(Color.Green))
                }
                Column(
                    modifier = Modifier.fillMaxHeight(), verticalArrangement = Arrangement.Bottom
                ) { Box(Modifier.size(50.dp).background(Color.Green)) }
            }
        }
        toolWindow.contentManager.apply {
            val content = factory.createContent(composePanel, null, false)
            addContent(content)
        }
    }
}

Repro video:

https://github.com/user-attachments/assets/63e55b5b-07f3-46c8-ad3a-34b1c3c61543

francisconoriega commented 3 months ago

it might be related to https://github.com/JetBrains/compose-multiplatform/issues/3206

francisconoriega commented 3 months ago

I've opened https://youtrack.jetbrains.com/issue/CMP-5856/Desktop-ComposePanel-size-breaks-with-.fillMax-modifiers on the Jetbrains side, but I still think it would be worth considering downgrading to 1.6 in jewel for the meantime

rock3r commented 3 months ago

Hi @francisconoriega, thanks for the thorough report! I will escalate with the Compose Multiplatform team. I can confirm I can also reproduce the issue, although it's intermittent and not happening 100% of the time.

Since we rely on some APIs introduced in 1.7, since 0.20, we can't downgrade to Compose 1.6.

francisconoriega commented 3 months ago

@rock3r I have found a workaround for this issue. I haven't done extensive testing, but so far it seems to be good.

The issue stopped happening by wrapping my top content with:

@Composable
fun Compose17IJSizeBugWorkaround(content: @Composable () -> Unit) = with(LocalDensity.current) {
    Box(modifier = Modifier.requiredSize(LocalWindowInfo.current.containerSize.toSize().toDpSize())) {
        content()
    }
}

Any changes in the panel size causes this to recompose, and get the correct size. By using requiredSize, it can set hard limits to any internal use of fillMax* calls.

I think that we could even have something like this inside public fun ToolWindow.addComposeTab(, so that anyone currently using facing the issue would get the fix.

Hopefully, if 1.7beta01 fixes it, we would just remove this single location.

Let me know if that sounds reasonable, I can send a PR

rock3r commented 1 day ago

The fix was shipped in Compose 1.7.0 and we have it, however in some cases I can still repro. Hence, we're leaving the workaround in for now