mate-desktop / mate-settings-daemon

MATE settings daemon
https://mate-desktop.org
GNU General Public License v2.0
43 stars 47 forks source link

xrander-applet-popup: remove draw callback and handle the GNOME themes #224

Closed lukefromdc closed 6 years ago

lukefromdc commented 6 years ago

Remove the draw callback added back in https://github.com/mate-desktop/mate-settings-daemon/pull/221 and move the necessary portions back into make_menu_item_for_output_title

lukefromdc commented 6 years ago

I tested this in all themes, found same behavior as in master after make_menu_item_for_output_title with the theme code from https://github.com/mate-desktop/mate-themes/tree/xrandr-applet-suggestions

lukefromdc commented 6 years ago

I tried to also special-case the GNOME themes for a provider giving black text on the labels, but found that having added that to the provider it could not then be removed when switching back to the MATE themes, meaning that a user would see the black fg text on switching away from a GNOME theme until they restarted mate-settings-daemon (or their session).

Gave up on that for tonight when neither a provider for a context and gtk_style_context_remove_provider nor switching back to a provider for the screen (in the static/non-overrideable css) plus gtk_style_context_remove_provider_for_screen would clear the black text added for the GNOME themes.

lukefromdc commented 6 years ago

I finally got special-casing the GNOME themes and taking that back off the MATE themes to work. To test, start up GtkInspector on the popup, close the popup, and interate through each theme, opening the popup each time. You should see black monitor label text on all the GNOME themes but not on any MATE theme not setting black text explicitly.

raveit65 commented 6 years ago

Do we really need to support all gnome themes? Raleigh is a gtk+-2 theme which use Adwaita as fallback for gtk+3 and i don't see it in gnome git. https://gitlab.gnome.org/GNOME/gtk/tree/master/gtk/theme

[rave@satellite Schreibtisch]$ ls -l /usr/share/themes/Raleigh/
insgesamt 4
drwxr-xr-x. 2 root root 4096  6. Jul 2017  gtk-2.0

Doesn't the adwaita fallback work here?

And the win32 theme seems not ship with fedora 26. Any way, 2 lines more won't hurt.

monsta commented 6 years ago

Hmm... how would the label look with the themes not hardcoded here (if the theme doesn't have anything special for xrandr applet too)?

monsta commented 6 years ago

Looks like it's gray...

Clearlooks-Phenix (no colored bg for some reason): virtualbox_debian testing_23_05_2018_15_43_07

Greybird: virtualbox_debian testing_23_05_2018_15_44_13

raveit65 commented 6 years ago

Looks like it's gray...

Clearlooks-Phenix (no colored bg for some reason):

because they use shiat code blocks like this:

 label,
 label:disabled {
     background-color: transparent;
 }

which is a old remnant from < gtk+-3.20

Greybird:

Can be added here in code as it is a popular xfce theme.

And before you ask, some themes use background-image: in css as bg for menus, eg. Greenlaguna, BlackMate, this will never be override by background-color:

lukefromdc commented 6 years ago

OK, thanks for the point about "background-image" as any theme that either sets that on a label inside a disabled menu item OR sets in on a label in a normal menuitem and does not take if off for a disabled menuitem won't display the label background color. I will add that to the CSS and take care of this, in the overrideable section.

I have both Raleigh and win32 here, on my local build of gtk 3.22.30. Like Adwaita, they are included in GTK itself. You won't see them in m-c-c but will see them in GtkInspector. Raleigh in general works poorly these days, but as recently as GTK 3.14 there was some discussion at GNOME of making it the default when theme engines were deprecated.

lukefromdc commented 6 years ago

In my local gtk+ repository with the 3.22 branch selected, I have these four directories:

gtk/theme/Adwaita gtk/theme/HighContrast gtk/theme/Raleigh gtk/theme/win32

Master is GTK4 and I would not be surprised if some things were removed there

lukefromdc commented 6 years ago

Latest commit forces black monitor label text in Greybird/Bluebird/Blackbird themes, ensures default bg images don't override the label color when not explicitly themed out, and keeps the GNOME high contrast themes from using a symbolic icon. A symbolic icon gets the disabled color by default, and keeping it off would require another cssprovider and it would be different for HighContrast and HighContrastInverse. Too much code just to support use of a symbolic icon there and only there.

xrandr_popup_hc_and_bird_themes

lukefromdc commented 6 years ago

One more cleanup: show the icon only if the user has icons in menus turned on (the default) as this should respect the user's global settings

raveit65 commented 6 years ago

Switching the icon on/off works well.

raveit65 commented 6 years ago

@lukefromdc What about xrandr-popup-turn-monitors-on-off-WIP ? Do you working on them?

lukefromdc commented 6 years ago

Theming is the same on that branch (it was rebased after this commit) and I've found that turning a monitor off is easy, the hard part is to turn it on I've got to find out where the geometry information was kept from when it was last used. In the configuration capplet (from m-s-d), it's not necessary to manually reconfigure a display before turning it back on, and the GUI shows the last used position. When turned on, its geometry, refresh rate, position, and rotation are same as before unless deliberately changed. All that information has to be saved somewhere-and it's NOT in ~/.config/monitors.xml (when the monitor in question is OFF) and in additon /etc/mate-settings-daemon/xrandr is empty at least on my machine.

raveit65 commented 6 years ago

Ok, so you need this branch.

~/.config/monitors.xml (when the monitor in question is OFF) and in additon /etc/mate-settings-daemon/xrandr

Still wondering that those dirs don't give the needed infos. I always delete ~/.config/monitors.xml to restore defaults if something goes wrong. /etc/mate-settings-daemon/xrandr is only use if you store your config system-wide.

lukefromdc commented 6 years ago

The necessary code turned out to be available from mate-settings-daemon's display capplet. Behavior in my tests with that code was exactly the same as turning a monitor off and on in the capplet without reconfiguration, which should be good enough given that the capplet can be called from the popup as well.

lukefromdc commented 6 years ago

https://github.com/mate-desktop/mate-settings-daemon/pull/226