Add dark mode checker depending on Qt version#3507
Add dark mode checker depending on Qt version#3507ann0see merged 1 commit intojamulussoftware:mainfrom
Conversation
|
macOS legacy and non legacy seem to work as expected. |
|
Windows seems to work now too. |
ann0see
force-pushed
the
bug/windowsDarkMode
branch
5 times, most recently
from
b53079e to
f70841f
Compare
|
Light mode might have a slightly wrong color for the line surrounding the mixer board:
But I suppose this is non blocking. Otherwise, I think this is good as is. |
|
I'd have a change log entry - helps search engines help any looking up the issue find "use the latest version" should fix it. |
| const QPalette defaultPalette; | ||
| if ( defaultPalette.color ( QPalette::WindowText ).lightness() > defaultPalette.color ( QPalette::Window ).lightness() ) | ||
| #else | ||
| if ( palette().color ( QPalette::Window ) == QColor::fromRgbF ( 0.196078, 0.196078, 0.196078, 1 ) ) |
There was a problem hiding this comment.
Can this not also compare against a threshold? (Somewhere halfway between the two tones in use.)
There was a problem hiding this comment.
I would then rather use the code above. But this worked for macOS and probably Linux (?).
There was a problem hiding this comment.
Do non-Q_OS_WINDOWS platforms have QPalette::Window? If they do, then we may as well use the same for all platforms if it works -- I'd rather have as few different ways of doing the same thing as possible.
We're looking to do "is colour scheme dark" -- if that means "is window text (EDIT: not darker) ligher than window background" and Qt supports getting the window text and window background colours in a way that supports the lightness() method, I can't think why we shouldn't do it the same on all.
(I really want to get rid of "does the window colour equal some random value that was present on someone's screen" -- most window managers allow a variety of colour schemes and this is as likely to be wrong for anyone customising the set up as it is right.)
There was a problem hiding this comment.
Yes, the link proposes the lightness comparison - so I assume that this is the Qt proposed way to go and it should work on all supported OS. I wanted to change as little behavior as possible, but yes - the new approach seems cleaner.
ann0see
force-pushed
the
bug/windowsDarkMode
branch
from
f70841f to
da2c04f
Compare
| if ( defaultPalette.color ( QPalette::WindowText ).lightness() > defaultPalette.color ( QPalette::Window ).lightness() ) | ||
| #endif | ||
| { | ||
| // Dark mode on macOS/Linux needs a light color |
There was a problem hiding this comment.
Of course, this is now "Dark mode on QT before 6.5.0" rather than OS-specific.
There was a problem hiding this comment.
They are not magic at all. rgb(x,y,z) or #ffffff are clearly defined color codes. In webdev I usually also don't use keywords as e.g. the hex value is more precise. Also IDEs usually show the color directly or can be configured to do so:
There was a problem hiding this comment.
The point is, someone reading the code has to work out in their head what colour they are. I'd rather use "lightgray" than "gainsboro" for readability to be honest - I had to look up what the colour code was to see what (220, 220, 220) looked like and find the css name -- with "lightgray" I'd have a pretty good idea immediately.
pljones
left a comment
pljones
left a comment
There was a problem hiding this comment.
Not fussed about my last comments.
ann0see
force-pushed
the
bug/windowsDarkMode
branch
from
da2c04f to
462b6f0
Compare
2 participants
Short description of changes
Solves darkmode color issue in mixer dialog:
Connected:
Disconnected:
CHANGELOG: SKIP
Context: Fixes an issue?
Fixes: #3395
Does this change need documentation? What needs to be documented and how?
No
Status of this Pull Request
Ready for review. Working
What is missing until this pull request can be merged?
Review
Checklist