-
Notifications
You must be signed in to change notification settings - Fork 853
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
FlatLaf: Enable Window decoration option for Linux #6391
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first reaction: nice. Thank you for working on this. The System.out
calls have to go though.
A first quick test was successful and even switching at runtime seems to be working. Review from the right people is already requested and I hope that they agree, that this the right direction.
platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/Installer.java
Outdated
Show resolved
Hide resolved
platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/Installer.java
Outdated
Show resolved
Hide resolved
platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/Installer.java
Outdated
Show resolved
Hide resolved
2ea6891
to
cacb91c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this on Windows (with some modifications) because I don't have a NB dev env on Linux. Works fine 👍
Anyway there are two questions:
- Option "FlatLaf Window decorations" is selected by default. Also on Linux. This means after merging this PR, NetBeans (and NB platform apps) use this by default on Linux. Is this wanted? Or should it be disabled on Linux by default?
- Should this also enabled for
JDialog
s on Linux? Currently it is only enabled forJFrame
s.
We also should take care on NB platform apps that already use FlatLaf window decorations on Linux. I know that @rkeen-siemens has one in his company. @rkeen-siemens would this PR cause problems for your app?
platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/Installer.java
Outdated
Show resolved
Hide resolved
platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/Installer.java
Outdated
Show resolved
Hide resolved
.../o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanelController.java
Outdated
Show resolved
Hide resolved
@DevCharly It should be disabled on Linux by default. Visual Studio Code disable it too: microsoft/vscode#65608. How can I change this? |
In class netbeans/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafPrefs.java Line 63 in cacb91c
and here netbeans/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafPrefs.java Line 67 in cacb91c
I would suggest to add a field to that class and use it instead of passing private static final boolean DEF_USE_WINDOW_DECORATIONS = SystemInfo.isWindows_10_orLater;
static boolean isUseWindowDecorations() {
return prefs.getBoolean(USE_WINDOW_DECORATIONS, DEF_USE_WINDOW_DECORATIONS);
}
static void setUseWindowDecorations(boolean value) {
putBoolean(USE_WINDOW_DECORATIONS, value, DEF_USE_WINDOW_DECORATIONS);
} |
cacb91c
to
dd18951
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
great! @demitriusbelai can you squash everything and force push into the PR branch? The resulting commit will be merged as-is. Since PRs are linked from the release notes, I think it would be nice here to describe in the PR text where to find the option (tools->options->appearance->flatlaf->window decorations). |
@DevCharly the other thing to check with @rkeen-siemens is that the quick patch (#6294) I had to make to his dialog fix hasn't caused the original issue to reappear. See also the original issue report at #5987 to see why it might be of concern here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I understand it correctly that the code in this PR will only run if the new setting is explicitly turned on? If so, it seems low-risk.
.../o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanelController.java
Outdated
Show resolved
Hide resolved
platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafPrefs.java
Show resolved
Hide resolved
platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/Installer.java
Outdated
Show resolved
Hide resolved
dd18951
to
8962e72
Compare
@mbien Sorry for the delay. I did squash and rebase. |
8962e72
to
73f9be6
Compare
Guys! I remove I chaged PopupUtil and It worked: --- a/ide/editor/src/org/netbeans/modules/editor/codegen/PopupUtil.java
+++ b/ide/editor/src/org/netbeans/modules/editor/codegen/PopupUtil.java
@@@ -38,6 -38,6 +38,7 @@@ import javax.swing.AbstractAction
import javax.swing.Action;
import javax.swing.JComponent;
import javax.swing.JDialog;
++import javax.swing.JRootPane;
import javax.swing.KeyStroke;
import javax.swing.SwingUtilities;
import org.openide.windows.WindowManager;
@@@ -81,6 -81,6 +82,7 @@@ public final class PopupUtil
popupWindow.setUndecorated(undecorated);
popupWindow.getRootPane().getInputMap( JComponent.WHEN_ANCESTOR_OF_FOCUSED_COMPONENT ).put( ESC_KEY_STROKE, CLOSE_KEY );
popupWindow.getRootPane().getActionMap().put( CLOSE_KEY, CLOSE_ACTION );
++ popupWindow.getRootPane().setWindowDecorationStyle(JRootPane.NONE);
//set a11y
String a11yName = content.getAccessibleContext().getAccessibleName(); However, I looked for setUndecorated in source code and found a lot of. So it is better keep JDialog untouched. |
how does this behave on other systems, do the window decoration changes take immediate effect when the option is applied? we should probably trigger the restart notification bubble (this might need a isLinux() check if the restart is unique to linux): diff --git a/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanel.java b/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanel.java
index f95768a..436c6d8 100644
--- a/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanel.java
+++ b/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanel.java
@@ -53,6 +53,7 @@
private static final Color DEFAULT = new Color(0, true);
private static final Color currentAccentColor = FlatLafPrefs.getAccentColor();
+ private static final boolean currentUseWindowDecorations = FlatLafPrefs.isUseWindowDecorations();
private static final RequestProcessor RP = new RequestProcessor(FlatLafOptionsPanel.class);
@@ -341,7 +342,7 @@
FlatLafPrefs.setUnderlineMenuSelection(underlineMenuSelectionCheckBox.isSelected());
FlatLafPrefs.setAlwaysShowMnemonics(alwaysShowMnemonicsCheckBox.isSelected());
- if (!Objects.equals(accentColor, currentAccentColor)) {
+ if (!Objects.equals(accentColor, currentAccentColor) || useWindowDecorationsCheckBox.isSelected() != currentUseWindowDecorations) {
askForRestart();
}
return false;
|
On Windows it do not need restart.
My fault. NetBeans was already asking for restart but now I know it is a bug when AccentColor is set DEFAULT. I will try to explain. When AccentColor is set DEFAULT, it is saved as 'null'. So NetBeans launch FlatLatOptionsPanel:
When store():
It can be fixed: diff --git a/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanel.java b/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanel.java
index 9c47c6c60e..da5eaca8d8 100644
--- a/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanel.java
+++ b/platform/o.n.swing.laf.flatlaf/src/org/netbeans/swing/laf/flatlaf/FlatLafOptionsPanel.java
@@ -53,7 +53,7 @@ import org.openide.util.RequestProcessor;
public class FlatLafOptionsPanel extends javax.swing.JPanel {
private static final Color DEFAULT = new Color(0, true);
- private static final Color currentAccentColor = FlatLafPrefs.getAccentColor();
+ private static final Color currentAccentColor = FlatLafPrefs.getAccentColor() != null ? FlatLafPrefs.getAccentColor() : DEFAULT;
private static final RequestProcessor RP = new RequestProcessor(FlatLafOptionsPanel.class); As soon as possible I will fix the PR. |
73f9be6
to
cc26875
Compare
Done! I put the fix for currentAccentColor together. Let me know if I need to make a new PR for that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good to me + tested on linux
thanks for your contribution - looking forward to using it in the next release
if everyone is ok with this I will press merge this weekend. Lets get some PRs in before feature freeze mid-October. |
no complaints -> merging |
Hi! This enable window decoration option for system that not support Native Window Decorations (Linux). This can be enable in Tools > Options > Appearance > FlatLaf > Window decorations.
See: https://www.formdev.com/flatlaf/window-decorations/#enable_disable_linux