Bind gtk-enable-animations setting to global animation speed slider
ClosedPublic

Authored by broulik on Wed, Jan 22, 9:29 AM.

Details

Summary

When it is set to "Instant", turn off GTK animations.

Test Plan

(I'd say this is good for 5.18)

  • Started gedit, clicked "Open", got it flying in animated.
  • Changed slider to Instant in workspace KCM, restarted gedit, clicked "Open", got it show immediately
  • Settings change doesn't appear to work at runtime with gtk3 but that's probably unrelated

Diff Detail

Repository
R99 KDE Gtk Configuration Tool
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Wed, Jan 22, 9:29 AM
Restricted Application added a project: Plasma. · View Herald TranscriptWed, Jan 22, 9:29 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Wed, Jan 22, 9:29 AM
broulik edited the test plan for this revision. (Show Details)Wed, Jan 22, 9:34 AM
broulik updated this revision to Diff 74080.Wed, Jan 22, 9:41 AM
  • Add GTK2 key
gikari added inline comments.Wed, Jan 22, 10:24 AM
kded/gtkconfig.cpp
188

Wrong settings key

broulik updated this revision to Diff 74094.Wed, Jan 22, 10:29 AM
  • Fix settings key
davidedmundson added inline comments.
kded/configvalueprovider.cpp
145

You don't need this.

KConfigWatcher does it automagically on change.

I did that because I wanted a way for N connections to only reparse the file once. It also means you can use the config watcher with no signal handling, just install and you get the correct values on the next read. Amazing.

My fault for not having enough docs

broulik added inline comments.Wed, Jan 22, 12:21 PM
kded/configvalueprovider.cpp
145

I know, I just kept it consistent with the other code. Let's clean up this everywhere else above separately?

broulik updated this revision to Diff 74108.Wed, Jan 22, 12:48 PM
  • Drop superfluous reparseConfiguration call
gikari added inline comments.Wed, Jan 22, 1:23 PM
kded/gtkconfig.cpp
189

org.gnome.desktop.interface param is not necessary - it is the default. Most of the settings are in this category in dconf.

broulik updated this revision to Diff 74115.Wed, Jan 22, 1:54 PM
  • Drop pointless argument

+1 for putting this in 5.18 once @gikari thinks it's ready.

gikari requested changes to this revision.Wed, Jan 22, 3:13 PM

Seems like dconf does not convert string to boolean.

(process:24285): GLib-GIO-CRITICAL **: 17:51:47.492: g_settings_set_value: key 'enable-animations' in 'org.gnome.desktop.interface' expects type 'b', but a GVariant of type 's' was given

The method setGtk3ConfigValueDconf should be parameterized with the argument paramValue to be boolean. Something like this might work:

void ConfigEditor::setGtk3ConfigValueDconf(const QString &paramName, bool paramValue, const QString &category)
{
    g_autoptr(GSettings) gsettings = g_settings_new(category.toUtf8().constData());
    g_settings_set_boolean(gsettings, paramName.toUtf8().constData(), paramValue);
}
kded/configvalueprovider.h
57

It's not clear what ownConfig config is. GtkConfig does not have its own config (and even if it has, it is a bit strange to watch their own config in this context). If I understand correctly it is the kdeglobals. It would be nice to name it kdeglobalsConfig or globalsConfig

This revision now requires changes to proceed.Wed, Jan 22, 3:13 PM

Also i found out, that gtk apps on start throw this error:

(org.gnome.Nautilus:4278): Gdk-WARNING **: 21:08:58.246: Cannot transform xsetting gtk-enable-animations of type gchararray to type gboolean

This is because of xsettingd config. You need to add Gtk/EnableAnimations to exceptions in QStringList nonStringProperties var in replaceValueInXSettingsdContents method, but it's only a hotfix. It is better to do some parameterezing with QVariants or C++ templates for ConfigEditor functions.

broulik planned changes to this revision.Wed, Jan 22, 8:37 PM
broulik updated this revision to Diff 74193.Thu, Jan 23, 8:17 AM

Some typos, without them everything is OK.

kded/configeditor.cpp
173

You have a typo. The correct name of the parameter is "gtk-enable-animations"

199

You have a typo. EnableAnimations.

broulik updated this revision to Diff 74203.Thu, Jan 23, 10:21 AM
  • Fix typos
gikari accepted this revision.Thu, Jan 23, 10:26 AM
This revision is now accepted and ready to land.Thu, Jan 23, 10:26 AM
This revision was automatically updated to reflect the committed changes.