Detect theme variant for Gnome and KDE #3414

metal3d · 2022-11-19T08:09:40Z

Description:

This detects theme variant for Gnome and KDE.

For Gnome, read values from gsettings to get the color-scheme (can be prefer-dark, prefer-light or "default"
For KDE, read the kdeglobals file and calculate the brightness if the theme is not the standard one - this method can also be applied for older Gnome version reading GTK themes, but is this useful?
Theme change detection: - Gnome: connect to dbus and wait for theme change - KDE, no idea at this time, but switching from KDE to Gnome has changed the Gnome theme... So maybe KDE is using dbus too.

Checklist:

Tests included.
Lint and formatter run with no errors.
Tests all pass.

- For Gnome, read values from `gsettings` to get the color-scheme (can be prefer-dark, prefer-light or "default" - For KDE, read the kdeglobals file and calculate the brightness if the theme is not the standard one - this method can also be applied for older Gnome version reading GTK themes, but is this useful? - Theme change detection: - Gnome: connect to dbus and wait for theme change - KDE, no idea at this time, but switching from KDE to Gnome has changed the Gnome theme... So maybe KDE is using dbus too.

coveralls · 2022-11-19T08:13:26Z

Coverage increased (+0.1%) to 61.776% when pulling 57d1aa9 on metal3d:xdg-variant-detection into 018b4bf on fyne-io:develop.

Jacalz · 2022-11-19T18:20:51Z

Would it, by any chance, be possible to stick to only the BDus values mentioned in #2657?

It seems less elegant and more prone to error if we should parse files specific to KDE or Gnome. The issue in question mentions a new standard and I think it would be wise to instead only rely on that. This way we reduce the risk of weird issues if the structures of the files we are parsing change in the future

Bluebugs · 2022-11-19T20:24:23Z

Would it, by any chance, be possible to stick to only the BDus values mentioned in #2657?

I would think that this should be the first method tried. As a fallback and due to the large number of old os supported by fyne, we need to still try the files.

It seems less elegant and more prone to error if we should parse files specific to KDE or Gnome. The issue in question mentions a new standard and I think it would be wise to instead only rely on that. This way we reduce the risk of weird issues if the structures of the files we are parsing change in the future

The file structure shouldn't massively change as this could also a problem for the application that exist today and use this file.

metal3d · 2022-11-19T21:38:58Z

I can avoid to use gsettings for Gnome and read the value from DBus. It's a complex code, but it gives the unit8 value of the variant.
KDE doesn't use the DBus property for now.

There are 2 possibilities:

I only change the call to gsettings and we accept, at this time, to read the config file of KDE for it
I remove everything to only use the DBus value, but this will not work on KDE for a certain time

As @Bluebugs mentioned, KDE didn't change the structure of the file, and will not for a while.

Using only DBus drastically reduces the code size. But KDE will not be supported. And AFAIK, it could be for a long time... KDE is not famous to quickly follow the standards.

BTW: KDE is messy, and it changes the Gnome/GTK theme each time I use it – it does this to ensure that GTK apps will match the Breeze theme (or whatever the theme you choose on KDE preferences). That means that each time I switch my session, and I'm back to Gnome, it changed the icon theme, the color theme, add window buttons, and many changes... It's exhausting, annoying, clearly not a behavior I appreciate.
I personally don't use it, I installed KDE to make tests, that's a regret now 😄

So, my opinion now: I personally prefer to use DBus with standard freedesktop values, because it's standard... Now, what are the Fyne rules? Do we accept "default dark theme" for KDE and wait them to respect the standards?

I don't have the power to decide 😉

andydotxyz

This is excellent, thanks so much.
The SetTheme needs to be changed though, as it will override developer settings.

app/app_xdg.go

andydotxyz · 2022-11-19T21:54:36Z

This is so cool, thanks :)

So, my opinion now: I personally prefer to use DBus with standard freedesktop values, because it's standard... Now, what are the Fyne rules? Do we accept "default dark theme" for KDE and wait them to respect the standards?

This would be my preference - but as you say it will not work for KDE for some time... so maybe the KDE code needs to remain.
For Gnome I think the DBus should be OK - this is a new setting if I understand correctly so older non-DBus versions won't have it anyway will they?

app/app_xdg.go

metal3d · 2022-11-19T22:07:18Z

This would be my preference - but as you say it will not work for KDE for some time... so maybe the KDE code needs to remain.

So, for now, I will leave the code like this (with changed you asked)

For Gnome I think the DBus should be OK - this is a new setting if I understand correctly so older non-DBus versions won't have it anyway will they?

Actually, I made this detection very recently. Months ago, I prepared themes for Gnome variants and KDE/Plasma:

Gnome, I was using gjs to get colors (and icons), and I made brightness computation
KDE, mainly the same as in this PR, reading file and calculate the brightness.

I decided to not reimplement the Gnome theme reading with gjs because of the complexity (it calls javascripts, following the current theme for detected version of target GTK version... see fyne-io/fyne-x#48 which is now a draft)

I really don't know if I should take some parts of what I did in fyne-x, or if I should use DBus value only for Gnome and remove the KDE specific case later.

In any case, I've got solutions. The hard part is to decide.

metal3d · 2022-11-19T22:09:35Z

To complete the answer, I can do (for Gnome):

Solution 1:
Only use DBUS

Solution 2:
Try DBUs, fallback to gjs call and calculate the brightness

Solution 3:
Only make gjs call (I don't like)

Jacalz · 2022-11-20T12:21:36Z

I am quite certain that I have read somewhere that KDE Plasma actually support the dark style preference. A quick Google search did also seem to indicate that it is the case: https://www.reddit.com/r/kde/comments/rwizt0/freedesktop_dark_style_preference_for_kde_apps/

metal3d · 2022-11-21T07:17:22Z

I am quite certain that I have read somewhere that KDE Plasma actually support the dark style preference. A quick Google search did also seem to indicate that it is the case: https://www.reddit.com/r/kde/comments/rwizt0/freedesktop_dark_style_preference_for_kde_apps/

I made many tests to check – when I select the light/dark theme in KDE / Plasma on Fedora 36, it doesn't change anything on the DBus property. It changes the kdeglobals file, it breaks Gnome theme by forcing GTK and icon theme, but the DBus property is still set to the same value.

Maybe it changed in Fedora 37 (I didn't upgrade yet), but at this time, it's not the case.

So, I wonder what to do.

If I only use this DBus variable, newest KDE will be OK and Gnome will be OK. But, previous KDE version will not respond.

Because it's only to detect the variant, that is not a breaking feature, and because the color-scheme variable in DBus is the "correct" way to decide... Should we not, finally, leave the previous behavior for desktop manager that are not yet ready to use it? And so, leave the previous behavior (dark theme by default) for them?

I mean, the decision could be:

DBus color scheme found:
- OK, set the color variant to the value found
- watch the variant change in DBus to react on changes
else
- set color variant to dark
- no watch

This will simplify the source code too.

Jacalz · 2022-11-21T07:21:48Z

I am personally leaning more to what you are describing above. I’d be interested in what the others think of it

metal3d · 2022-11-21T09:21:06Z

Actually, if we change the behavior by what I described, the functions should be renamed to "XXXFreedesktop" - that will not be specific to Gnome or KDE 😄

Bluebugs · 2022-11-21T18:07:30Z

Sounds like a good plan to me too.

metal3d · 2022-11-21T18:08:57Z

I will change source code in a few minutes/hours, if it's not OK for @andydotxyz I can revert.

andydotxyz · 2022-11-21T23:06:11Z

I will change source code in a few minutes/hours, if it's not OK for @andydotxyz I can revert.

I am happy to go with the consensus here - we can always iterate as we get user feedback.

- Gnome already makes use of color-scheme from freedesktop portal settings - KDE should provide the value in current and next version - waiting for it: fallback to dark theme variant - For all others desktop managers, fallback to dark theme variant

The variant should be calculated by the work made at fyne-io/fyne#3414 So, we can now only get colors and icons from the applied theme in Gnome or KDE.

Jacalz

Thanks. I annoyed a few minor things that can be tidied up but it is getting very close

Jacalz · 2022-11-22T08:35:37Z

app/app_xdg.go

+ case 1:
+ return theme.VariantDark


I don't think you need three cases here. You can remove "1" case, and just add a comment that dark is a one, then let it be caught by the default

Right, I wondered if that was interesting for the reading. I can change.

app/app_xdg.go

Jacalz

I just tested this on Fedora 36 with Gnome 42 and it does not seem to work. Nothing happens when I change between light and dark mode. The app just always use the dark mode no matter what preference I set in the settings

Jacalz

I now retested after having upgraded to Fedora 37 and Gnome 43. It works flawlessly. I don't know why I did not see it working before. Nice work 🙂
Just found a few last notes. Should be very close to landing 👍

Jacalz · 2022-11-28T09:25:02Z

app/app_xdg.go

+
+ var value uint8
+ if err = call.Store(&value); err != nil {
+ log.Println("failed to read theme variant from D-Bus", err)


Suggested change

log.Println("failed to read theme variant from D-Bus", err)

fyne.LogError("Failed to read theme variant from D-Bus", err)

Jacalz · 2022-11-28T09:25:25Z

app/app_xdg.go

+ dbus.WithMatchInterface("org.freedesktop.portal.Settings"),
+ dbus.WithMatchMember("SettingChanged"),
+ ); err != nil {
+ log.Println(err)


Let's use fyne.LogError here as well.

Jacalz · 2022-11-28T09:25:35Z

app/app_xdg.go

+func watchFreedekstopThemeChange() {
+ conn, err := dbus.SessionBus()
+ if err != nil {
+ log.Println("Unable to connect to session D-Bus", err)


Let's use fyne.LogError here as well.

app/app_xdg.go

Jacalz

Works great. Thanks for working on this. It will make Fyne applications work so much better on Linux :)

Jacalz · 2022-11-29T19:23:35Z

@andydotxyz Did we decide that it was fine to merge this even though we are in a feature freeze? I have a vague recollection of talking about that but I can't remember.

andydotxyz · 2022-11-29T20:43:27Z

Yes I think it's good - "bug fixes" still allowed and this is technically one. Though an epic one 😀 .
Once we get the blockers closes it will be code freeze and no more new fixes unless blocking.

Jacalz · 2022-11-29T21:13:11Z

Great. You still have required changes. Feel free to review and merge if you feel that it is ready :)

andydotxyz

Great thanks

The variant should be calculated by the work made at fyne-io/fyne#3414 So, we can now only get colors and icons from the applied theme in Gnome or KDE.

metal3d added 2 commits November 19, 2022 09:03

Fix a bad syntax listening dbus event chan

45bbbdc

andydotxyz mentioned this pull request Nov 19, 2022

Update Linux/XDG app settings to delegate theme to new fd.o color-scheme #2657

Closed

Jacalz self-requested a review November 19, 2022 09:25

andydotxyz requested changes Nov 19, 2022

View reviewed changes

app/app_xdg.go Outdated Show resolved Hide resolved

app/app_xdg.go Outdated Show resolved Hide resolved

andydotxyz reviewed Nov 19, 2022

View reviewed changes

app/app_xdg.go Outdated Show resolved Hide resolved

metal3d added 5 commits November 19, 2022 23:19

Fix function name and theme switch

dfbbd8d

Make functions to parse color and brightness

7bf9848

Use DBus instead of gsettings to get the variant

4b64bf8

Fix bad doc comment

537a263

Fix unused error variable

4e98a8c

metal3d requested review from andydotxyz and removed request for Jacalz November 20, 2022 09:25

metal3d added a commit to metal3d/fyne-x that referenced this pull request Nov 22, 2022

Simplify the integration, see details

94b21de

The variant should be calculated by the work made at fyne-io/fyne#3414 So, we can now only get colors and icons from the applied theme in Gnome or KDE.

metal3d mentioned this pull request Nov 22, 2022

Propose desktop integration fyne-io/fyne-x#48

Draft

Jacalz requested changes Nov 22, 2022

View reviewed changes

metal3d added 2 commits November 22, 2022 10:01

Remove redondant case

a38d7ec

Break on reached wanted message

15957da

metal3d requested review from Jacalz and andydotxyz and removed request for andydotxyz and Jacalz November 22, 2022 09:58

Jacalz requested changes Nov 22, 2022

View reviewed changes

Jacalz requested changes Nov 28, 2022

View reviewed changes

metal3d added 3 commits November 28, 2022 17:32

Use Fyne.LogError

014f66f

No need size of 10 in chan

d079c2f

No need to set size...

57d1aa9

metal3d requested a review from Jacalz November 28, 2022 16:40

Jacalz approved these changes Nov 29, 2022

View reviewed changes

andydotxyz approved these changes Nov 29, 2022

View reviewed changes

andydotxyz merged commit dbdb9fc into fyne-io:develop Nov 29, 2022

lucor mentioned this pull request Jan 12, 2023

Update Linux/XDG default theme selection #3561

Merged

3 tasks

metal3d added a commit to metal3d/fyne-x that referenced this pull request Jun 16, 2023

Simplify the integration, see details

082795e

The variant should be calculated by the work made at fyne-io/fyne#3414 So, we can now only get colors and icons from the applied theme in Gnome or KDE.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect theme variant for Gnome and KDE #3414

Detect theme variant for Gnome and KDE #3414

metal3d commented Nov 19, 2022

coveralls commented Nov 19, 2022 •

edited

Loading

Jacalz commented Nov 19, 2022

Bluebugs commented Nov 19, 2022

metal3d commented Nov 19, 2022

andydotxyz left a comment

andydotxyz commented Nov 19, 2022

metal3d commented Nov 19, 2022

metal3d commented Nov 19, 2022

Jacalz commented Nov 20, 2022

metal3d commented Nov 21, 2022

Jacalz commented Nov 21, 2022

metal3d commented Nov 21, 2022

Bluebugs commented Nov 21, 2022

metal3d commented Nov 21, 2022

andydotxyz commented Nov 21, 2022

Jacalz left a comment

Jacalz Nov 22, 2022

metal3d Nov 22, 2022

Jacalz left a comment

Jacalz left a comment

Jacalz Nov 28, 2022

Jacalz Nov 28, 2022

Jacalz Nov 28, 2022

Jacalz left a comment

Jacalz commented Nov 29, 2022

andydotxyz commented Nov 29, 2022

Jacalz commented Nov 29, 2022

andydotxyz left a comment

	log.Println("failed to read theme variant from D-Bus", err)
	fyne.LogError("Failed to read theme variant from D-Bus", err)

Detect theme variant for Gnome and KDE #3414

Detect theme variant for Gnome and KDE #3414

Conversation

metal3d commented Nov 19, 2022

Description:

Checklist:

coveralls commented Nov 19, 2022 • edited Loading

Jacalz commented Nov 19, 2022

Bluebugs commented Nov 19, 2022

metal3d commented Nov 19, 2022

andydotxyz left a comment

Choose a reason for hiding this comment

andydotxyz commented Nov 19, 2022

metal3d commented Nov 19, 2022

metal3d commented Nov 19, 2022

Jacalz commented Nov 20, 2022

metal3d commented Nov 21, 2022

Jacalz commented Nov 21, 2022

metal3d commented Nov 21, 2022

Bluebugs commented Nov 21, 2022

metal3d commented Nov 21, 2022

andydotxyz commented Nov 21, 2022

Jacalz left a comment

Choose a reason for hiding this comment

Jacalz Nov 22, 2022

Choose a reason for hiding this comment

metal3d Nov 22, 2022

Choose a reason for hiding this comment

Jacalz left a comment

Choose a reason for hiding this comment

Jacalz left a comment

Choose a reason for hiding this comment

Jacalz Nov 28, 2022

Choose a reason for hiding this comment

Jacalz Nov 28, 2022

Choose a reason for hiding this comment

Jacalz Nov 28, 2022

Choose a reason for hiding this comment

Jacalz left a comment

Choose a reason for hiding this comment

Jacalz commented Nov 29, 2022

andydotxyz commented Nov 29, 2022

Jacalz commented Nov 29, 2022

andydotxyz left a comment

Choose a reason for hiding this comment

coveralls commented Nov 19, 2022 •

edited

Loading