Skip to content
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

[NETBEANS-5697] Various LAF/HiDPI improvements on Windows #2965

Merged
merged 3 commits into from
Jul 17, 2021

Conversation

eirikbakke
Copy link
Contributor

@eirikbakke eirikbakke commented May 19, 2021

This PR includes a few smaller improvements that were made to the Windows LAF while working on modernized tab components. Several of these relate to NetBeans' appearance when HiDPI scaling is active. Summary:

  • Apply previous border tweaks for HiDPI screens in a few more places, notably in tooltips and the quick search text field, and in the editor toolbar. Improve JSpinner borders.
  • Make the background color of tooltips consistent with that of other Windows 10 apps.
  • Add some pixels of left margin to the editor toolbar.
  • More improvements added in another commit, see the second pair of screenshots further below.

The screenshots below are from before the fixes were applied.

Smaller fixes

  • Also apply the workarounds from NETBEANS-3592 to JCheckboxMenuItem and friends. This fixes the following problem, where menu item checkmarks can become tiny or huge in certain situations (due to HiDPI-related bugs in Swing's Windows LAF, see https://bugs.openjdk.java.net/browse/JDK-8211715 ):

210517 Menu Checkbox Bug

Details:
* Apply previous border tweaks for HiDPI screens in a few more places, notably
  in tooltips and the quick search text field, and in the editor toolbar.
  Improve JSpinner borders.
* Make the background color of tooltips consistent with that of other Windows 10
  apps.
* Add some pixels of left margin to the editor toolbar.
@DevCharly
Copy link
Member

You could also remove bottom (and top?) separator of the main toolbar:

image

It is created here:

if ("Windows".equals(UIManager.getLookAndFeel().getID())) { //NOI18N
if( isXPTheme() ) {
if( isWindows8() ) {
toolbarPanel.setBorder( BorderFactory.createEmptyBorder() );
} else {
//Set up custom borders for XP
toolbarPanel.setBorder(BorderFactory.createCompoundBorder(
upperBorder,
BorderFactory.createCompoundBorder(
BorderFactory.createMatteBorder(0, 0, 1, 0,
fetchColor("controlShadow", Color.DARK_GRAY)),
BorderFactory.createMatteBorder(0, 0, 1, 0, mid))
)); //NOI18N
}

Looking at that code it actually seems to be a bug that the toolbar has top and bottom borders on Windows 10, because it does not have borders on Windows 8...

@eirikbakke
Copy link
Contributor Author

Good idea, will try it... (in fact, I think I already removed that border in my own NetBeans Platform application)

@eirikbakke eirikbakke changed the title [NETBEANS-5697] Various smaller LAF/HiDPI improvements in the Windows LAF [NETBEANS-5697] Various LAF/HiDPI improvements on Windows May 31, 2021
@eirikbakke
Copy link
Contributor Author

I added another commit with more improvements around the toolbar and menu bar area, including the proposed border change. Here is the latest look, with the newest changes annotated, at 150% HiDPI scaling:

More updates annotated

Here is the same window at 100% HiDPI scaling, without the annotations:

More updates

@DevCharly
Copy link
Member

that looks much better 👍

I think you can remove method ToolbarConfiguration.isWindows8(). Seems to be unused now.

@rasa-app
Copy link
Contributor

rasa-app commented Jun 2, 2021

IMHO, removing bottom border of toolbar completely, causes a kind of irregularity because toolbar icons messing with the tab area, I think it's better to remove just shadows and keep a simple border or just like FlatLaf, make the toolbar background a little darker or lighter and make a distinguish between toolbar and other parts.

@eirikbakke
Copy link
Contributor Author

Here's a mockup with a line under the toolbar, without adding spacing:

line

I don't think the line works very well here. It makes things look too cramped--alternatively it will require more margin to be added back around the toolbar, which consumes precious vertical space. Alternatively, different background colors don't look consistent; it's kind of like using too many fonts in a text document. Plus, whenever there's a change in the background color you need additional margin.

(On the topic of "change in background color", I also experimented with making the menu bar grey, or the toolbar white, to avoid the change between the menu bar and the toolbar. But the menu bar is always white on Windows 10, and a white toolbar looked odd.)

I do understand what you mean by "irregularity" with the space next to the tabs. But most of the time this will be filled up by tabs, so the typical appearance of the IDE won't have this problem.

Details:
* Make the text in the HeapView toolbar component more readable by adding an
  outline around the text (benefits all LAFs). Avoid some painting bugs around
  the edges on 150% HiDPI scaling on Windows. Adjust colors on Windows.
* Make the build configuration dropdown in the toolbar use a consistent
  background color. Mostly relevant on the Windows LAF.
* On the Windows LAF, get rid of an unnecessary border under the main
  application toolbar. But add a border separating the white menu bar part of
  the window with the grey toolbar/background part of the window (applies
  whether a toolbar is actually shown or not). Adjust spacing.
* On the Windows LAF, modernize the look of the "grip" (drag and drop
  affordance) portion of the main toolbar.
* On the Windows LAF, remove the borders around the "Quick Search" component
  in the menu bar, and make its background flush with the rest of the menu bar.
* On the Windows LAF, make the window system borders slightly lighter.
@eirikbakke
Copy link
Contributor Author

I did a force push just to remove the now-unused ToolbarConfiguration.isWindows8() method, as earlier suggested. Other than that I think the discussion items were resolved here (judging by rasa-app's thumbs-up emoji :-).

I have used this patch in my working IDE and platform app for two months, so I think it should be safe to merge (once Travis passes again)...

@ebarboni ebarboni added this to the 12.5 milestone Jul 16, 2021
@matthiasblaesing
Copy link
Contributor

This looks like a sane change, has positive reviews, does not introduce a revolution and seems to pretty much contained and is tested. I suggest to get this in now.

@eirikbakke minor comment for the future: I would add the issue marker to the commit messages themselves - its the individual commits, that are shown when running git blame (or the using the corresponding netbeans action) and it would make it easier to find the issue that motivated a change.

@geertjanw geertjanw merged commit e95ee7b into apache:master Jul 17, 2021
@eirikbakke
Copy link
Contributor Author

Thanks! @matthiasblaesing Sure, I will reference the issue in each individual commit message in the future.

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.

6 participants