Review Request: [Picture Frame] Update function for url's and configuration of download path
etienne.rebetez at oberwallis.ch
etienne.rebetez at oberwallis.ch
Sat Mar 27 16:07:14 CET 2010
> On 2010-03-23 17:50:42, Aaron Seigo wrote:
> > personally, i don't see the need for a url configuration page at all. the download path should be the from desktop-wide coniguration and the picture should always be overwritten (the frame plasmoid is not a downloader; see comments below for more on this).
> >
> > the "update picture every" option doesn't follow the same UI style as seen on e.g. the slideshow configuration. it should probably be something like
> >
> > Autoupdate: [ 01 hours 00 Mins ]
> >
> > with a specialValueText of "Never" so when it is set to 0 hours and 0 mins, it shows (and means) "Never"
Thanks for your feedback and time :)
Yes, i think you are right with the download configuration. In fact, i was confiused myself, when i found my drag'n'droped picture in the download folder the first time i used the new feature.
After having implementet the update function, i quite overshot my inital target... Therefore i'll atempt to provide a new smaller patch only with the update function.
> On 2010-03-23 17:50:42, Aaron Seigo wrote:
> > trunk/KDE/kdeplasma-addons/applets/frame/picture.cpp, line 43
> > <http://reviewboard.kde.org/r/3334/diff/1/?file=21285#file21285line43>
> >
> > shouldn't this be in the cache directory, rather than in the user's downloads folder? and yes, i see that you just pulled this from Picture::slotFinished, but that looks wrong too :)
> >
> > i guess the real question is this: is the picture frame an image collection downloader or is it a "show a picture on my computer screen" plasmoid? if it's the latter, then there's no need to save the image somewhere permanent (e.g. ~/Downloads) and there's no need to have a "should we overwrite the picture" option as it should only ever keep one image around on disk at any time: the image it's currently showing.
So i'll make a separate patch for Bug 222759 using the cache directory and removing the "increment-picture-name-thing".
- eti
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3334/#review4627
-----------------------------------------------------------
On 2010-03-21 19:46:38, eti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3334/
> -----------------------------------------------------------
>
> (Updated 2010-03-21 19:46:38)
>
>
> Review request for Plasma.
>
>
> Summary
> -------
>
> I was quite happy that the picture frame could handle url's in 4.4.
> So i added the ability to update the picture with an url periodicaly (i.e. live cams, weather data or <picture that changes over time>).
> This would be a rather small fix, but i also came accross Bug 222759 and with the updatefunction the download path got quite spamed.
> Now it's possible to set the download path and the ability to ovewrite the existing picture when downloaded.
>
> By the way, i also removed the picture class from the slideshow class. The slideshow now manages only the path's and the frame class acceses the picture class directly.
>
> Let me know wat you think;)
>
>
> Diffs
> -----
>
> trunk/KDE/kdeplasma-addons/applets/frame/CMakeLists.txt 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/configdialog.h 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/configdialog.cpp 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/frame.h 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/imageSettings.ui 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/picture.h 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/picture.cpp 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/slideshow.h 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/slideshow.cpp 1105894
> trunk/KDE/kdeplasma-addons/applets/frame/urlSettings.ui PRE-CREATION
>
> Diff: http://reviewboard.kde.org/r/3334/diff
>
>
> Testing
> -------
>
>
> Screenshots
> -----------
>
> Picture Frame settings
> http://reviewboard.kde.org/r/3334/s/335/
>
>
> Thanks,
>
> eti
>
>
More information about the Plasma-devel
mailing list