D5932: KCM for controlling Night Color

[David Edmundson]

davidedmundson added a comment.


  looks mostly good.
  
  Make sure you test with  --reverse, I think your activator checkbox will be broken

INLINE COMMENTS

> kcm_nightcolor.desktop:17
> +Name=Night Color
> +Name[de]=Nachtfarbe
> +Comment=Adjust color temperatur at night to reduce eye strain

FYI, adding translations here is a waste of time

Scripty will ignore this when it extracts the text; then they'll be replaced when it puts the translations back

> LocationsAutoView.qml:21
> +
> +import org.kde.plasma.components 2.0 as PlasmaComponents
> +

you shouldn't be using Plasma Components in KCMs. Use QQC directly.

Plasma Components follow the plasma theme which can lead to a white text on white situation.

> main.qml:31
> +
> +    implicitWidth: units.gridUnit * 20
> +    implicitHeight: units.gridUnit * 20

We have an actual implicitWidth from the layout itself, we don't need to guess

> main.qml:341-342
> +            id: errorMessage
> +            anchors.centerIn: parent
> +            width: 0.8 * main.width
> +

just fill the parent. It's already set to main.width*0.8

REPOSITORY
  R119 Plasma Desktop

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

To: subdiff, #plasma
Cc: davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170522/25e89666/attachment.html>