Review Request: custom toolbox support

Aaron Seigo aseigo at kde.org
Mon Oct 19 23:27:09 CEST 2009


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1861/#review2720
-----------------------------------------------------------



/trunk/KDE/kdelibs/plasma/containment.cpp
<http://reviewboard.kde.org/r/1861/#comment2018>

    wouldn't it be "cleaner" reading to just do:
    
    PanelToolBox *pt = new PanelToolBox(q);
    toolBox = pt;
    
    (same before for the desktop one)



/trunk/KDE/kdelibs/plasma/containment.cpp
<http://reviewboard.kde.org/r/1861/#comment2017>

    should be a cast to DesktopToolBox, no?



/trunk/KDE/kdelibs/plasma/containment.cpp
<http://reviewboard.kde.org/r/1861/#comment2019>

    it's casting it to a PanelToolBox but storing it as an InternalToolBox?
    
    why isn't this just done in the switch statement above where it creates a PanelToolBox?



/trunk/KDE/kdelibs/plasma/toolbox.h
<http://reviewboard.kde.org/r/1861/#comment2020>

    AbstractToolBox?



/trunk/KDE/kdelibs/plasma/toolbox.h
<http://reviewboard.kde.org/r/1861/#comment2025>

    needs apidox; and QT_VERSION macro is unneeded as we require 4.6 now anyways?



/trunk/KDE/kdelibs/plasma/toolbox.h
<http://reviewboard.kde.org/r/1861/#comment2021>

    in plasma.h instead?



/trunk/KDE/kdelibs/plasma/toolbox.h
<http://reviewboard.kde.org/r/1861/#comment2024>

    this is actually a pretty poor name that we've kept only because it's an internal API ;) 
    
    it can also be an edge ... setLocation would be the "natural" name imho ... maybe the corner values should be add to Plasma::Location and that enum used?



/trunk/KDE/kdelibs/plasma/toolbox.h
<http://reviewboard.kde.org/r/1861/#comment2023>

    const?



/trunk/KDE/kdelibs/plasma/toolbox.h
<http://reviewboard.kde.org/r/1861/#comment2022>

    const pointer?


- Aaron


On 2009-10-17 21:48:21, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1861/
> -----------------------------------------------------------
> 
> (Updated 2009-10-17 21:48:21)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> this adds a setToolboxItem function.
> it makes possible for the openToolBox() function to work if the containment totally manages the toolbox by itself.
> it uses a "showing" property that custom toolboxes should implement to show/hide the toolbox
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/CMakeLists.txt 1036057 
>   /trunk/KDE/kdelibs/plasma/applet.cpp 1036057 
>   /trunk/KDE/kdelibs/plasma/containment.h 1036057 
>   /trunk/KDE/kdelibs/plasma/containment.cpp 1036057 
>   /trunk/KDE/kdelibs/plasma/private/desktoptoolbox.cpp 1036057 
>   /trunk/KDE/kdelibs/plasma/private/desktoptoolbox_p.h 1036057 
>   /trunk/KDE/kdelibs/plasma/private/paneltoolbox.cpp 1036057 
>   /trunk/KDE/kdelibs/plasma/private/paneltoolbox_p.h 1036057 
>   /trunk/KDE/kdelibs/plasma/private/toolbox.cpp 1036057 
>   /trunk/KDE/kdelibs/plasma/private/toolbox_p.h 1036057 
>   /trunk/KDE/kdelibs/plasma/toolbox.h PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/toolbox.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/1861/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco
> 
>



More information about the Plasma-devel mailing list