-
Notifications
You must be signed in to change notification settings - Fork 60
WIP: Add OS settings-aware dark theming #124
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
base: pyside6
Are you sure you want to change the base?
WIP: Add OS settings-aware dark theming #124
Conversation
|
Sadly the folder buttons looked decent on linux but awful on windows, back to the drawing board I suppose. |
…n for bools, increase contrast between red and green for values
91554a0 to
9d1c3c3
Compare
5c3df31 to
040c677
Compare
dihm
left a comment
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.
Few changes, and I'd like to hammer out the highlight situation a little more.
runmanager/__main__.py
Outdated
| else: | ||
| print(f"Unable to get color palette from os, got {check_if_light_or_dark()}") |
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.
This branch is no longer needed.
runmanager/__main__.py
Outdated
| if QT_ENV.lower() == 'pyqt5': | ||
| return "light" | ||
| style_hints = QtGui.QGuiApplication.styleHints() | ||
| if style_hints.colorScheme() == Qt.ColorScheme.Dark: |
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.
A minor style thing, but could we change this call to QtCore.Qt.ColorScheme.Dark? Then we don't need another distinct import for just this line and the call matches usage in other places.
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.
Oh yes that's the more pythonic way
Edit: I realize I was thinking about something else nevermind that but the suggestion is still good
runmanager/__main__.py
Outdated
| ) | ||
| from labscript_utils.qtwidgets.outputbox import OutputBox | ||
| import qtutils.icons | ||
| from qtutils.qt import QT_ENV |
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.
Lets merge this import into the import on line 49.
runmanager/__main__.py
Outdated
| p.color(QtGui.QPalette.Highlight) | ||
| ) |
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'm not sure I'm sold on this change. If we are going to replace the custom highlight, we should drop it everywhere. But I'm honestly slightly more inclined to keep the custom highlight color usage here. Let's workshop it some more...
Does not handle custom colors.
Reverts using palette highlight
background brush is asked for (ie after theme change).
… between components

This adds runmanager gui functionality to ensure PySide6 uses the correct palette if the user sets their OS theme to be light or dark.
Still to do is to ensure the highlight is sensible (Since PySide6 interacts oddly with the OS highlight and accents) and also to ensure the buttons in the top right have the correct palette applied.
