D8916: QML port of fonts kcm

Eike Hein noreply at phabricator.kde.org
Tue Nov 21 08:50:36 UTC 2017


hein added inline comments.

INLINE COMMENTS

> fonts.cpp:570
> +    emit smallFontChanged();
> +    setNeedsSave(true);
> +}

Instead of always setting this to true in a prop setter, you need to implement a method that checks whether the new value is actually different from the stored configuration and then sets true or false based on that. Otherwise you are turning on the 'Apply' button but never turning it off again. Cf. LaunchFeedback::updateNeedsSave() in https://phabricator.kde.org/D8911.

> main.qml:175
> +                enabled: dpiCheckBox.checked
> +                value: kcm.fontAASettings.dpi
> +                onValueChanged: kcm.fontAASettings.dpi = dpiSpinBox.value

This and similar bindings will break when the user changes the spinbox value, so after a manual adjustment e.g. defaults() won't work any longer. It needs a seperate Connections à la https://phabricator.kde.org/D8911.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D8916

To: mart, #plasma, #vdg
Cc: hein, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20171121/d6c87dcd/attachment.html>


More information about the Plasma-devel mailing list