Review Request: New Applet handle system

[Giulio Camuffo]

> On 2010-09-03 15:59:17, Aaron Seigo wrote:
> > 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.

the patch was done against the last revision. i don't know what to do about it.


> On 2010-09-03 15:59:17, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/private/controlelement.cpp, lines 70-83
> > <http://svn.reviewboard.kde.org/r/5155/diff/2/?file=34957#file34957line70>
> >
> >     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.

agreed. as i moved it in ControlElement i realised it can really go in MoveControl.


> On 2010-09-03 15:59:17, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/private/controlelement_p.h, line 38
> > <http://svn.reviewboard.kde.org/r/5155/diff/2/?file=34958#file34958line38>
> >
> >     a pointer to the handle shouldn't be necessary.

but the handle provides a pointer to the containment and some things like position(). The ResizeControl must ask the handle to know which corner to keep static.


> On 2010-09-03 15:59:17, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/private/desktophandle.h, line 1
> > <http://svn.reviewboard.kde.org/r/5155/diff/2/?file=34959#file34959line1>
> >
> >     this file needs to be a _p.h

just, there's already a _p.h. I thought that since it will become public it can stay as it is for now.


- Giulio


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


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/5312d68d/attachment-0001.htm