D26825: Bind gtk-enable-animations setting to global animation speed slider
This revision was automatically updated to reflect the committed changes. Closed by commit R99:66ab37df7aed: Bind gtk-enable-animations setting to global animation speed slider (authored by broulik). REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26825?vs=74203=74207 REVISION DETAIL https://phabricator.kde.org/D26825 AFFECTED FILES kded/configeditor.cpp kded/configeditor.h kded/configvalueprovider.cpp kded/configvalueprovider.h kded/gtkconfig.cpp kded/gtkconfig.h To: broulik, #plasma, gikari Cc: ngraham, davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
broulik updated this revision to Diff 74203. broulik added a comment. - Fix typos REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26825?vs=74193=74203 REVISION DETAIL https://phabricator.kde.org/D26825 AFFECTED FILES kded/configeditor.cpp kded/configeditor.h kded/configvalueprovider.cpp kded/configvalueprovider.h kded/gtkconfig.cpp kded/gtkconfig.h To: broulik, #plasma, gikari Cc: ngraham, davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
gikari added a comment.
Some typos, without them everything is OK.
INLINE COMMENTS
> configeditor.cpp:173
> QStringLiteral("gtk-primary-button-warps-slider"),
> +QStringLiteral("enable-animations"),
> };
You have a typo. The correct name of the parameter is
"__gtk-__enable-animations"
> configeditor.cpp:199
> QStringLiteral("Gtk/PrimaryButtonWarpsSlider"),
> +QStringLiteral("Gtk/EnableAnimation"),
> };
You have a typo. EnableAnimation__s__.
REPOSITORY
R99 KDE Gtk Configuration Tool
REVISION DETAIL
https://phabricator.kde.org/D26825
To: broulik, #plasma, gikari
Cc: ngraham, davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev,
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
broulik updated this revision to Diff 74193. REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26825?vs=74115=74193 REVISION DETAIL https://phabricator.kde.org/D26825 AFFECTED FILES kded/configeditor.cpp kded/configeditor.h kded/configvalueprovider.cpp kded/configvalueprovider.h kded/gtkconfig.cpp kded/gtkconfig.h To: broulik, #plasma, gikari Cc: ngraham, davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
gikari added a comment. 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 `QVariant`s or C++ templates for `ConfigEditor` functions. REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D26825 To: broulik, #plasma, gikari Cc: ngraham, davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
ngraham added a comment. +1 for putting this in 5.18 once @gikari thinks it's ready. REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D26825 To: broulik, #plasma, gikari Cc: ngraham, davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
gikari requested changes to this revision.
gikari added a comment.
This revision now requires changes to proceed.
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 , bool
paramValue, const QString )
{
g_autoptr(GSettings) gsettings =
g_settings_new(category.toUtf8().constData());
g_settings_set_boolean(gsettings, paramName.toUtf8().constData(),
paramValue);
}
INLINE COMMENTS
> configvalueprovider.h:57
> KSharedConfigPtr kwinConfig;
> +KSharedConfigPtr ownConfig;
> };
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`
REPOSITORY
R99 KDE Gtk Configuration Tool
REVISION DETAIL
https://phabricator.kde.org/D26825
To: broulik, #plasma, gikari
Cc: ngraham, davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev,
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
broulik updated this revision to Diff 74115. broulik added a comment. - Drop pointless argument REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26825?vs=74108=74115 REVISION DETAIL https://phabricator.kde.org/D26825 AFFECTED FILES kded/configvalueprovider.cpp kded/configvalueprovider.h kded/gtkconfig.cpp kded/gtkconfig.h To: broulik, #plasma, gikari Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
gikari added inline comments.
INLINE COMMENTS
> gtkconfig.cpp:189
> +
> ConfigEditor::setGtk2ConfigValue(QStringLiteral("gtk-enable-animations"),
> enableAnimations);
> +
> ConfigEditor::setGtk3ConfigValueDconf(QStringLiteral("enable-animations"),
> enableAnimations, QStringLiteral("org.gnome.desktop.interface"));
> +
> ConfigEditor::setGtk3ConfigValueSettingsIni(QStringLiteral("gtk-enable-animations"),
> enableAnimations);
`org.gnome.desktop.interface` param is not necessary - it is the default. Most
of the settings are in this category in dconf.
REPOSITORY
R99 KDE Gtk Configuration Tool
REVISION DETAIL
https://phabricator.kde.org/D26825
To: broulik, #plasma, gikari
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh,
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
broulik updated this revision to Diff 74108. broulik added a comment. - Drop superfluous `reparseConfiguration` call REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26825?vs=74094=74108 REVISION DETAIL https://phabricator.kde.org/D26825 AFFECTED FILES kded/configvalueprovider.cpp kded/configvalueprovider.h kded/gtkconfig.cpp kded/gtkconfig.h To: broulik, #plasma, gikari Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
broulik added inline comments. INLINE COMMENTS > davidedmundson wrote in configvalueprovider.cpp:146 > 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 I know, I just kept it consistent with the other code. Let's clean up this everywhere else above separately? REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D26825 To: broulik, #plasma, gikari Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
davidedmundson added inline comments.
INLINE COMMENTS
> configvalueprovider.cpp:146
> +{
> +ownConfig->reparseConfiguration();
> +KConfigGroup generalCfg = ownConfig->group(QStringLiteral("KDE"));
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
REPOSITORY
R99 KDE Gtk Configuration Tool
REVISION DETAIL
https://phabricator.kde.org/D26825
To: broulik, #plasma, gikari
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh,
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
broulik updated this revision to Diff 74094. broulik added a comment. - Fix settings key REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26825?vs=74080=74094 REVISION DETAIL https://phabricator.kde.org/D26825 AFFECTED FILES kded/configvalueprovider.cpp kded/configvalueprovider.h kded/gtkconfig.cpp kded/gtkconfig.h To: broulik, #plasma, gikari Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
gikari added inline comments.
INLINE COMMENTS
> gtkconfig.cpp:188
> +const QString enableAnimations = configValueProvider->enableAnimations();
> +ConfigEditor::setGtk2ConfigValue(QStringLiteral("gtk-button-images"),
> enableAnimations);
> +
> ConfigEditor::setGtk3ConfigValueDconf(QStringLiteral("enable-animations"),
> enableAnimations, QStringLiteral("org.gnome.desktop.interface"));
Wrong settings key
REPOSITORY
R99 KDE Gtk Configuration Tool
REVISION DETAIL
https://phabricator.kde.org/D26825
To: broulik, #plasma, gikari
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
broulik updated this revision to Diff 74080. broulik added a comment. - Add GTK2 key REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26825?vs=74077=74080 REVISION DETAIL https://phabricator.kde.org/D26825 AFFECTED FILES kded/configvalueprovider.cpp kded/configvalueprovider.h kded/gtkconfig.cpp kded/gtkconfig.h To: broulik, #plasma, gikari Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D26825: Bind gtk-enable-animations setting to global animation speed slider
broulik created this revision. broulik added reviewers: Plasma, gikari. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. broulik requested review of this revision. REVISION SUMMARY When it is set to "Instant", turn off GTK animations. TEST PLAN - 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 REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D26825 AFFECTED FILES kded/configvalueprovider.cpp kded/configvalueprovider.h kded/gtkconfig.cpp kded/gtkconfig.h To: broulik, #plasma, gikari Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
