Improve vertical alignment of DialogDisplayer icon (question mark, warning symbol etc.)

[eirikbakke]

In DialogDisplayer dialogs, there's typically an "information", "question mark" or "warning" type icon next to the text. It shows up top-aligned by default, which makes it look mis-aligned relative to the message text. This PR centers the icon vertically. See below:

As the dialog is constructed by JOptionPane, the tweakIconLabelVerticalAlignment method applies the fix by descending into the child components and finding the one conveniently named "OptionPane.iconLabel".

This is purely a cosmetic UI improvement.

[mbien]

@eirikbakke would you like to rebase this on delivery? I think this would be a good candidate for NB17 rc2.

[eirikbakke]

Sure, let me see if I can figure out how to do that. Do I need to create a new PR?

[eirikbakke]

OK, I think I figured it out...

[mbien]

Sure, let me see if I can figure out how to do that. Do I need to create a new PR?

@eirikbakke no its fine. I think rebasing is probably not even needed since the branches didn't diverge yet. But it doesn't hurt.

i changed target to delivery since that is the right one :)

[eirikbakke]

Oh, it's literally called "delivery". I see. Thank you! Will step aside now and not touch anything :-)

[mbien]

although this mitigates the issue, I am wondering if the actual problem is the vertical size of the window:

(manually resized, this patch not included)

[eirikbakke]

@mbien Well, if the text is center-aligned vertically, then so should the icon in any case. And there are cases where dialogs can contain a lot more text.

[mbien]

I don't have a strong opinion on this, its just that I believe that it is intended for the icon to be in the NW edge in larger dialogs.

found some screenshots with larger dialogs: https://dummyscodes.blogspot.com/2015/06/joptionpane-with-multiple-inputs.html https://www.onlinetutorialspoint.com/java/java-swing-joptionpane-html-content-example.html https://stackoverflow.com/questions/21059080/joptionpane-input-dialog-menu

If there is more content in the window, it doesn't look misaligned to me and if there is only one line it can be centered by packing the window.

In this particular case the window is simply too big, even after the icon is centered to my eyes.

[eirikbakke]

I'd be nervous about changing the size or using pack(). With pack(), you often get an awkward size when there are components with Swing HTML in them, JScrollPanes, text wrapping or such.

Vertically centering the icon seems like a simple and safe way to make things look OK in all cases.

(Even in the screenshots you shared the top-aligned icon looks mis-aligned to me; it would need more top margin. Very hard to get this right for all dialogs.)

[mbien]

NbPresenter is already using pack(). The issue seems to be that JOptionPane sets the minimum height to 90. And looking at the code there is already a workaround added for Metal LAF. https://github.com/apache/netbeans/blob/31028d282a7c7870e68ab51439be50ce691b7de5/platform/core.windows/src/org/netbeans/core/windows/services/NbPresenter.java#L511-L524

which isn't active when FlatLAF is enabled.

[mbien]

here is an in-IDE example where the center alignment doesn't look very good in my opinion:

NB16:

(this window has it's height also incorrectly calculated)

NB16, manually adjusted:

[eirikbakke]

Hmm, that "Working copy" example looks a lot taller than 90 pixels (the image is 376 pixels tall). What's going on there?

If we resize the dialog, though, the centered icon will still look better. If you really want it on top, there would need to be some top margin inserted.

[mbien]

the minimum size influences how a single line message looks. Min height of 90 for the JOptionPane is too much which makes it look like on your screenshots, while it should look like this. Since it is set to 50 on Metal and Basic LAF, I have the suspicion that someone tried to fix this before (but it reappeared with FlatLaf since the workaround is skipped there).

The last screenshots were an example to demonstrate that centering the icon doesn't look good (to me) in dialogs which more content in them.

[eirikbakke]

The https://github.com/apache/netbeans/pull/5335#issuecomment-1399358156 screenshots were an example to demonstrate that centering the icon doesn't look good (to me) in dialogs which more content in them.

If the dialog is properly sized, then it would look good, no? This means: 1) If we don't do anything about the dialog size, then vertically centering the icons improves the appearance of the dialogs. 2) If we do fix the dialog size, then the cases where vertically centering the icon looks bad go away.

In other words, vertically centering the icon is a good thing to do in any case. Resizing the dialog is a separate issue with more potential to break things.

[mbien]

If we don't do anything about the dialog size, then vertically centering the icons improves the appearance of the dialogs.

to me it marginally improves the look of a certain type of dialog: one line messages On the screenshot you provided, both before and after windows look to me as if someone placed labels on a frame without bothering to configure the layout or frame size. Both do look unfinished.

If we do fix the dialog size, then the cases where vertically centering the icon looks bad go away.

It makes the situation worse for already existing dialogs which display more content. Fixing the size there would still have the icon centered which looks non-standard.

This is of course just my opinion. However, I do believe that it is fairly common to anchor an icon to the NW corner in dialogs. I can see the same layout in the master pw window of thunderbird or firefox for example:

another example is the standard gnome keychain unlock dialog:

Since this UI pattern works for both, small dialogs and big dialogs, it probably became popular and also made it into JDK's dialog utilities. I don't see it as a bug, the bug appears to be the incorrect sizing of the content inserted right from the icon which causes this weird behavior since the window can't pack correctly. A 1 line label isn't 90 pixels tall, yet it is what its preferred size is for some reason and someone tried to fix this before.

[eirikbakke]

OK, this requires a different approach, then. No hurry to get this PR into NetBeans 17.

To continue the discussion, in your example below, I'd say that at the very least the top of the message text should align with the top of the icon (i.e. pushing the icon further down than in this screenshot). Agree/disagree?

Mockup for the purpose of the example:

[neilcsmith-net]

That Thunderbird mockup looks worse than the original in my opinion. I think I agree with @mbien here that vertically centring the icon is not the answer - fixing sizing probably is.

I don't think this is a good fit for the release given ongoing discussion. Putting this back to master with do-not-merge on pending resolution of discussion and testing across LAFs.

I think rebasing is probably not even needed since the branches didn't diverge yet.

As clarification on that point, master diverges immediately on branch. Any PR that picked up the spec version change from master would require rebasing.

[neilcsmith-net]

Waking this discussion up!

We're a few weeks from freeze for NB18 now. We should make a decision if there is anything to address here, and if so, how.

[eirikbakke]

No hurry with this PR... I have another adjustment to this patch pending which I will try the next time I do a new NetBeans build.

[eirikbakke]

I pushed a new adjustment to this PR, which adds some top margin to the icon instead of vertically centering it. The specific margin height is chosen so that the icon ends up appearing centered next to single-line messages while appearing (almost) top-aligned next to taller messages:

With @mbien's adjustments from https://github.com/apache/netbeans/pull/5625 , the minimum height of the dialog also ends up a little smaller (note: only the top screenshot above includes the latter adjustment; I'm not sure where in NetBeans I can easily trigger a taller dialog for testing), and single-line messages end up looking more vertically centered relative to the title bar and buttons than before.

(Edited:) After trying various values for the minimum height, I ended up preferring a value of 50, which is what was previously in place for MetalLookAndFeel. This adjustment is now in place for FlatLAF as well.

[mbien]

@eirikbakke is it ok to move this to NB 19?

[eirikbakke]

@mbien Yeah, no problem!

[neilcsmith-net]

@mbien @eirikbakke are we at the "is it OK to move this to NB 20" point now? :smile: Be good to decide whether this is going in rather than pushing back milestones.

Did try and leave two comments on specific lines, which now seem to have disappeared from the UI?! Be good to see if the LaF specific logic can be removed (I wonder why the comment on minimum size from the LaF is not working), and feels like the boolean flag should be configurable or removed.

[eirikbakke]

I think the "do not merge" label can be removed; this PR is ready to be committed except for a minor requested change.

[Chris2011]

So reopening it?

[eirikbakke]

Yes please, sorry--it takes me a while to get back to PRs, as switching/recompiling branches of the netbeans repo is a multi-hour operation on my Windows machine...

[neilcsmith-net]

Removed the do not merge as I realised that's something I added when moving things around with NB17.

If @eirikbakke @DevCharly and/or @mbien want to reopen and re-review, then it can be reopened. I'm just closing a few things that have bumped along from milestone to milestone without getting any further towards resolution.

[neilcsmith-net]

as switching/recompiling branches of the netbeans repo is a multi-hour operation on my Windows machine...

:open_mouth: what's going on there? It's a 7min operation on my machine. That might be something to look into?!

[mbien]

just my opinion but I am not convinced that the icon position needs to be adjusted. If you run a picture search for "error dialog", or take a look at the examples I posted somewhere above, you will see that it is fairly common that the icon is anchored in the NW corner.

NB has many layout bugs though, some dialogs might be too large and need to be adjusted so that the components don't float around, some might contain hidden components which break the layout manager (see screenshot of #6355).

(and yes building NB takes about 7,5 mins on my PC I built ~2016 -> there must be something wrong)

[eirikbakke]

It's WSL 1 on Windows, on a 2018 laptop, with the NetBeans repo on a Windows file system (WSL 1 is actually faster than WSL 2 in this case). "git checkout" takes several minutes, "git status" takes 18 seconds, and a clean build will hog up the machine long enough that I usually break for dinner...

I'll be switching to a new MacBook soon, though. Having a native UNIX terminal, no Windows Defender, and new hardware will make everything much faster.

[eirikbakke]

just my opinion but I am not convinced that the icon position needs to be adjusted.

I think this is clearly wrong:

[mbien]

^ the dialog size is wrong I agree -> https://github.com/apache/netbeans/pull/5335#issuecomment-1399324873

(the layout does its job if the size is properly adjusted)

[eirikbakke]

@mbien I think this was all discussed before, but if you prefer I can just leave this unmerged.

[mbien]

lets take a look at another dialog which displays a panel with sub-optimal layout management: -> all I did was to quickly fix the content which used grid bag layout, the dialog is unchanged. It used empty panels for padding, couldn't properly calculate its default size and the gaps were not set for the purpose of it being included into a dialog etc

netbeans/ide/projectui/src/org/netbeans/modules/project/ui/problems/BrokenReferencesAlertPanel.java

actually... let me open a PR for this :) (edit: #6575)

[mbien]

draft for one-liners: #6577