-
Notifications
You must be signed in to change notification settings - Fork 41
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
Fix white glitches occurring while resizing #304
Conversation
…control" property to each included theme
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.
Thank you for fixing this. I wasn’t aware that the control
property could be used to change the native window background. See above for a recommendation on how to avoid duplicating the property in every theme.
@@ -83,6 +83,7 @@ Theme.highContrast = true | |||
%widgetFillDefault = #00D1E5 | |||
|
|||
####Controls#### | |||
%control = #FFFFFF |
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.
I don’t like that this duplicates the background
property in every theme. I’d suggest to instead set the property in ThemeDefaultsInitTask based on the value of background
.
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.
You are right — this is a preferable solution indeed. But, in order to avoid background
property duplication, what about creating a global reference to background
property in /darklaf/core/src/main/resources/com/github/weisj/darklaf/globals.properties?
Or, just for clarity's sake, a better approach would be to create ThemeDefaultsInitTask#initControl method for initialising the control
property?
Thanks in advance.
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.
That’s actually the better solution :). Maybe add a comment explaining why it’s needed as well.
I apologise for messing up things with the repository so, unfortunately, I needed to re-fork your repo, but I have some good news: I have also committed the requested changes to the new fork. In my opinion it's ok to create a new PR and close the PR we are writing on. What I am supposed to do, in your opinion though? |
You can just create a new PR thanks a lot :) |
Done! |
Fix white glitches occurring while resizing JFrame.
Proposed fix:
– add the missing "control" property to each included theme.
GIF showing the glitch:

Result of the proposed fix:

Test environment:
OS: macOS 12.1 (21C52)
Architecture: Intel x64
JVM Vendor: JetBrains s.r.o
Java Runtime Version: 11.0.12