----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/344/#review512 -----------------------------------------------------------
direction looks good. some small things to improve i think :) /trunk/KDE/kdelibs/plasma/extender.cpp <http://reviewboard.kde.org/r/344/#comment306> how about: foreach (ExtenderItem *item, attachedItems()) { if (item->isGroup()) { .. } } /trunk/KDE/kdelibs/plasma/extendergroup.cpp <http://reviewboard.kde.org/r/344/#comment303> seems a bit of a shame to have to go through all the items in an extender like this. i suppose, however, there won't ever be that many items so the overhead will be low and the cost of bookkeeping with an internal QList in ExtenderGroup would not pay off at all while increasing memory usage and complexity. bleh. /trunk/KDE/kdelibs/plasma/extenderitem.h <http://reviewboard.kde.org/r/344/#comment304> need apidox :) but do we even need this? one should be able to qobject_cast as needed? /trunk/KDE/kdelibs/plasma/extenderitem.cpp <http://reviewboard.kde.org/r/344/#comment305> would make sense to have an ExtenderPrivate::findGroup(const QString&) for this to prevent cycling through all items, creating a list, then cycling through that list when its returned. doesn't need to be in the public api imho, but we can make the internals more efficient here. - Aaron On 2009-03-16 12:38:12, Rob Scheepmaker wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/344/ > ----------------------------------------------------------- > > (Updated 2009-03-16 12:38:12) > > > Review request for Plasma. > > > Summary > ------- > > I've been working on adding support for grouping to extenders and using this > to group jobs in the plasma systemtray: a useful feature we already discussed > at Tokamak. I've already discussed the extender api bit at Tokamak with > Aaron, but changed a couple of things about it while implementing because > they could have been more elegant. There are still some relatively minor > problems I know of, and it hasn't been that thoroughly tested yet, but I > wanted to see if you think I'm going in the right direction here. > > The basic idea is that to group extender items, you'll instantiate an > ExtenderGroup, which is a subclass of ExtenderItem. The thing this class > adds, are expand and collapse buttons, to show/hide the items that belong to > this group, and the ability to let the group automatically hide itself > whenever it becomes empty. To add items to a group you'll have to call > setGroup on each ExtenderItem you wish to add to that group. This association > with a group will get stored in the item's config(). For the rest it behaves > like any other extender item. > > I will still have to add behavior that automatically moves all items > belonging to this group with it, whenever you detach the ExtenderGroup. Also, > the usage of groups in systemtray could still use improvement. > > Screenshot showing the job grouping in action: > http://quartz.student.utwente.nl/~rob/extendergrouping.png Clicking the > arrow-down here would collapse this group (keeping only the total progress > item) > > > Diffs > ----- > > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/CMakeLists.txt > 939911 > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/core/job.h 939911 > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/core/manager.h > 939911 > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/core/manager.cpp > 939911 > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/applet.cpp 939911 > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/jobwidget.cpp > 939911 > /trunk/KDE/kdelibs/plasma/CMakeLists.txt 939911 > /trunk/KDE/kdelibs/plasma/applet.h 939911 > /trunk/KDE/kdelibs/plasma/extender.h 939911 > /trunk/KDE/kdelibs/plasma/extender.cpp 939911 > /trunk/KDE/kdelibs/plasma/extendergroup.h PRE-CREATION > /trunk/KDE/kdelibs/plasma/extendergroup.cpp PRE-CREATION > /trunk/KDE/kdelibs/plasma/extenderitem.h 939911 > /trunk/KDE/kdelibs/plasma/extenderitem.cpp 939911 > /trunk/KDE/kdelibs/plasma/private/extenderitem_p.h 939911 > > Diff: http://reviewboard.kde.org/r/344/diff > > > Testing > ------- > > > Thanks, > > Rob > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
