<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://git.reviewboard.kde.org/r/108906/">http://git.reviewboard.kde.org/r/108906/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 12th, 2013, 2:49 p.m. UTC, <b>Matěj Laitl</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;">I don't think creating a custom subclass is the right way to implement drag & drop support. The right way should be implementing QAbstractItemModel for the queue (with drag&drop methods) and then the GUI would automatically support it with none or minimal changes. Similar to what Bart said, we prefer using the .ui files as much as possible. Also, when the QAbstractItemModel approach is used, there should be no need to split the patch into 2 review requests.</pre>
</blockquote>
<p>On February 12th, 2013, 4:40 p.m. UTC, <b>Bart Cerneels</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;">I didn't even consider what the patch is actually supposed to do. Not completely sure that a QAIM subclass is the best option. D'n'D for moving is a bit harder then regular dropMimeData. But a model for a list of simple list of tracks that can be ordered like that could be reused a lot, even after the big PlaylistQueue refactor.</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;">> Not completely sure that a QAIM subclass is the best option. D'n'D for moving is a bit harder then regular dropMimeData.
I still consider implementing QAIM much nicer than reinventing the wheel and implementing the drag & drop at the QWidget level.</pre>
<br />
<p>- Matěj</p>
<br />
<p>On February 11th, 2013, 10:43 a.m. UTC, Bartosz Szreder wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok.</div>
<div>By Bartosz Szreder.</div>
<p style="color: grey;"><i>Updated Feb. 11, 2013, 10:43 a.m.</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;">The new ListWidget (PlaylistQueueEditorListWidget - feeling enterprisey today) supports drag'n'drop of self-contained ListItems to itself. On successful dropEvent a signal is emitted, which is intercepted by a new slot reloadQueue() in PlaylistQueueEditor. This slot in turn recreates the queue via The::playlistActions().
Things to consider in future:
- dragging from the playlist into the queue editor
- making the whole operation more efficient than clearing and recreating whole queue on each dropEvent
- possibly less hacky way of implementing things? maybe some backend interface would need to be added/exposed for the queue editor? any advice from a seasoned Amarok developer would be very valuable
If this isn't going to be accepted as-is into Amarok codebase then I'm willing to put some time into reworking the solution.</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;">Tested on two different installations, no bugs, regressions or instabilities noticed.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=263563">263563</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/CMakeLists.txt <span style="color: grey">(043dc64)</span></li>
<li>src/playlist/PlaylistQueueEditor.h <span style="color: grey">(40b8cdf)</span></li>
<li>src/playlist/PlaylistQueueEditor.cpp <span style="color: grey">(f647e37)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108906/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>