<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/5155/">http://svn.reviewboard.kde.org/r/5155/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 3rd, 2010, 3:59 p.m., <b>Aaron Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On September 3rd, 2010, 5:40 p.m., <b>Giulio Camuffo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">the patch was done against the last revision. i don't know what to do about it.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">your editor removed spaces at the ends of three lines (one in applet.cpp, one in containment.cpp and one in extender.cpp) but the patch does not show this change in it. not sure exactly what bit is messing up here (i'm guessing the svn diff), but i'll commit with those spaces gone and you should be able to re-generate the diff successfully at that point.
try r1171395 of kdelibs/plasma.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 3rd, 2010, 3:59 p.m., <b>Aaron Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="/r/5155/diff/2/?file=34958#file34958line38" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdelibs/plasma/private/controlelement_p.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class ControlElementPrivate</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">38</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">AbstractHandle</span> <span class="o">*</span><span class="n">handle</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">a pointer to the handle shouldn't be necessary.</pre>
</blockquote>
<p>On September 3rd, 2010, 5:38 p.m., <b>Giulio Camuffo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">the handle should not be required for such implementation specifics:
* the containment comes from whatever creates the element. e.g. the Applet.
* details such as which corner to "anchor" on should probably be sent to the Control by the Handle during mouse events
i think one of the things that makes this difficult right now is that the handle is simply forwarding on mouse events. it would be more useful to know, e.g., if the item is simply triggerable (meaning all it needs to know is that the mouse was pressed and then subsequently released in its bounding rect, in the case of the current handles) or if the item needs to do mouse tracking. in both cases, a triggered call would be made (in the mouse tracking case, on mouse press; in the no-tracking case, on successfully mouse release) and then only mouse movement would need to be forwarded in the mouse tracking case (e.g. resize control). in the mouse tracking case, additional information could be sent, such as the anchor corner, using a subclass of QGraphicsMouseEvent. information about the cause of the triggering (e.g. which mouse button(s)) could be passed to the trigger(..) method.
in the end, the ControlElement and the Handle should be very loosely coupled: it should be possible to change the Handle to a completely different class without touching ControlElement.
</pre>
<br />
<p>- Aaron</p>
<br />
<p>On September 1st, 2010, 4:22 p.m., Giulio Camuffo wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma, Aaron Seigo and Marco Martin.</div>
<div>By Giulio Camuffo.</div>
<p style="color: grey;"><i>Updated 2010-09-01 16:22:54</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>trunk/KDE/kdelibs/plasma/CMakeLists.txt <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/applet.h <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/applet.cpp <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/containment.h <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/containment.cpp <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/extenders/extender.cpp <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/extenders/extenderitem.cpp <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/abstracthandle.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/abstracthandle.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/applet_p.h <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/applethandle.cpp <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/applethandle_p.h <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/configurecontrol.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/configurecontrol.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/containment_p.h <span style="color: grey">(1170608)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/controlelement.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/controlelement.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/controlelement_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/desktophandle.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/desktophandle.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/maximizecontrol.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/maximizecontrol.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/movecontrol.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/movecontrol.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/removecontrol.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/removecontrol.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/resizecontrol.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/resizecontrol.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/rotatecontrol.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdelibs/plasma/private/rotatecontrol.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5155/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>