[JEWEL-1205] Fix Menu Items Looking Too Tall in Presentation Mode

[DanielSouzaBertoldi]

Context

Our menu items are looking a little too big when opening a ✨ Jewel ✨ menu in presentation mode. Curiously, sub menu items don't have the same issue. Check the evidences section below to see how menu items were being rendered.

Now, the fix is really, really, REALLY simple. HOWEVER, it took me a really long time to finally crack this case open and I'm kinda mad because of it. If you wish to read just one more novel of mine (I promise this won't be the last), check the collapsible section below (and don't forget to bring a glass of whatever is your favorite drink to drink while code reviewing this --I promise I won't judge you for this, even if you decide on drinking sparkling water you goofball)

The curve ball tale 🕵🏻

Alright first of all, one of the things that really stood out to me was that MenuItemBase had a .defaultMinSize(minHeight = itemMetrics.minHeight) while MenuSubmenuItem did not.

Interesting.

I thought. I commented out this modifier from MenuItemBase, ran the IDE project, tested with presentation mode both on and off, and the issue was fixed!

Nice.

I celebrated. This looked like the world famous "quick win" that developers are so eager to have around. But then, it hit me: if I were to remove this modifier, what would happen to itemMetrics.minHeight? Would we need to just delete it for good? No, that doesn't seem to be the right thing to do.

Then, the next step of this quest started: trying to find out where the code of Intellij's Menu was and how they were applying the height for each menu item. This by itself took a long time, since there are so many intertwined classes and whatnot. Anyways, this is the boring part so I'll skip ahead.

I finally struck gold when I came across two classes: BegMenuItemUI.java and IdeaMenuUI.java. The former is the class that is used to render menu items in an ActionPopupMenu, exactly what I needed. The latter is basically a helper class.

Inside the getPreferredSize() function from BegMenuItemUI, there was this call at the end:

IdeaMenuUI.patchPreferredSize(comp, rect.getSize());

Here's the code IdeaMenuUI.patchPreferredSize():

public static @NotNull Dimension patchPreferredSize(Component c, Dimension preferredSize) {
  if (ExperimentalUI.isNewUI() && !IdeaPopupMenuUI.isMenuBarItem(c)) {
    JBInsets outerInsets = IdeaPopupMenuUI.isPartOfPopupMenu(c)
                           ? JBUI.CurrentTheme.PopupMenu.Selection.outerInsets()
                           : JBUI.CurrentTheme.Menu.Selection.outerInsets();
    return new Dimension(preferredSize.width, JBUI.CurrentTheme.List.rowHeight() + outerInsets.height());
  }

  return preferredSize;
}

No way.

I muttered. This code is basically proving to us that, if the user is using the new ui, then the height for the menu item is just:

JBUI.CurrentTheme.List.rowHeight() + outerInsets.height()

Which we were already doing. Of couse, we don't calculate both and set the result as the total height of a menu item, but we do apply the outerInsets + the min height for the menu item given the value in rowHeight().

That means that our code was correct after all. So why was this behavior happening? That didn't make any sense. We could "fix it" by just removing the minHeight modifier, but why?

After adding some logs (basically just adding a print statement inside onSizeChanged {}), I could guarantee that the min height we were setting was way smaller than the items actual size:

[MenuItemBase] minHeight: 24.0.dp, actual height: 48  // presentation mode disabled
[MenuItemBase] minHeight: 42.0.dp, actual height: 147 // presentation mode enabled

Nothing was making sense anymore. The min height is too small. It's not like we are forcing the item to have the same height as minHeight.

I went over IntelliJ's code over and over, tried to add the same calculations on Jewel, the bug kept happening. I finally gave up since I ran out of ideas and asked the OG Gemini about it and....... I couldn't believe the answer.

It was a double scaling error all along. At first I was skeptical but once I tried using the unscaled value of rowHeight(), the bug disappeared. No need to delete the minHeight modifier. I was flabbergasted to say the least.

It then proceeded to explain to me why the double scaling:

BUI.CurrentTheme.List.rowHeight() already returns an scaled value. So for instance, it can be scaled by 1.0f for "normal" mode and 1.75f for presentation mode. When we call .dp from Compose, it also scales the value by a given factor (taking into account your screen display/pixel density).

So, the base height for rows is 24. In presentation mode it gets scaled up to 42 (24x1.75). Then, Compose does its thing and scales up by 3.5f in my case (2.0f - retina display * 1.75 - from IJ's). 42 * 3.5f = 147. Bam. That's exactly the actual height of a menu item when presentation mode was enabled.

The reason minHeight caused this is because we were basically forcing Compose to apply this scaling factor on 42 instead of the unscaled value of 24.

I never would've thought that this could be a scaling problem. Well, you live and you learn right? At least next time I'll also be suspicious of scaling issues.

Thanks for reading, leave your review on a piece of paper and mail it to me 😎

Changes

• Use the unscaled value of JBUI.CurrentTheme.List.rowHeight() in org.jetbrains.jewel.bridge.theme.IntUiBridgeMenu
• Also took this opportunity to refactor a bit of menu item/sub menu item code. They had quite a bit of duplicated code. Yep, this means I added yet another layer of Composable
  • Previously sub menu items didn't have this scaling problem because we weren't setting the minHeight. This was fixed with this refactor
• Created a handy unscaledDp function in BridgeUtils

Evidences

	Screen Recording
Before	Screen.Recording.2026-02-12.at.09.16.47.mov
After	Screen.Recording.2026-02-12.at.09.12.59.mov

Release notes

Bug fixes

• Menu items now keep their size when rendered in Presentation Mode

[DanielSouzaBertoldi]

Had to set a fixed height so that our Popup can be rendered in its entirety instead of smushed

[daaria-s]

oh I remember this bug, nice research 😁

[daaria-s]

just curious, is it true that there might be more places where we need to use it but we don't know about them?

[DanielSouzaBertoldi]

Not that I was able to find yet 😮

We do fetch the rowHeight() value in our readFromLaF() function over in BridgeGlobalMetrics but we are already unscaling it there

[rock3r]

Hmm we do unscaling for Insets in this file *see Insets.toPaddingValues()), so if the Ints we get are indeed scaled, we need to check. I don't know if it's a potential issue, but it seems like retrieveIntAsDp*()s do need a closer look to validate whether they're fetching the right values or not.

Otherwise you're saying "it's fine to use retrieveIntAsDp*s everywhere except in this one case where it would do a double scale", which seems a bit odd in principle. Effectively, JBUI.CurrentTheme.List.rowHeight() does the same thing we do, calling JBUI.getInt() which calls UIManager.get() — retrieveIntAsDp*() also does the same, then calling .dp on the value (which is where the double scaling may happen).

[DanielSouzaBertoldi]

Hmmm come to think of it, maybe it's interesting to create a Detekt rule for this? I can't think of any scenarios where using a scaled dp value would make sense 🤔

[rock3r]

New detekt rules get my blanket "hell yea"

[rock3r]

@DanielSouzaBertoldi can we haz that detekt rule? :)

[rock3r]

I think we've lost the disabledAppearance here, unless I'm missing something

[rock3r]

Hmm we do unscaling for Insets in this file *see Insets.toPaddingValues()), so if the Ints we get are indeed scaled, we need to check. I don't know if it's a potential issue, but it seems like retrieveIntAsDp*()s do need a closer look to validate whether they're fetching the right values or not.

Otherwise you're saying "it's fine to use retrieveIntAsDp*s everywhere except in this one case where it would do a double scale", which seems a bit odd in principle. Effectively, JBUI.CurrentTheme.List.rowHeight() does the same thing we do, calling JBUI.getInt() which calls UIManager.get() — retrieveIntAsDp*() also does the same, then calling .dp on the value (which is where the double scaling may happen).

[rock3r]

Wouldn't this subtly change MenuSubmenuItem behavior? Before the refactor it reset selected = false on enabled changes, whereas the shared helper now preserves selected. So if a submenu is currently open / hover-selected and then becomes disabled, I think it would stay selected (and keep the submenu open) until something else dismisses it. Is that intentional?