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
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to