Review Request: toolbox actions as toolbar when zoomed out

Aaron Seigo aseigo at kde.org
Fri Nov 28 18:27:04 CET 2008


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.vidsolbach.de/r/285/#review273
-----------------------------------------------------------

Ship it!


there's probably room for performance improvement, but i'm not sure how much it even matters as it looks decent as is now. that there is room for improvement if needed is re-assuring, and profiling will tell us later if anything needs to be further tweaked.

other than the following couple of comments, let's get this in and refine it there =)


/trunk/KDE/kdelibs/plasma/applet.cpp
<http://reviewboard.vidsolbach.de/r/285/#comment221>

    should be:
    
    if (c && c->d->toolbox)



/trunk/KDE/kdelibs/plasma/private/desktoptoolbox.cpp
<http://reviewboard.vidsolbach.de/r/285/#comment222>

    that comment is no longer needed =)



/trunk/KDE/kdelibs/plasma/private/desktoptoolbox.cpp
<http://reviewboard.vidsolbach.de/r/285/#comment223>

    if (containment && viewTransform != q->viewTransform())
    
    unnecessarily nested if's make me itchy ;)


- Aaron


On 2008-11-28 07:14:06, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/285/
> -----------------------------------------------------------
> 
> (Updated 2008-11-28 07:14:06)
> 
> 
> 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/applet.cpp
>   /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
> 
>



More information about the Plasma-devel mailing list