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

(qt_gui) setFallbackThemeName(original_theme) #279

Merged
merged 1 commit into from Nov 14, 2023

Conversation

MatthijsBurgh
Copy link
Contributor

@MatthijsBurgh MatthijsBurgh commented Aug 16, 2023

Old:
old

New:
new

Iron backport: #281
Humble backport: #280

Fixes: ros-visualization/rqt_image_view#79

@MatthijsBurgh
Copy link
Contributor Author

@cottsay @clalancette could you take a look at this as you worked on #250

@cottsay
Copy link
Member

cottsay commented Aug 16, 2023

Would it be possible for a system to be configured to use Tango by default and another theme as a fallback already? Wouldn't this regress the behavior in that scenario?

@MatthijsBurgh
Copy link
Contributor Author

The changes you made in #250 always overrule the default theme, if I understand the code correctly.

So in case Tango was the default theme, you end up with Tango being the default and the backup. I don't know a system/Qt can/will have a fallback theme by default.
I checked on my system and it didn't have a fallback theme or fall back search paths, while running X11 on ubuntu 22.04 (These should/could be set, see https://doc.qt.io/qt-5/qicon.html#fallbackThemeName):

original_theme='Vimix'
QIcon.themeSearchPaths()=['/opt/ros/humble/share/tango_icons_vendor/resource/icons', '/home/USERNAME/.local/share/icons', '/usr/local/share/icons', '/usr/share/icons', '/var/lib/snapd/desktop/icons', ':/icons']
QIcon.fallbackThemeName()=''
QIcon.fallbackSearchPaths()=[]

I changed the logic to prevent ending up with Tango as default and fallback theme.

@MatthijsBurgh
Copy link
Contributor Author

@cottsay could you have a look again after adding some changes based on your comment?

@MatthijsBurgh
Copy link
Contributor Author

@cottsay friendly ping ;) Including #280 and #281

@MatthijsBurgh
Copy link
Contributor Author

@cottsay firendly ping ping ;)

@MatthijsBurgh
Copy link
Contributor Author

@clalancette could you do the 2nd review and merge?

Please also merge the backports.

P.S. Please create new releases for the various distros.

@MatthijsBurgh
Copy link
Contributor Author

@clalancette can you please do the 2nd review?

Or can we merge it with only the approval of @cottsay?

@clalancette
Copy link
Contributor

CI:

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

@clalancette clalancette merged commit bad1d9a into ros-visualization:rolling Nov 14, 2023
2 checks passed
@MatthijsBurgh
Copy link
Contributor Author

@clalancette Thanks! Do we want the backports to Humble and Iron too? See #280 and #281

@MatthijsBurgh MatthijsBurgh deleted the rolling-1 branch November 15, 2023 07:12
@MatthijsBurgh
Copy link
Contributor Author

@cottsay @clalancette Could you create a new rolling release including this PR?

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

Successfully merging this pull request may close these issues.

[ros2] button icons missing
3 participants