[JEWEL-1013,JEWEL-1016] Using compose drop shadow APIs for popups #3253
Conversation
faogustavo
requested review from
rock3r,
jakub-senohrabek,
daaria-s and
nebojsa-vuksic
| Shadow( | ||
| radius = style.metrics.shadowSize, | ||
| color = colors.shadow, | ||
| offset = DpOffset(0.dp, style.metrics.shadowSize / 2), |
There was a problem hiding this comment.
On Swing, it was using the same size from the shadow as the offset, to prevent shadow on top.
On this Compose call, only half of the value is needed to reproduce the desired UI.
| full size | base-line | half size |
|---|---|---|
 |
 |
 |
| .setCancelOnClickOutside(currentProperties.dismissOnClickOutside) | ||
| .setCancelOnWindowDeactivation(currentProperties.dismissOnClickOutside) | ||
| .setLocateWithinScreenBounds(false) | ||
| .setShowShadow(false) |
There was a problem hiding this comment.
I don't think this is the right approach — we should use the same shadow that other popups do (which is drawn by the OS, now Swing).
Us drawing our own shadow is the right choice only when we're not using native popups. It's a fallback.
If the native popups don't show shadows, I think the issue may be in CMP clipping it out (that famous custom property I keep asking you about and that I do not think should be at all necessary for native popups!)
There was a problem hiding this comment.
Updated to make sure we only use it when the native pop-up flag is disabled.
| Swing | Compose |
|---|---|
 |
 |
There was a problem hiding this comment.
Thanks :) Does the native popup shadow work now, or do we still need a separate bug for that?
There was a problem hiding this comment.
It should be good. I'll boot the Windows/Linux VMs to ensure it works on other platforms as well. But all CMP and Native shadows should have the same appearance
There was a problem hiding this comment.
Looks like it's not working because we need to use the 'Undecorated' window type to hide the navigation bar. I'll check if there are other ways to achieve similar results.
The "Window.shadow" property only applies to MacOS :/
...-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/popup/JDialogRenderer.kt
Outdated
Show resolved
Hide resolved
faogustavo
force-pushed
the
gv/JEWEL-1013
branch
from
4c1f43f to
a429927
Compare
Evidences