Review Request: New Applet handle system

[Aaron Seigo]

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


the patch does not apply cleanly to trunk; it needs to be updated / regenerated.

instead of putting all of these classes into private/, i think it may make more sense to start a new dir in kdeliba/plasma/ called e.g. handles/, much as we did for animations.

i've also included some comments on ControlElement as a starting point.


trunk/KDE/kdelibs/plasma/private/controlelement.h
<http://svn.reviewboard.kde.org/r/5155/#comment7544>

    this class is in the public API of Applet, so it would need to be marked with PLASMA_EXPORT and moved out of the private/ directory.



trunk/KDE/kdelibs/plasma/private/controlelement.h
<http://svn.reviewboard.kde.org/r/5155/#comment7542>

    will probably require a "UserElement" that is some larger number at the end to allow for custom elements.



trunk/KDE/kdelibs/plasma/private/controlelement.h
<http://svn.reviewboard.kde.org/r/5155/#comment7543>

    needs to marked with the explicity keyword.



trunk/KDE/kdelibs/plasma/private/controlelement.h
<http://svn.reviewboard.kde.org/r/5155/#comment7549>

    since this is a pure virtual, the class should be called AbstractControlElement.



trunk/KDE/kdelibs/plasma/private/controlelement.cpp
<http://svn.reviewboard.kde.org/r/5155/#comment7545>

    i don't think this belongs in ControlElement. it may make sense inside MoveElement, or could even be handled inside of the handle itself if it needs to be generic. it feels out of place here, however, and is only used by the MoveElement implementation.



trunk/KDE/kdelibs/plasma/private/controlelement_p.h
<http://svn.reviewboard.kde.org/r/5155/#comment7546>

    a pointer to the handle shouldn't be necessary.



trunk/KDE/kdelibs/plasma/private/desktophandle.h
<http://svn.reviewboard.kde.org/r/5155/#comment7548>

    this file needs to be a _p.h


- Aaron


On 2010-09-01 16:22:54, Giulio Camuffo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5155/
> -----------------------------------------------------------
> 
> (Updated 2010-09-01 16:22:54)
> 
> 
> Review request for Plasma, Aaron Seigo and Marco Martin.
> 
> 
> Summary
> -------
> 
> This is a rewamp of the Applet handle system. Through its modular architecture it easily allows modifications and reuse of code.
> 
> It features a base Handle class, AbstractHandle, and a base class for the control elements, ControlElement. I developed an handle based on the actual AppletHandle, DesktopHandle, and the control elements for the usual operations.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/plasma/CMakeLists.txt 1170608 
>   trunk/KDE/kdelibs/plasma/applet.h 1170608 
>   trunk/KDE/kdelibs/plasma/applet.cpp 1170608 
>   trunk/KDE/kdelibs/plasma/containment.h 1170608 
>   trunk/KDE/kdelibs/plasma/containment.cpp 1170608 
>   trunk/KDE/kdelibs/plasma/extenders/extender.cpp 1170608 
>   trunk/KDE/kdelibs/plasma/extenders/extenderitem.cpp 1170608 
>   trunk/KDE/kdelibs/plasma/private/abstracthandle.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/abstracthandle.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/applet_p.h 1170608 
>   trunk/KDE/kdelibs/plasma/private/applethandle.cpp 1170608 
>   trunk/KDE/kdelibs/plasma/private/applethandle_p.h 1170608 
>   trunk/KDE/kdelibs/plasma/private/configurecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/configurecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/containment_p.h 1170608 
>   trunk/KDE/kdelibs/plasma/private/controlelement.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/controlelement.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/controlelement_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/desktophandle.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/desktophandle.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/maximizecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/maximizecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/movecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/movecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/removecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/removecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/resizecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/resizecontrol.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/rotatecontrol.h PRE-CREATION 
>   trunk/KDE/kdelibs/plasma/private/rotatecontrol.cpp PRE-CREATION 
> 
> Diff: http://svn.reviewboard.kde.org/r/5155/diff
> 
> 
> Testing
> -------
> 
> It isn't finished. It's missing the touch events management (which, however, it's hard for me to do, 'cause i don't have any touch screen device) and a better drag and drop system between containments. I'd like, however, to know what you think about what i've done, especially about the architecture.
> 
> What's here works, though.
> 
> 
> Thanks,
> 
> Giulio
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20100903/50c9d117/attachment-0001.htm