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

[broulik (Kai Uwe Broulik)]

broulik added a comment.


  Do you actually cache the downloaded image somewhere on disk? I wouldn't want every startup of plasma download the image. Also, does it check for mobile internet being used instead of Wifi, looks like we need a NM dataengine.
  
  @graesslin Perhaps we need to re-introduce the X-Required-Permission stuff so a wallpaper can specify it needs internet and can then be filtered out from the lock screen config?

INLINE COMMENTS

> config.qml:71
> +            model: [
> +                { "label": i18n("Scaled and Cropped"), "fillMode": Image.PreserveAspectCrop },
> +                { "label": i18n("Scaled"), "fillMode": Image.Stretch },

Those quotes are unneccessary, label: "foo" is enough

> config.qml:79
> +
> +            Component.onCompleted: { readSizePosition(); }
> +            onCurrentIndexChanged: { writeSizePosition(); }

Those {} are unneccessary, Component.onCompleted: readSizePosition()

> config.qml:84
> +                for (var i in model) {
> +                    if (model[i]["fillMode"] == wallpaper.configuration.FillMode) {
> +                        sizePositionCombo.currentIndex = i;

model[i].fillMode ===

> config.qml:211
> +
> +                nameFilters: [ "Image files (*.jpg *.jpe *.jpeg *.png *.bmp *.svg *.svgz)" ]
> +                selectExisting: true

Can we do mime filters instead?

> config.qml:213-214
> +                selectExisting: true
> +                selectFolder: false
> +                selectMultiple: false
> +

I suppose those are the default values and don't need to explicitly mentioning?

> main.qml:39
> +
> +    function evalEnabledSources() {
> +        enabledSources.length = 0;

Can you make this more declarative, ie.?

  property var enabledSources: {
      // the stuff of this function
  }

> main.qml:40
> +    function evalEnabledSources() {
> +        enabledSources.length = 0;
> +        for (var i in allSources) {

Urgh

REPOSITORY
  rKDEPLASMAADDONS Plasma Addons

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

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

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