search

JetBrains / jewel

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

Popups should use native windows #97

Open rock3r opened 1 year ago

rock3r commented 1 year ago

Our popup windows (e.g., menus, dropdowns, tooltips, etc) don't use native Swing popup windows. This causes a number of issues, such as:

  1. Popups are limited to showing inside the canvas that generated them
  2. They may not match the other Swing native windows in behavior
  3. Shadow needs to be implemented per-OS and it's hard to match OS behaviour, especially on Linux where there are a million variables at play
rock3r commented 11 months ago

This depends on https://github.com/JetBrains/compose-multiplatform/issues/2924 being done

rock3r commented 11 months ago

@Walingar we have internal users who are strongly requesting this. Can we have a commitment on a delivery date ASAP?

Walingar commented 9 months ago

As I know 1.6.0 Compose Multiplatform will provide some ways to show popup outside of the window.

Also there is a way to use JBPopup (that I will recommend), see this code as an example: https://github.com/JetBrains/intellij-community/blob/241.9959/platform/compose/src/com/intellij/platform/compose/ActionGroupPopup.kt

devkanro commented 9 months ago

Also there is a way to use JBPopup (that I will recommend)

But we can use JBPopup only in bridge, standalone should support it too

rock3r commented 9 months ago

Given we're on Compose 1.6 already, I reckon there should be an API to use for that in standalone apps? The way I see it, the API should be common between bridge and standalone, but the actual implementation should use the best available API.

rock3r commented 8 months ago

https://github.com/JetBrains/compose-multiplatform-core/pull/992 contains a new API for popups in 1.6.0 that we can use. We need to turn on a flag to enable the new behaviour on tooltips and popup APIs. Thanks @MatkovIvan for letting me know!

YasserDbeis commented 7 months ago

This bug is causing an issue with compose tooltips at the border of compose and swing components getting cut off so I tried adding the following line as indicated in https://github.com/JetBrains/compose-multiplatform-core/pull/992: System.setProperty("compose.layers.type", "COMPONENT")

I then tested an IconButton wrapped in a tooltip, where the tooltip content was a simple Text("foo"). The component was not consistently hoverable anymore and had no tooltip at all. It also took a few clicks before the onClick handler of the IconButton was invoked. Some of these symptoms can be attributed to the exception thrown on hover: 


java.lang.IllegalStateException: No TextStyle provided. Have you forgotten the theme?
    at _COROUTINE._BOUNDARY._(CoroutineDebugging.kt:46)
    at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2.invokeSuspend(Recomposer.kt:537)
    at androidx.compose.runtime.Recomposer$recompositionRunner$2$3.invokeSuspend(Recomposer.kt:946)
    at androidx.compose.runtime.Recomposer$recompositionRunner$2.invokeSuspend(Recomposer.kt:945)
    at androidx.compose.ui.scene.ComposeSceneRecomposer$1.invokeSuspend(ComposeSceneRecomposer.skiko.kt:72)
    Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [androidx.compose.ui.scene.ComposeContainer$DesktopCoroutineExceptionHandler@3311a37e, androidx.compose.runtime.BroadcastFrameClock@3980788f, CoroutineId(7078), "coroutine#7078":StandaloneCoroutine{Cancelled}@40e545f8, FlushCoroutineDispatcher@12c35d03]
Caused by: java.lang.IllegalStateException: No TextStyle provided. Have you forgotten the theme?
    at org.jetbrains.jewel.foundation.theme.JewelThemeKt$LocalTextStyle$1.invoke(JewelTheme.kt:101)
    at org.jetbrains.jewel.foundation.theme.JewelThemeKt$LocalTextStyle$1.invoke(JewelTheme.kt:100)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at androidx.compose.runtime.LazyValueHolder.getCurrent(ValueHolders.kt:29)
    at androidx.compose.runtime.LazyValueHolder.getValue(ValueHolders.kt:31)
    at androidx.compose.runtime.CompositionLocalMapKt.read(CompositionLocalMap.kt:88)
    at androidx.compose.runtime.ComposerImpl.consume(Composer.kt:2050)
    at org.jetbrains.jewel.foundation.theme.JewelTheme$Companion.getTextStyle(JewelTheme.kt:114)
    at org.jetbrains.jewel.ui.component.TextKt.Text-fLXpl1I(Text.kt:39)
    at com.android.tools.profilers.taskbased.tabs.taskgridandbars.taskbars.ComposableSingletons$TaskTitleBarKt$lambda-1$1.invoke(TaskTitleBar.kt:66)
    at com.android.tools.profilers.taskbased.tabs.taskgridandbars.taskbars.ComposableSingletons$TaskTitleBarKt$lambda-1$1.invoke(TaskTitleBar.kt:66)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:108)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:35)
    at org.jetbrains.jewel.ui.component.TooltipKt$Tooltip$1$1$1$1.invoke(Tooltip.kt:72)
    at org.jetbrains.jewel.ui.component.TooltipKt$Tooltip$1$1$1$1.invoke(Tooltip.kt:71)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:108)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:35)
    at androidx.compose.runtime.CompositionLocalKt.CompositionLocalProvider(CompositionLocal.kt:228)
    at org.jetbrains.jewel.foundation.theme.JewelThemeKt.OverrideDarkMode(JewelTheme.kt:109)
    at org.jetbrains.jewel.ui.component.TooltipKt$Tooltip$1$1.invoke(Tooltip.kt:71)
    at org.jetbrains.jewel.ui.component.TooltipKt$Tooltip$1$1.invoke(Tooltip.kt:51)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:108)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:35)
    at androidx.compose.runtime.CompositionLocalKt.CompositionLocalProvider(CompositionLocal.kt:228)
    at org.jetbrains.jewel.ui.component.TooltipKt$Tooltip$1.invoke(Tooltip.kt:49)
    at org.jetbrains.jewel.ui.component.TooltipKt$Tooltip$1.invoke(Tooltip.kt:48)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:108)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:35)
    at androidx.compose.foundation.TooltipArea_desktopKt$TooltipArea$6$2.invoke(TooltipArea.desktop.kt:170)
    at androidx.compose.foundation.TooltipArea_desktopKt$TooltipArea$6$2.invoke(TooltipArea.desktop.kt:159)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:108)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:35)
    at androidx.compose.ui.window.Popup_skikoKt$PopupLayout$2$1.invoke(Popup.skiko.kt:559)
    at androidx.compose.ui.window.Popup_skikoKt$PopupLayout$2$1.invoke(Popup.skiko.kt:452)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:108)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:35)
    at androidx.compose.ui.platform.ZeroInsetsConfig.excludeSafeInsets(PlatformInsets.skiko.kt:95)
    at androidx.compose.ui.window.Popup_skikoKt$PopupLayout$2.invoke(Popup.skiko.kt:452)
    at androidx.compose.ui.window.Popup_skikoKt$PopupLayout$2.invoke(Popup.skiko.kt:439)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:108)
    at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:35)
    at androidx.compose.runtime.RecomposeScopeImpl.compose(RecomposeScopeImpl.kt:169)
    at androidx.compose.runtime.ComposerImpl.recomposeToGroupEnd(Composer.kt:2469)
    at androidx.compose.runtime.ComposerImpl.skipCurrentGroup(Composer.kt:2738)
    at androidx.compose.runtime.ComposerImpl.doCompose(Composer.kt:3353)
    at androidx.compose.runtime.ComposerImpl.recompose$runtime(Composer.kt:3304)
    at androidx.compose.runtime.CompositionImpl.recompose(Composition.kt:781)
    at androidx.compose.runtime.Recomposer.performRecompose(Recomposer.kt:1097)
    at androidx.compose.runtime.Recomposer.access$performRecompose(Recomposer.kt:124)
    at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$1.invoke(Recomposer.kt:569)
    at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$1.invoke(Recomposer.kt:537)
    at androidx.compose.runtime.BroadcastFrameClock$FrameAwaiter.resume(BroadcastFrameClock.kt:42)
    at androidx.compose.runtime.BroadcastFrameClock.sendFrame(BroadcastFrameClock.kt:71)
    at androidx.compose.ui.scene.BaseComposeScene.render(BaseComposeScene.skiko.kt:150)
    at androidx.compose.ui.scene.ComposeSceneMediator$DesktopSkikoView.onRender(ComposeSceneMediator.desktop.kt:497)
    at org.jetbrains.skiko.swing.SkiaSwingLayer$skikoViewWithClipping$1.onRender(SkiaSwingLayer.kt:50)
    at org.jetbrains.skiko.swing.MetalSwingRedrawer$onRender$1$1.invoke(MetalSwingRedrawer.kt:73)
    at org.jetbrains.skiko.swing.MetalSwingRedrawer$onRender$1$1.invoke(MetalSwingRedrawer.kt:59)
    at org.jetbrains.skiko.ResourceUtilsKt.autoCloseScope(ResourceUtils.kt:34)
    at org.jetbrains.skiko.swing.MetalSwingRedrawer.onRender(MetalSwingRedrawer.kt:59)
    at org.jetbrains.skiko.swing.SwingRedrawerBase.redraw(SwingRedrawerBase.kt:51)
    at org.jetbrains.skiko.swing.SkiaSwingLayer.paint(SkiaSwingLayer.kt:111)
    at androidx.compose.ui.scene.skia.SwingSkiaLayerComponent$contentComponent$1.paint(SwingSkiaLayerComponent.desktop.kt:50)
    at java.desktop/javax.swing.JComponent.paintChildren(JComponent.java:955)
    at java.desktop/javax.swing.JComponent.paint(JComponent.java:1124)
    at java.desktop/javax.swing.JLayeredPane.paint(JLayeredPane.java:586)
    at androidx.compose.ui.scene.SwingComposeSceneLayer$container$1.paint(SwingComposeSceneLayer.desktop.kt:63)
    at java.desktop/javax.swing.JComponent.paintChildren(JComponent.java:955)
    at java.desktop/javax.swing.JComponent.paint(JComponent.java:1124)
    at java.desktop/javax.swing.JLayeredPane.paint(JLayeredPane.java:586)
    at java.desktop/javax.swing.JComponent.paintChildren(JComponent.java:955)
    at java.desktop/javax.swing.JComponent.paint(JComponent.java:1124)
    at java.desktop/javax.swing.JComponent.paintToOffscreen(JComponent.java:5312)
    at java.desktop/javax.swing.BufferStrategyPaintManager.paint(BufferStrategyPaintManager.java:247)
    at java.desktop/javax.swing.RepaintManager.paint(RepaintManager.java:1347)
    at java.desktop/javax.swing.JComponent._paintImmediately(JComponent.java:5260)
    at java.desktop/javax.swing.JComponent.paintImmediately(JComponent.java:5070)
    at java.desktop/javax.swing.RepaintManager$4.run(RepaintManager.java:882)
    at java.desktop/javax.swing.RepaintManager$4.run(RepaintManager.java:865)
    at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
    at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
    at java.desktop/javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:865)
    at java.desktop/javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:838)
    at java.desktop/javax.swing.RepaintManager.prePaintDirtyRegions(RepaintManager.java:787)
    at java.desktop/javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1909)
    at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:318)
    at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:792)
    at java.desktop/java.awt.EventQueue$3.run(EventQueue.java:739)
    at java.desktop/java.awt.EventQueue$3.run(EventQueue.java:733)
    at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
    at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
    at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:761)
    at com.intellij.ide.IdeEventQueue.defaultDispatchEvent(IdeEventQueue.kt:698)
    at com.intellij.ide.IdeEventQueue._dispatchEvent$lambda$12(IdeEventQueue.kt:593)
    at com.intellij.openapi.application.impl.RwLockHolder.runWithoutImplicitRead(RwLockHolder.kt:105)
    at com.intellij.ide.IdeEventQueue._dispatchEvent(IdeEventQueue.kt:593)
    at com.intellij.ide.IdeEventQueue.access$_dispatchEvent(IdeEventQueue.kt:77)
    at com.intellij.ide.IdeEventQueue$dispatchEvent$processEventRunnable$1$1$1.compute(IdeEventQueue.kt:362)
    at com.intellij.ide.IdeEventQueue$dispatchEvent$processEventRunnable$1$1$1.compute(IdeEventQueue.kt:361)
    at com.intellij.openapi.progress.impl.CoreProgressManager.computePrioritized(CoreProgressManager.java:843)
    at com.intellij.ide.IdeEventQueue$dispatchEvent$processEventRunnable$1$1.invoke(IdeEventQueue.kt:361)
    at com.intellij.ide.IdeEventQueue$dispatchEvent$processEventRunnable$1$1.invoke(IdeEventQueue.kt:356)
    at com.intellij.ide.IdeEventQueueKt.performActivity$lambda$1(IdeEventQueue.kt:1021)
    at com.intellij.openapi.application.TransactionGuardImpl.performActivity(TransactionGuardImpl.java:106)
    at com.intellij.ide.IdeEventQueueKt.performActivity(IdeEventQueue.kt:1021)
    at com.intellij.ide.IdeEventQueue.dispatchEvent$lambda$7(IdeEventQueue.kt:356)
    at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.kt:393)
    at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:207)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:128)
    at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:117)
    at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:113)
    at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:105)
    at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:92)

However, after adding style = TextStyles.Default to circumvent the exception, all of the behavior was the same but it did not throw the exception. Just adding this as some context/a data point.

With that said, it would be amazing if we could prioritize this change's integration into Jewel to unblock the usage of popup elements.

YasserDbeis commented 7 months ago

I went ahead and sent in my own custom tooltipPlacement to adjust the alignment depending on where the component on the screen is and it seemed to do the trick (avoids crossing the compose-swing boundary). So I suppose this can serve as somewhat of a workaround in the meantime; it does not completely resolve the issue if the application window's width is small enough.

rock3r commented 7 months ago

CC @obask if you end up picking this up after you're done with the current markdown stuff...

rock3r commented 3 months ago

@hamen context and info on how to use native popups:

https://www.jetbrains.com/help/kotlin-multiplatform-dev/whats-new-compose.html#desktop-experimental

tl;dr:

compose.layers.type=WINDOW

We'll have to verify nothing breaks, too

hamen commented 3 months ago

Hi all

I added System.setProperty("compose.layers.type", "WINDOW") to Jewel Standalone and ran some tests with the popups. The following video shows where we are now. A few things to notice:

Recording2024-08-13180200-ezgif com-video-to-gif-converter

I also tried to set PopupProperties(focusable = false). It solved the systembar flickering, but then the popops don't receive IME events and key presses, and the keyboard navigation is compromised in menus and dropdowns.

I can obtain the same multiple tooltip scenario also in the IDE plugin sample: image

MatkovIvan commented 3 months ago

The top systembar flickers when a popup is opened or closed due to the focus change

It sounds expected if you open a focusable window

There is a strange edge case in which if you can open multiple popups at once

Multiple Popup are allowed, if you need some limitation, it should be done in higher level components

I also tried to set PopupProperties(focusable = false). It solved the systembar flickering, but then the popops don't receive IME events and key presses, and the keyboard navigation is compromised in menus and dropdowns.

So you cannot focus non-focusable element. Sounds expected too.

hamen commented 3 months ago

Thank you for the feedback Ivan.

rock3r commented 2 months ago

@MatkovIvan after some more testing, the issue seems to be that there are cases in which TooltipArea isn't dismissing its tooltip when using the native popups flag. The issue is https://youtrack.jetbrains.com/issue/CMP-5961/TooltipArea-tooltip-doesnt-disappear-as-expected

This doesn't happen when using "fake" popups, and mostly repros when moving the mouse quickly; my working theory is that it has something to do with mouse exit events not being handled properly when there is a separate window popup for whatever reason.

hamen commented 2 months ago

I'm linking the new YouTrack ticket https://youtrack.jetbrains.com/issue/CMP-2924