Details
- Reviewers
- None
- Group Reviewers
Plasma VDG - Maniphest Tasks
- T7244: Fonts (somewhere in branch)
- Commits
- R119:e7032a030b83: needssave only when settings actually changed
R119:24b960f92284: QML port of fonts kcm
tried all the options
Diff Detail
- Repository
- R119 Plasma Desktop
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Can you mark this as "Fixes T7244", so that task will be automatically closed when this goes in?
FWIW, the list of all KCM redesign tasks can be found at https://phabricator.kde.org/tag/plasma_kcm_redesign/
| kcms/fonts/fonts.cpp | ||
|---|---|---|
| 632 | 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 D8911. | |
| kcms/fonts/package/contents/ui/main.qml | ||
| 176 | 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 D8911. | |
| kcms/fonts/package/contents/ui/main.qml | ||
|---|---|---|
| 176 | no, it doesn't seem to break. when the change in value is donefrom the c++ side, which is the case of a qqc2 spinbox (as opposed to qqc1) the bindings actually stay in place, would break only if from javascript side a spinbox.value=foo was executed | |
Could we possibly take the opportunity to turn font anti-aliasing on by default instead of leaving it with "Vendor Default" (a string that doesn't have any meaning to most users)?
i would really not mess with the distro settings, which usually have a reason. we risk to break way too many corner cases
one thing that may make sense, is to indeed put a "show advanced controls" there, (thing that i usually hate ;)