Preferences: choose dark or light icons depending on OS theme#3484
Conversation
|
In conjunction with #3464 the preferences should now be usable with all sorts of themes. Menubar theming per skin is on its way... :) |
|
@Holzhaus Can you check how that looks like with i3wm? |
|
@Be-ing Did you try this? Any comments on the code? |
ronso0
force-pushed
the
pref-icon-color
branch
2 times, most recently
from
aabef00 to
0355444
Compare
Sorry, I switched to pop-shell recently. |
Holzhaus
left a comment
Holzhaus
left a comment
There was a problem hiding this comment.
Thanks. Works fine on pop-shell/GNOME.
Looks like some unrelated mass reformattings slipped in. Please either remove them or fix the missing p prefix for all pointer types.
| // does not include the offset, so it is moved from the last position it had. | ||
| #else /* __WINDOWS__ */ | ||
| // On linux, when the window is opened for the first time by the window manager, | ||
| // QT does not have information about the frame size so the offset is zero. |
There was a problem hiding this comment.
| // QT does not have information about the frame size so the offset is zero. | |
| // Qt does not have information about the frame size so the offset is zero. |
okay, I'll fix that after adding the QDir suggestion. |
ronso0
force-pushed
the
pref-icon-color
branch
from
0355444 to
f9742c5
Compare
|
I fixed the pointer prefixes and added the QDir suggestion. |
ronso0
force-pushed
the
pref-icon-color
branch
from
f9742c5 to
6250bc5
Compare
|
QDir issues fixed, ready again. |
uklotzde
left a comment
uklotzde
left a comment
There was a problem hiding this comment.
Just some minor comments.
There are also many opportunities for wrapping string literals into QStringLiteral(). Qt legacy burden, unfortunately.
|
This is a already huge PR. No need for more refactoring here. I agree to defer those non-mandatory changes until later. |
|
so all LGTY? |
|
will fix the conflicts now |
Done, CI all green |
|
oha! I tried merging 2.3 into main locally and that is fun already right now. Some not so trivial conflicts in dlgpreferences.cpp After this PR merged some addPageWidget() optimizations will clash.. |
|
reset to Draft. there's one !fixup commit I missed to squash. I'll do this when you consider this ready to be merged. |
@Holzhaus Final LGTM? |
|
I didn't review this again because it's still marked as draft. Should I review? |
|
Yes please. Per commit this is easy to review, starting with what you already reviewed. |
|
Holzhaus
left a comment
Holzhaus
left a comment
There was a problem hiding this comment.
Code look good, thank you. When the missing icon error on start is fixed, we can merge IMHO.
|
fixed. |
Great, thanks for being quick! Seems like a created a small mess with those forgotten !fixup commits and merging 2.3 later on. |
dark foreground text color > dark icons, light text > light icons
# Conflicts: # res/mixxx.qrc # src/preferences/dialog/dlgpreferencepage.h # src/preferences/dialog/dlgpreferences.cpp
ronso0
force-pushed
the
pref-icon-color
branch
from
ac0f7ac to
59fa313
Compare
|
Puuh, done. |
|
@Holzhaus |
|
Thanks. merging 2.3 to main now...huu fun |
3 participants
... because previously dark icons were hard to recognise with dark themes.
This is a rather simple approach that checks whether the palette's primary text color is dim or not and chooses the respective icon set. The two sets of dark and light icons include all icons in the side bar and those in the controller preset drop-down menu.
I removed the gradients and use plain color fill colors now. Icon folders are much smaller now because of 'optimized SVGs'.
To allow adding the icon set switch I refactored DlgPreferences to add the individual Pref page widgets and tree icons in a much more compact way in
addPageWidget()insetad of separate slots.