----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3470/#review4821 -----------------------------------------------------------
Ship it! looks good (modulo the tooltip thing); the other comments are relevant to the code that already exists before your adjustments. so i'd say commit what you have here, and we can work on those other issues after that. /dev/null <http://reviewboard.kde.org/r/3470/#comment4301> it definitely doesn't need to start from scratch; this should really be changed to just showing/hiding items that do/don't match. i think this was done this way for ease of initial development only. /dev/null <http://reviewboard.kde.org/r/3470/#comment4302> setting the orientation and filtering ought to be separate, yes. when the list itself changes orientation, only the layout's orientation needs adjusting, not all the relayouting. /dev/null <http://reviewboard.kde.org/r/3470/#comment4303> yes, this also handles wheel event stuff. probably should just be "scrollRight" and "scrollLeft". the IconList::Wheel bit can also be killed as there is no difference anymore between how wheeling and keyboard animations are handled. - Aaron On 2010-03-31 20:12:41, Chani Armitage wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/3470/ > ----------------------------------------------------------- > > (Updated 2010-03-31 20:12:41) > > > Review request for Plasma. > > > Summary > ------- > > this factors out the appletbrowser code into separate classes so that it can > be shared with the as-yet-unwritten activity manager UI. I created two new > classes, IconList and IconListElement, which AppletsList and AppletIconWidget > now inherit. > > I think the class names kinda suck, so I'd be fine with renaming them if > someone has a better suggestion :) hrm, actually I was thinking of > AbstractIconList and AbstractIcon, since both have pure virtuals. > > since I haven't actually written the activity manager there'll probably be a > bit of code moved around later, but I think it'll stay fairly close to this > initial split, and I want to get the code in before any merge conflicts come > up. > > I haven't attempted to fix any bugs; I just split up the code, and made a > note of anything that needs attention later. > > > Diffs > ----- > > /dev/null PRE-CREATION > /dev/null PRE-CREATION > /dev/null PRE-CREATION > /dev/null PRE-CREATION > /dev/null PRE-CREATION > /trunk/KDE/kdebase/workspace/libs/plasmagenericshell/CMakeLists.txt 1106791 > > /trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appleticon.h > 1106791 > > /trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appleticon.cpp > 1106791 > > /trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appletslist.h > 1106791 > > /trunk/KDE/kdebase/workspace/libs/plasmagenericshell/widgetsexplorer/appletslist.cpp > 1106791 > > Diff: http://reviewboard.kde.org/r/3470/diff > > > Testing > ------- > > it seems to work as before. > there's just one regression: the tooltips aren't updated on scroll. shouldn't > be too hard to fix, just slipped my mind. > > > Thanks, > > Chani > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
