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

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to