Review Request: 'Next image' action for image plugin (and wallpaper contextual actions) - kdebase diff

[Yuen Hoe Lim]

> On 2009-10-12 20:08:28, Chani wrote:
> > /trunk/KDE/kdebase/workspace/plasma/generic/containmentactions/contextmenu/menu.cpp, line 194
> > <http://reviewboard.kde.org/r/1822/diff/1/?file=12457#file12457line194>
> >
> >     iirc this is going to leak.
> >     what I did for other separators was to include them in the monster list above - _sep1 and _sep2

Oh yeah. Will fix when I get home tonight.


> On 2009-10-12 20:08:28, Chani wrote:
> > /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/image.cpp, line 86
> > <http://reviewboard.kde.org/r/1822/diff/1/?file=12459#file12459line86>
> >
> >     setting an empty list is redundant :)

Is it? I was thinking that when the user switches from slide show mode to single-image mode the contextual actions should be cleared (there shouldn't be a 'next image' option when there is only one image), but now that I think about it I'm not sure if we're even using the same instance anymore when the user makes such a switch.

At any rate, I think it's harmless, how bout keeping it just as an extra check to avoid surprises :)


> On 2009-10-12 20:08:28, Chani wrote:
> > /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/image.cpp, line 90
> > <http://reviewboard.kde.org/r/1822/diff/1/?file=12459#file12459line90>
> >
> >     according to the docs this is the same as append... huh. I never knew.

For people used to std::vector I suppose :)


- Yuen Hoe


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1822/#review2619
-----------------------------------------------------------


On 2009-10-11 14:16:47, Yuen Hoe Lim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1822/
> -----------------------------------------------------------
> 
> (Updated 2009-10-11 14:16:47)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> (I can't get my changes in kdebase and kdelibs into the same diff, so this is 'diff 2 of 2', with the kdebase changes)
> 
> Adds support for wallpaper plugins to specify their own contextual actions that will be added to the containment context menu. These wallpaper contextual actions can be enabled/disabled like any other contextual actions via the mouse plugin context menu configuration ui (by checking/unchecking 'Wallpaper Actions').
> 
> Also adds a 'Next Wallpaper Image' contextual action for the image wallpaper plugin using this new support. This functionality only activates when image plugin is in 'slide show' mode (and of course if it is enabled in the mouse plugin ui).
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/generic/containmentactions/contextmenu/menu.cpp 1033402 
>   /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/image.h 1033402 
>   /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/image.cpp 1033402 
> 
> Diff: http://reviewboard.kde.org/r/1822/diff
> 
> 
> Testing
> -------
> 
> Built and briefly tested the functionalities mentioned in the description - they work fine as far as I can tell.
> 
> 
> Thanks,
> 
> Yuen Hoe
> 
>