----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.vidsolbach.de/r/285/#review271 -----------------------------------------------------------
a few little things, but impressive how small the patch actually ended up being. neat. /trunk/KDE/kdelibs/plasma/private/desktoptoolbox.cpp <http://reviewboard.vidsolbach.de/r/285/#comment219> we are passed a Containment* into the ctor, so we may as well hold on to it as a member and avoid the overhead of casting /trunk/KDE/kdelibs/plasma/private/desktoptoolbox.cpp <http://reviewboard.vidsolbach.de/r/285/#comment220> this call is a bit expensive, and will fail if you are viewing the same Containment in two views with different zooms. perhaps that's a real edge case that doesn't really ever happen in the Real Wordl(tm)? for performance, it might make more sense to handle this in the Containment's paint event with the widget that is passed in? or i suppose that would be Applet::paint; would be a little messy, but might be better for performance and internal messyness (for the right reasons, anyways) is ok. the idea would be to set a state on the toolbox in the Applet's paint based on the QWidget passed in; hmmmm... though that may also break with multiple views. at least it would be faster though =) - Aaron On 2008-11-27 14:34:54, Marco Martin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.vidsolbach.de/r/285/ > ----------------------------------------------------------- > > (Updated 2008-11-27 14:34:54) > > > Review request for Plasma. > > > Summary > ------- > > this makes the toolbox actions to be drawn in an horizontal line and the > cashew isn't drawn, it feels reeally dirty the polling for view transform in > the paint event, but really don't know how to react differently (a zoomed > view signal could be useful maybe?, or maybe this approach is the least > damage compared to adding signals now:) > > there are still know problems/things, like: > maybe should be drawn outside the containment, but for that need to change > the containment positioning, so would rather get an initial working thing in > before :) > > on the second level of zoom the toolbox becomes bigger than the containments, > so only icons should be shown, iconwidget lacks such an option, so either > adding it now or using toolbuttons > > anything else? (yes, pretty sure it is:p) > > > Diffs > ----- > > /trunk/KDE/kdelibs/plasma/private/desktoptoolbox.cpp > /trunk/KDE/kdelibs/plasma/private/toolbox.cpp > /trunk/KDE/kdelibs/plasma/private/toolbox_p.h > > Diff: http://reviewboard.vidsolbach.de/r/285/diff > > > Testing > ------- > > > Thanks, > > Marco > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
