[Differential] [Commented On] D1976: Image of the Day wallpaper plugin

[bgupta (Boudhayan Gupta)]

bgupta added inline comments.

INLINE COMMENTS

> davidedmundson wrote in config.qml:27
> If you switch these to ColumnLayout / RowLayout (from QtQuick.Layouts, which you're currently not using) you can get rid of a lot of the anchors and widths in this code.

I was initially using ColumnLayout, but the problem there was that the contents were expanding to fill the entire height of the dialog, with huge spacing between rows. Using Column just works.

> davidedmundson wrote in config.qml:56
> it defeats the point of having spacing as semanticly defined macros, if people then do maths with it to get any arbitrary value

I looked at `config.qml` files from the color and image wallpaper plugins from `plasma-workspace`. They seem to do things like this, so I did the same.

> davidedmundson wrote in config.qml:106
> this won't acheive anything
> 
> you've set a width, but by default there's no eliding, so it'll just overflow past here anyway.

I don't see any overflow on my computer. Is there any?

> davidedmundson wrote in config.qml:264
> why?

The objectName? Setting an objectName is something that I do by force of habit since trying to add QML bits to Spectacle (to be able to get to objects from the C++ side by name), but of course there's no use for objectNames here. I'll remove them in a later pass.

> davidedmundson wrote in main.qml:93
> where is resetTimer defined?

Ah yes, I forgot to remove that line.

> davidedmundson wrote in main.qml:105
> we generally try to avoid pixel sizes.

What would be an appropriate substitute in this case? units.smallSpacing?

> davidedmundson wrote in main.qml:111
> why?
> 
> you're displaying this at a fixed size

It's true that there's no regular scaling involved. I clearly misunderstood the point of mipmap scaling over regular smoothing. I'll swap this in favour of smoothing.

REPOSITORY
  rKDEPLASMAADDONS Plasma Addons

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

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

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