-
Notifications
You must be signed in to change notification settings - Fork 1
Qt-material theming #35
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: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
…Ubotix/rov-26 into qt-material-WATERMELON-THEME
ShannonGriswold
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.
There are some warnings in the console when launched that I believe are caused by these changes. Could you please find what is causing these warnings and handle it, or determine that they are not caused by this branch.
[run_pilot-1] WARNING:root:The style 'Fusion' does not exist.
[run_photosphere-8] Serving HTTP on 0.0.0.0 port 8000 (http://0.0.0.0:8000/) ...
[run_pilot-1] MESA: error: ZINK: failed to choose pdev
[run_pilot-1] libEGL warning: egl: failed to create dri2 screen
[run_operator-4] WARNING:root:The style 'Fusion' does not exist.
[run_operator-4] MESA: error: ZINK: failed to choose pdev
[run_operator-4] libEGL warning: egl: failed to create dri2 screen
ShannonGriswold
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.
I have not had the chance to test on a mac or the competition laptop to see if the other two warnings occur on a different environment.
src/surface/gui/gui/app.py
Outdated
| else 'light_blue.xml' | ||
| if theme_param == 'light' | ||
| else 'watermelon.xml' | ||
| ) |
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 code is confusing on what conditions set the value of base_theme to each possible value. Could you please refactor this into an easier to read format such as if and elif, switch cases, or some easier to read format. Also, because we default to dark theme when there is no parameter, I think it would be good to default to dark theme when anyone enters a value for the parameter that we don't have a theme for instead of using watermelon for that situation.
src/surface/gui/gui/app.py
Outdated
| else 'watermelon.xml' | ||
| ) | ||
| if base_theme == 'watermelon.xml': | ||
| base_theme = Path(get_package_share_directory('gui')) / 'styles' / ('watermelon.xml') |
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 line is causing an error in the IDE because base_theme is storing a string, and you are assigning a Path variable to it which isn't a string. It is also confusing that you already set the value of base_theme if we are in watermelon theme on line 45 but then you are setting it to a different value here. It might be better to combine this into the original way you set the value for base_theme so that you aren't trying to set a value for this condition twice.
ShannonGriswold
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.
Looks good!
Pull Request
What type of PR is this? (check all applicable)
Description
Uses qt-material to add theming to the gui
Related Tickets & Documents