> On June 24, 2012, 1:41 a.m., David Edmundson wrote:
> > I think you've overcomplicated this:
> >
> > Everything after currentContainment could be replaced with simply:
> >
> > ---
> > currentContainment->setWallpaper(name, mode);
> > if (!path.isEmpty()) {
> > currentContainment->wallpaper()->setUrls(KUrl(path));
> > }
> > ---
> >
> > if the name and mode haven't changed setWallpaper will do nothing.
> > if the plugin doesn't support setUrls it will do nothing, if it does - your
> > API will work with it \o/.
> > Containment::setWallpaper appears to save when changing, same for setUrls
> > on the plugin.
> >
> > I tested the above and it saves and restores fine :)
When the name is "image" and mode is "Slideshow", image with path doesn't seem
to be added to the slideshow. So I wasn't sure if using setUrls for all cases
was a good idea. Is it okay to do that ?
As for the rest - It just didn't strike me to use setUrls after setWallpaper
and that lead to over complication :P Learnt something. And will update the
diff soon.
- Varun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105319/#review15052
-----------------------------------------------------------
On June 22, 2012, 2:36 p.m., Varun Herale wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105319/
> -----------------------------------------------------------
>
> (Updated June 22, 2012, 2:36 p.m.)
>
>
> Review request for Plasma.
>
>
> Description
> -------
>
> This patch is for hosting a dbus-interface that can be used to load any
> installed wallpaper plugin onto current desktop containment. In case of
> default "image" plugin, the path to the image can also be sent which will
> change the wallpaper.
>
>
> Diffs
> -----
>
> plasma/desktop/shell/dbus/org.kde.plasma.App.xml eefce32
> plasma/desktop/shell/plasmaapp.h 6ae0c89
> plasma/desktop/shell/plasmaapp.cpp 7abd8fc
>
> Diff: http://git.reviewboard.kde.org/r/105319/diff/
>
>
> Testing
> -------
>
> Tested on different activities and made sure it works for per-virtual desktop
> containment.
>
> Haven't tested on a system with multiple screens though, as I don't have
> access to one. Could someone please test for that ?
>
>
> Thanks,
>
> Varun Herale
>
>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
