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

Provide a standard dark style preference key in the settings portal #629

Closed
Exalm opened this issue Sep 9, 2021 · 19 comments · Fixed by #633
Closed

Provide a standard dark style preference key in the settings portal #629

Exalm opened this issue Sep 9, 2021 · 19 comments · Fixed by #633

Comments

@Exalm
Copy link
Contributor

Exalm commented Sep 9, 2021

Background: https://blog.elementary.io/the-need-for-a-freedesktop-dark-style-preference/

That's now live in elementary OS 6, and I'm currently implementing the same thing in GNOME, and I think it would be a good time to actually standardize it instead of having separate incompatible preferences.

The main reason to do it here and not just have a gsettings key in org.gnome.desktop.interface and pass that through the portal as is would be apps like Firefox or Telegram - right now they don't really have any way to get the system preference on elementary, or the upcoming one on GNOME, or the similar looking preference in KDE, etc.

Settings portal currently doesn't define any keys and leaves it to implementation. What if we actually defined a key that the implementation would proxy where they need it?

My prototype implementation uses the following:

  • Namespace: org.freedesktop.appearance
  • Key: color-scheme
  • Possible values: strings default, prefer-light, prefer-dark.

The elementary setting is https://github.com/elementary/default-settings/blob/master/accountsservice/io.elementary.pantheon.AccountsService.xml#L43-L52

CC @danrabbit @cassidyjames from the elementary side

CC @aleixpol from the KDE side - I'm not sure who would be the relevant person here

@aleixpol
Copy link
Contributor

aleixpol commented Sep 9, 2021

Hey, it does make a lot of sense.

It won't be entirely trivial to implement in our end, since our users select a colour scheme to use rather than light vs dark but seeing how varied toolkits are nowadays, standardising colour schemes would definitely be out of reach.

+1 in general from the our side. I've shared it in some KDE channels in case someone has any ideas.
Thanks!

@danirabbit
Copy link

We currently abstract this away a bit for developers in Granite so it would be fairly simple to adopt whatever is agreed upon here without breaking API for developers. Very +1 from the elementary camp. Thanks :)

@Exalm
Copy link
Contributor Author

Exalm commented Sep 9, 2021

It won't be entirely trivial to implement in our end, since our users select a colour scheme to use rather than light vs dark but seeing how varied toolkits are nowadays, standardising colour schemes would definitely be out of reach.

Yeah... I suppose it would need to be a separate thing from the general KDE color schemes. In theory it also should be possible to do it based on whether the window foreground color is lighter or darker than background.

@DarkTrick
Copy link

Thank you for the effort on implementing it on GNOME.

I would suggest a file for storing the setting, though. This way it's the most compatible and long-lived format.

Suggestion:

  • file ~/.config/freedesktop.org/setting.conf
  • file format: Key/value format (easily readable with GKeyFile)
  • key: color-scheme
  • value: 0 (default), 1 (prefer-light), 2 (prefer-dark)

@Exalm
Copy link
Contributor Author

Exalm commented Sep 10, 2021

That's incompatible with sandboxing. Also, where we store it is not really relevant - we'll just use gsettings, other portal implementations are free to use whatever they want - a file would be perfectly fine on that side. But since there's no guarantee at all apps will be able to actually access that file (and by default they won't be), actually accessing that file from the apps won't work, you need an intermediate interface - in this case we already have a portal used for similar purposes except with implementation-specific keys, so why not reuse it.

@GeorgesStavracas
Copy link
Member

Aleix's comment on Matrix when asked what other Qt & KDE folks thought:

we discussed it briefly in a couple of channels, everyone seems either ambivalent or happy about it, so 👍️

@Exalm
Copy link
Contributor Author

Exalm commented Sep 15, 2021

Ok, so I suppose the only other person I need an ack from is @TingPing as the portal author.

@TingPing
Copy link
Member

@Exalm 👍

@Exalm
Copy link
Contributor Author

Exalm commented Sep 15, 2021

Ok, great!

So then on technical side looks like the only thing on xdg-desktop-portal side would be updating docs and bumping the interface version and tests.

@Exalm
Copy link
Contributor Author

Exalm commented Sep 19, 2021

Hm, looking at it again I wonder if it should be a new interface instead, like the power profile monitor portal. That seems a lot more straightforward. :)

With that in mind, I propose: (with the impl interface looking the same)

<node name="/" xmlns:doc="http://www.freedesktop.org/dbus/1.0/doc.dtd">
  <!--
    org.freedesktop.portal.Appearance:
    @short_description: Appearance portal

    The appearance portal allows sandboxed applications to query
    the system about its appearance preferences.

    This documentation describes version 1 of this interface.
  -->
  <interface name="org.freedesktop.portal.Appearance">

    <!--
        color-scheme:

        The system's preferred color scheme.

        Supported values are:
        <simplelist>
          <member>0: No preference</member>
          <member>1: Prefer dark appearance</member>
          <member>2: Prefer light appearance</member>
        </simplelist>
    -->
    <property name="color-scheme" type="u" access="read"/>

    <property name="version" type="u" access="read"/>
  </interface>
</node>

I've also changed the values from strings to a uint, seems unnecessary to pass strings around for this.

@GeorgesStavracas
Copy link
Member

I'm not entirely sure about calling it color-scheme, but moving it into its own interface sounds like a good idea.

@TingPing
Copy link
Member

TingPing commented Sep 19, 2021

I will admit the line between interfaces is somewhat arbitrary.

I think PowerProfileMonitor makes sense to me because at the higher level abstractions (GLib), it its a different interface (You create GPowerProfilerMonitor).

But to me color schemes is conceptually just another setting. I'd expect a toolkit to use it like it uses org.freedesktop.portal.Settings. The documentation for that interface is "This interface provides read-only access to a small number of host settings required for toolkits similar to XSettings." which I believe describes this.

@Exalm
Copy link
Contributor Author

Exalm commented Sep 19, 2021

That was my thought as well when proposing it in the settings portal initially, just thought it might be cleaner to have it separately. Fair enough. In that case:

  <!--
    org.freedesktop.portal.Settings:
    @short_description: Settings interface

    This interface provides read-only access to a small number
    of host settings required for toolkits similar to XSettings.
    It is not for general purpose settings.

    Currently the interface provides the following keys:

    <variablelist>
      <varlistentry>
        <term>org.freedesktop.appearance color-scheme u</term>
        <listitem><para>
          Indicates the system's preferred color scheme.
          Supported values are:
          <simplelist>
            <member>0: No preference</member>
            <member>1: Prefer dark appearance</member>
            <member>2: Prefer light appearance</member>
          </simplelist>
          Since version 2.
        </para></listitem>
      </varlistentry>
    </variablelist>

    Implementations can provide other keys; they are entirely
    implementation details that are undocumented. If you are a
    toolkit and want to use this please open an issue.

    This documentation describes version 2 of this interface.
  -->

I'm not entirely sure about calling it color-scheme,

See https://gitlab.gnome.org/GNOME/libadwaita/-/merge_requests/224#note_1258529

Other platforms usually call it either color scheme (web), appearance (Apple platforms), theme (UWP, Flutter, Xamarin), or night mode (Android). Of these 4, night mode doesn't seem appropriate at all, theme is a loaded word and better to avoid it, appearance is another possibility though.

@TingPing
Copy link
Member

      Since version 2.

Its not an API change. You don't need to bump the version. It should already safely handle unknown values.

@Exalm
Copy link
Contributor Author

Exalm commented Sep 19, 2021

Makes sense, removed that part.

  <!--
    org.freedesktop.portal.Settings:
    @short_description: Settings interface

    This interface provides read-only access to a small number
    of host settings required for toolkits similar to XSettings.
    It is not for general purpose settings.

    Currently the interface provides the following keys:

    <variablelist>
      <varlistentry>
        <term>org.freedesktop.appearance color-scheme u</term>
        <listitem><para>
          Indicates the system's preferred color scheme.
          Supported values are:
          <simplelist>
            <member>0: No preference</member>
            <member>1: Prefer dark appearance</member>
            <member>2: Prefer light appearance</member>
          </simplelist>
          Unknown values should be considered same as 0.
        </para></listitem>
      </varlistentry>
    </variablelist>

    Implementations can provide other keys; they are entirely
    implementation details that are undocumented. If you are a
    toolkit and want to use this please open an issue.

    This documentation describes version 1 of this interface.
  -->

@TingPing
Copy link
Member

TingPing commented Sep 19, 2021

You can document that clients should treat an unknown value as 0 for no preference of course. Otherwise looks good.

@Exalm
Copy link
Contributor Author

Exalm commented Sep 19, 2021

Added "Unknown values should be considered same as 0.". Now we have the name, I'm not super attached to color-scheme - just went with what elementary had and the other variants didn't seem significantly better.

@TingPing
Copy link
Member

I like color-scheme, it even allows room for growth :P.

Exalm added a commit to Exalm/xdg-desktop-portal that referenced this issue Sep 19, 2021
Specify a key for getting the system's preferred color scheme.

Fixes flatpak#629
Exalm added a commit to Exalm/xdg-desktop-portal that referenced this issue Sep 19, 2021
Specify a key for getting the system's preferred color scheme.

Fixes flatpak#629
TingPing pushed a commit that referenced this issue Sep 20, 2021
Specify a key for getting the system's preferred color scheme.

Fixes #629
@n0toose
Copy link

n0toose commented Sep 27, 2021

I think I can say +1 from the @tenacityteam camp. Thank you <3

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 a pull request may close this issue.

7 participants