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

Why disable custom themes? #39

Open
phil294 opened this issue Feb 25, 2023 · 5 comments
Open

Why disable custom themes? #39

phil294 opened this issue Feb 25, 2023 · 5 comments
Assignees

Comments

@phil294
Copy link

phil294 commented Feb 25, 2023

Hi! Thanks for all the work on this. As far as I can tell until now, it works flawlessly! Except, of course, hardcoding Adwaita (dark/light) is not so cool, as this completely breaks the OS-wide Ui integration.
Now in the source, it says Custom themes are broken. I haven't been able to verify this yet: I disabled that line and found that everything seems to work out fine. For example, this is Chicago95, a heavily customized theme, with a popup and some controls:

imagen

It's exactly how it should look like and how it also looks without AppImage in the first place.

Do you have some more details on this? Some way to replicate bad behavior? Because otherwise, I am in favor of keeping this line disabled and shipping it as is to my users.

phil294 added a commit to phil294/AHK_X11 that referenced this issue Feb 26, 2023
@TheTumultuousUnicornOfDarkness TheTumultuousUnicornOfDarkness added question Further information is requested and removed question Further information is requested labels Feb 27, 2023
@TheTumultuousUnicornOfDarkness
Copy link
Collaborator

Hello,

In some case, custom themes can work fine: it depends of GTK version inside AppImage and GTK version on your system.
For example, I see Chicago95 requires GTK+ 3.22 or 3.24. If AppImage was built under Ubuntu 16.04 with GTK 3.18, then the custom theme will fail to load properly on Ubuntu 18.04 (GTK 3.22) and Ubuntu 20.04 (GTK 3.24), leading to a broken theme.

  1. In that same file, delete the line export GTK_THEME="$APPIMAGE_GTK_THEME" # Custom themes are broken (this is a temporary fix)

In fact, there is no need for this workaround, see by yourself:

APPIMAGE_GTK_THEME="${APPIMAGE_GTK_THEME:-"Adwaita:$GTK_THEME_VARIANT"}"

If you set APPIMAGE_GTK_THEME=chicago95, it will use your custom theme.

Some way to replicate bad behavior?

Yes, I am able to reproduce this issue with CPU-X 4.0.0 AppImage:
CPU-X_4 0 0_AppImage
As you can see on the left, if I set CPUX_GTK_THEME=Arc-Darkest (my GTK theme), then tabs and buttons are ugly and there are a lot of warnings in console. With Adwaita, it is okay, because Adwaita is built inside GTK.
Please note that CPUX_GTK_THEME environment variable is the predecessor of APPIMAGE_GTK_THEME.
FYI, it is supposed to look like this (i.e. natively, without AppImage) with my custom GTK theme:
CPU-X_4 5 2_native

Because otherwise, I am in favor of keeping this line disabled and shipping it as is to my users.

Feel free to export APPIMAGE_GTK_THEME=chicago95 if you are sure it is safe for your users.
Adwaita is set by default in AppImage to avoid issues with custom themes as showed above.

@phil294
Copy link
Author

phil294 commented Feb 27, 2023

Thank for for the detailed response.

In some case, custom themes can work fine: it depends of GTK version inside AppImage and GTK version on your system.
For example, I see Chicago95 requires GTK+ 3.22 or 3.24. If AppImage was built under Ubuntu 16.04 with GTK 3.18, then the custom theme will fail to load properly on Ubuntu 18.04 (GTK 3.22) and Ubuntu 20.04 (GTK 3.24), leading to a broken theme.

This makes sense, so it's about Gtk breaking things over time again. I'll try this out and also replicate your screenshots and find what other themes are affected by this. Do you know if it is generally a safe bet to build on Ubuntu 18.04 then?

In fact, there is no need for this workaround, see by yourself:

APPIMAGE_GTK_THEME="${APPIMAGE_GTK_THEME:-"Adwaita:$GTK_THEME_VARIANT"}"

I have seen this, but I'm not sure how forcing a different theme other than Adwaita on my users would solve anything. Yes I can set this to chicago95, but what's the point if nobody has it installed? The only thing that is justified to put as APPIMAGE_GTK_THEME appears to be a theme that is available on all common Linux distros, and Adwaita seems to be the only one where this applies.

Or do you mean to also ship the custom theme alongside? Either way, I wouldn't want to force any specific theme ever, neither on the system nor a single gtk app. Linux is all about customization which this would break. OS theme integration far outweighs ugly tabs on some exotic distro IMO. Question is just how exotic that is. May I ask what distro you are building CPU-X on?

@TheAssassin
Copy link
Member

TheAssassin commented Feb 27, 2023

Well, theming is always an issue. Mostly technically, but also UX wise, especially when applications should be distributed on more than one platform from the same package. Distributions can invest a lot more time into testing, after all. See also https://stopthemingmy.app/ and maybe also https://invidious.snopyta.org/watch?v=hdRGVej3RyI.

If you want a specific theme, shipping it is the most reliable way to do so. Otherwise, using a known good fallback is a good idea.

Edit: APPIMAGE_GTK_THEME is probably more of a power user feature and not really supposed to be used by developers, I guess. As a user, you can set this in your profile to force apps packaged with this plugin to look more "native".

@phil294
Copy link
Author

phil294 commented Feb 27, 2023

No, I don't want a specific them. I also don't want a fallback. I want to not break customization for the user.

I understand how this might be preferable for some Gtk application authors though, but I wonder if overwriting APPIMAGE_GTK_THEME by default is really that good of an idea.

But anyway, I can (and am doing) just alter the script, so all is good. :-)

@TheAssassin
Copy link
Member

Please don't hesitate to contribute a way to opt out of this behavior. This shouldn't require a lot of code. Patching the auto generated script sounds like a really annoying workaround.

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

No branches or pull requests

3 participants