Review Request: Add context menu to classic style Kickoff menu

Christian Loose christian.loose at hamburg.de
Sat Mar 21 19:39:40 CET 2009



> On 2009-03-21 10:09:28, Aaron Seigo wrote:
> > 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.

Thanks for the review. Yes, i will try harder to remove the code duplication.


- Christian


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


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
> 
>



More information about the Plasma-devel mailing list