> 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 > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
