[Differential] [Requested Changes To] D1976: Image of the Day wallpaper plugin

sebas (Sebastian Kügler) noreply at phabricator.kde.org
Thu Jun 23 10:31:15 UTC 2016


sebas requested changes to this revision.
sebas added a reviewer: sebas.
sebas added a comment.
This revision now requires changes to proceed.


  There are lots of ;  (at the end of every line of QML. We generally don't do that. In order to make the code more consistent, please remove them. (They're fine in your javascript functions.)

INLINE COMMENTS

> config.qml:49
> +
> +            font.pointSize: theme.defaultFont.pointSize * 1.25;
> +            font.bold: true;

Use PlasmaExtras.Heading instead of sizing on your own, this will lead to more consistency. Look at other config dialogs how it's done.

> config.qml:120
> +                width: parent.width - 2 * units.smallSpacing;
> +                height: theme.mSize(theme.defaultFont).height;
> +                color: bgColorDialog.color;

use units.gridUnit here.

> config.qml:178
> +                if (intervalHours.value < 1) {
> +                    intervalMinutes.minimumValue = 15;
> +                } else {

So it will at most switch every 15 minutes? Seems a bit long as minimal value to me...

> config.qml:205
> +            id: fallbackImageButton;
> +            width: theme.mSize(theme.defaultFont).width * 24;
> +            text: {

units.gridUnit instead of theme.mSize

> config.qml:237
> +
> +            font.pointSize: theme.defaultFont.pointSize * 1.25;
> +            font.bold: true;

PlasmaExtras.Heading

REPOSITORY
  rKDEPLASMAADDONS Plasma Addons

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: bgupta, #plasma, sebas
Cc: sebas, graesslin, davidedmundson, plasma-devel, #plasma, jensreuterberg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160623/a637daf2/attachment.html>


More information about the Plasma-devel mailing list