Skip to content
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

Always prefer 'Tango' icon theme #250

Merged
merged 2 commits into from
Apr 29, 2021
Merged

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Apr 28, 2021

The legacy behavior was to attempt to detect if an existing theme was already set which supplied some of the needed icons, and if not, use the Tango theme specifically. If that didn't work either, revert the change to the original theme (or lack thereof).

The current behavior changes the theme only if no theme is set, and only if tango_icons_vendor is installed. This breaks down on Ubuntu Focal because the default theme (hicolor) doesn't supply the necessary icons for rqt to function properly. On Fedora, the default theme (Adwaita) appeared to work properly.

This change makes qt_gui always prefer the 'Tango' theme, and always prefer the version supplied by tango_icons_vendor (if available). If 'Tango' is not available, fall back to the system theme (or lack thereof).

Related to #222, #224, and ros-visualization/tango_icons_vendor#8
Fixes ros-visualization/rqt#249

The legacy behavior was to attmpt to detect if an existing theme was
already set which supplied some of the needed icons, and if not, use the
Tango theme specifically. If that didn't work either, revert the change
to the original theme (or lack thereof).

The current behavior changes the theme only if no theme is set, and only
if tango_icons_vendor is installed.

This change makes qt_gui always prefer the 'Tango' theme, and always
prefer the version supplied by tango_icons_vendor (if available). If
'Tango' is not available, fall back to the system theme.

Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay added the bug label Apr 28, 2021
@cottsay cottsay requested a review from ahcorde April 28, 2021 01:41
@cottsay cottsay self-assigned this Apr 28, 2021
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Contributor

I tested this out on Linux, and it seems to be working well. I'm going to also test this out on Windows to make sure things still work there.

@clalancette
Copy link
Contributor

I tested this out on Linux, and it seems to be working well. I'm going to also test this out on Windows to make sure things still work there.

Bah. I can't get Windows to work, but it doesn't work before this change anyway. Something must be messed up in my environment.

Can someone else test out Windows here?

@cottsay cottsay requested a review from ahcorde April 28, 2021 15:51
@cottsay
Copy link
Member Author

cottsay commented Apr 28, 2021

Can someone else test out Windows here?

@ivanpauno and/or @hidmic - can you give this change a try on your Galactic test instances?

@ivanpauno
Copy link
Contributor

@ivanpauno and/or @hidmic - can you give this change a try on your Galactic test instances?

Sure, I have a windows instance running.

@ivanpauno
Copy link
Contributor

Sure, I have a windows instance running.

Tested on Windows, I see the exact same icons before and after the PR 🎉

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants