Review Request: Add context menu to classic style Kickoff menu

[Aaron Seigo]

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


the concept is good, however the two showContextMenu methods in ContextMenuFactory should be harmonized. both produce a QModelIndex and then from that point forward it looks like the code is pretty much duplicated between the two.

i'd suggest creating a showContextMenu that takes a QModelIndex and a QPos and either calling that from both methods or just using that directly from both places in the menu code (so, getting the index in the menus first before calling ContextMenuFactory::showContextMenu).

once that bit of code dupe is removed, this can go in.

- Aaron


On 2009-03-21 09:45:15, Christian Loose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/377/
> -----------------------------------------------------------
> 
> (Updated 2009-03-21 09:45:15)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch adds the kickoff context menu to the classic style menu (simpleapplet). It uses the same ContextMenuFactory class like the Kickoff menu.
> 
> 
> This addresses bug 158302.
>     https://bugs.kde.org/show_bug.cgi?id=158302
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/CMakeLists.txt 941485 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/simpleapplet/simpleapplet.h 941485 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/simpleapplet/simpleapplet.cpp 941485 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/contextmenufactory.h 941485 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/contextmenufactory.cpp 941485 
> 
> Diff: http://reviewboard.kde.org/r/377/diff
> 
> 
> Testing
> -------
> 
> Tested with svn trunk.
> 
> 
> Thanks,
> 
> Christian
> 
>