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

Implement xdg-desktop-portal accent-color spec #12674

Merged
merged 7 commits into from
Nov 19, 2023

Conversation

affederaffe
Copy link
Contributor

What does the pull request do?

Use the standardized xdg-desktop-portal setting to get the accent color
flatpak/xdg-desktop-portal#815

Currently, no DE actually implements it as it just got merged, hence WIP

What is the current behavior?

Accent colors only work on KDE

What is the updated/expected behavior with this PR?

Accent colors should work on every DE that provides them via the settings portal

How was the solution implemented (if it's not obvious)?

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

@affederaffe affederaffe marked this pull request as draft August 26, 2023 13:08
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0038807-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@orowith2os
Copy link

orowith2os commented Aug 27, 2023

It's fine if you remove the WIP tag, if the only reason is DEs implementing the key; the only actual requirement should be a stable xdg-desktop-portal release with the key. And even then, you can implement it without a stable release, or any release at all.

@maxkatz6
Copy link
Member

maxkatz6 commented Sep 4, 2023

@jmacato @kekekeks should we merge this PR now? Before actual distros support this feature. Since it's considered as a part of the standard now.

@jmacato
Copy link
Member

jmacato commented Sep 4, 2023

PR/Concept looks good to me, although i'll wait what @kekekeks has to say

@kekekeks
Copy link
Member

kekekeks commented Sep 4, 2023

The support for the spec seems to be merged into KDE yesterday, so I'd like to wait for the next KDE Neon build to test.

Not strictly required though.

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0039153-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@affederaffe
Copy link
Contributor Author

Testing it on KDE Neon, it doesn't work.
Though I think actually KDE isn't following the spec here: When changing the accent color, the SettingChanged signal is invoked with the value

   string "org.freedesktop.appearance"
   string "accent-color"
   variant       array [
         variant             double 0.0313726
         variant             double 0.611765
         variant             double 0.541176
      ]

but as per spec:

In the case that an accent color is requested, the underlying value would be a struct containing three doubles, each being an RGB value in the range of [0,1].

@jmacato
Copy link
Member

jmacato commented Sep 10, 2023

@affederaffe could we handle that without straying out of spec? perhaps a special case for KDE? (although ik that would suck...)

@orowith2os
Copy link

@jmacato KDE should fix their portal if it's not following the spec, you shouldn't be working around their bug.

@affederaffe
Copy link
Contributor Author

@jmacato I've asked in the KDE Plasma Matrix room to see if this is actually a spec violation or just me not reading said spec correctly. For now, I'd just wait since no DE has officially released any version that supports it anyway.

kdesysadmin pushed a commit to KDE/xdg-desktop-portal-kde that referenced this pull request Sep 11, 2023
@affederaffe affederaffe marked this pull request as ready for review September 12, 2023 21:31
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0039404-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0039525-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 requested a review from jmacato September 16, 2023 02:58
@jmacato
Copy link
Member

jmacato commented Sep 16, 2023

@affederaffe which KDE version is the spec implemented? could i use Debian Sid for testing?

@easyteacher
Copy link

easyteacher commented Sep 16, 2023 via email

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0039592-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@affederaffe affederaffe changed the title [WIP] Implement xdg-desktop-portal accent-color spec Implement xdg-desktop-portal accent-color spec Sep 18, 2023
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0039865-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@affederaffe
Copy link
Contributor Author

Confirmed to work on Plasma 5.27.8 and 6, GNOME still hasn't merged the relevant PRs...
Additionally, the current solution breaks on Plasma 6, probably due to a bug on their side as far as I can tell.

@easyteacher
Copy link

Confirmed to work on Plasma 5.27.8 and 6, GNOME still hasn't merged the relevant PRs... Additionally, the current solution breaks on Plasma 6, probably due to a bug on their side as far as I can tell.

https://invent.kde.org/plasma/xdg-desktop-portal-kde/-/merge_requests/244

@maxkatz6 maxkatz6 enabled auto-merge November 19, 2023 02:15
@maxkatz6 maxkatz6 added this pull request to the merge queue Nov 19, 2023
Merged via the queue into AvaloniaUI:master with commit fce38aa Nov 19, 2023
6 checks passed
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.

7 participants