D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders

Mihai Sorin Dobrescu noreply at phabricator.kde.org
Sun Jul 7 17:36:37 BST 2019


msdobrescu added a comment.


  IMHO, for lines like these, 'm_currentSlide = m_slideFilterModel->indexOf(path) - 1;', checks must be added because it is transparent to the QML code and let room for failure.
  
  Besides that, did you know that if there are no wallpapers, could be added one or more by drag and drop?
  Image files could be added by drag and drop, but never appear, although they are in the list, where should be folders only IMHO.
  They stay there, nothing happens, no background change on the desktop.
  Also, if a folder is added, 'Apply' button pressed, then removed, after 'Apply', the background remains set to the last image, although I would have expected to have no image set as wallpaper.

INLINE COMMENTS

> davidre wrote in image.cpp:600
> Currently the slideshow would restart. I guess we could check here if the index is -1. Or alternatively don't trigger this from the qml side if the image is unchecked. In my mind this even better because then we enable the apply button, too.

IMHO, here must be checked because it is transparent to the QML code and lets room for failure.

REPOSITORY
  R120 Plasma Workspace

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

To: davidre, #plasma, davidedmundson
Cc: davidedmundson, msdobrescu, ngraham, filipf, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190707/1b1e33d6/attachment-0001.html>


More information about the Plasma-devel mailing list