Skip to content

Laf fixes #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 24, 2023
Merged

Laf fixes #79

merged 1 commit into from
Apr 24, 2023

Conversation

tferr
Copy link
Contributor

@tferr tferr commented Oct 3, 2022

I finally looked into things that are not scaling properly under FlatLaf. I found only 3 issues, each associated with one commit here, to make it easier to cherrypick things.

Console

Commit 37a3cb2: The console dimensions were hardwired, so I tried to make it so that dimensions are set in terms of columns/lines computed from the active monospaced font. Also the menu bar contains only a single "Clear" command. I find this confusing because that clear command does nothing when the Log tab is selected. Since there is a 'Clear' command in the contextual menus of both tabs, I propose we remove it.

Color widgets

Commit 01459f8: It was using a hardwired height. Now it is scalable, with height matching that of a JTextField. imagej/imagej-ui-swing#90 extends this to color tables

Number widgets

Commit 95bc9b3: I could not find any reason why this code (discussed in #77) needs to exist:

Dimension spinnerSize = spinner.getSize();
spinnerSize.width = 50;
spinner.setPreferredSize(spinnerSize);

A width of 50 seems arbitrary, and everything seems to work fine if we just remove that code!? @imagejan, what do you think?

FileList widgets

SwingFileListWidget was the other place where I found hardwired dimensions, namely here:

scrollPane.setPreferredSize(new Dimension(350, 100));

(@imagejan, without specifying dimensions the JscrollPane does collapse, but perhaps something like JList#setPrototypeCellValue("prototype value"); could be used to set things in a 'scalable' manner?

(update: edited for clarity)

@ctrueden
Copy link
Member

ctrueden commented Oct 3, 2022

@tferr This is great!

Two quick nitpicks: 1) this work needs to be rebased over the latest master branch (there are conflicts); 2) I am not a fan of removing menu bars/items just because right-click context menus exist. It is considered bad UI design (at least historically by Apple HCI documents) to have functionality that is only accessible in context menus. See e.g. web browsers, which have Cut/Copy/Paste et al. in both context menu and an Edit menu.

About the spinner size 50: it looks like that got added with 3a91c1a, when @imagejan added support for floating-point spinner values. I'm guessing it was done experimentally to make the floating point values appear in a better way than the default? So before removing it, we should test spinners with floating point values to see how they behave with these patches in place.

@tferr
Copy link
Contributor Author

tferr commented Oct 3, 2022

About the spinner size 50: it looks like that got added with 3a91c1a, when @imagejan added support for floating-point spinner values.

AhAh, makes sense! But we probably need to make it that the '50' scales up proportionally to the font in use.

On the menubar:
Agree, but then we probably need to make it work with the Log pane. It is super weird that it does nothing when the Log tab is selected.

Hehe, we know things are looking sharp when all it is left are just these almost-futilities. 😉

Console: Define height relative to font size currently in use. This ensures it adopts the same height as other widgets in hiDPI screens
Console: Compute scroll Pane dimensions from font metrics. I.e., rather than using a hardwired dimension that does not scale well on HiDPI, use line/columns instead.

SwingColorWidget: Do not use harwired height.
@tferr
Copy link
Contributor Author

tferr commented Oct 3, 2022

Removed the controversial JSpinner and Console menu bar changes. Should be ready to merge now. @imagejan: I would still ask you to look at the constant widths/heights in the JSpinner and the FileListWidget.

@tferr
Copy link
Contributor Author

tferr commented Apr 20, 2023

@ctrueden, can this be merged? I need to mentally move on, and it is hard to test the new release when there is stuff like this still pending

@ctrueden ctrueden merged commit 184dc98 into scijava:master Apr 24, 2023
@ctrueden
Copy link
Member

@tferr Thanks for the ping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants