<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/103603/">http://git.reviewboard.kde.org/r/103603/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 22nd, 2012, 10:04 p.m., <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;">Could you please explain what are the benefits (direct, current - not hypothetical or philosofical) of the TreeItem -> NormalTreeItem, MergedTreeItem split? Where exactly is avoids code duplication?</pre>
</blockquote>
<p>On June 1st, 2012, 1:58 p.m., <b>Lucas Gomes</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;">Ok, first ignore revision five. In it I've tryed to put the extra info string with the podcast title in Qt::DisplayRole(Look at PodcastModel changes in revision 5), but the extra info won't appear if the PodcastBrowser width isn't bigger enought to show the '\n' character. It's surely the kind of hack that I'd prefer to avoid.
It's not possible to do this without making the use of delegates. I've tryed again yesterday, but with no success. And the problem is that we cannot set a delegate to merged view mode in the current implementation(Look by yourself in PlaylistBrowserCategory).
Also, I have split TreeItems in Normal and Merged because the latter doesn't show the provider item. So the logic is a bit different and it isn't possible to have only one for both view modes if we want to show some extra info(This review proposal).
Well, since we aren't showing track count in the playlist and collection browser anymore, we can simplify it by using a single provider for them. Though I believe that it's best to have separate(normal and merged) classes. Basically, because they'd be already done in case we want to change anything via their delegators.
Anyone has any suggestion about how to achieve the extra info goal in a different/more appropriate manner?</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;">Warning: Past revisions aren't ready yet :o.
Almost forgot. I've found some bugs yesterday too, so I'll wait to know if everyone understands the situation. Then I'll make another revision that's really working as expected. Or do it in another way if someone has an alternative solution.</pre>
<br />
<p>- Lucas</p>
<br />
<p>On January 29th, 2012, 6:42 p.m., Lucas Gomes wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/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 Amarok and Bart Cerneels.</div>
<div>By Lucas Gomes.</div>
<p style="color: grey;"><i>Updated Jan. 29, 2012, 6:42 p.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;">This is my attempt to make QTreeView subclasses items, used in Amarok, more pretty by displaying some extra information. Notice that these extra information are usually the quantity of tracks in a album, the quantity of episodes in a podcast and the quantity of episodes marked as new in a podcast.
So, please help me to improve this feature even more by answering some questions:
1) Should I show the quantity of tracks on playlists listed in PlaylistBrowser too?
2) Is there any extra information that you think it's relevant to be showed somewhere (In QTreeViews)?
Link for my personal repository (Look for ui-improve branch):
http://quickgit.kde.org/index.php?p=clones%2Famarok%2Fgomes%2Fmaskmaster-amarok.git&a=summary
</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;">This patch should build. Everything is working as expected and there aren't any known issues.</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>src/browsers/collectionbrowser/CollectionBrowserTreeView.cpp <span style="color: grey">(35a8222)</span></li>
<li>src/browsers/collectionbrowser/CollectionMergedTreeItemDelegate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/collectionbrowser/CollectionMergedTreeItemDelegate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/collectionbrowser/CollectionNormalTreeItemDelegate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/collectionbrowser/CollectionNormalTreeItemDelegate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/CollectionTreeItemModelBase.cpp <span style="color: grey">(e7f8e62)</span></li>
<li>ChangeLog <span style="color: grey">(70dd420)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(4241e69)</span></li>
<li>src/browsers/AbstractTreeViewDelegate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/AbstractTreeViewDelegate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/collectionbrowser/CollectionTreeItemDelegate.h <span style="color: grey">(8a189e6)</span></li>
<li>src/browsers/collectionbrowser/CollectionTreeItemDelegate.cpp <span style="color: grey">(755be00)</span></li>
<li>src/browsers/collectionbrowser/CollectionWidget.cpp <span style="color: grey">(ac1c26d)</span></li>
<li>src/browsers/playlistbrowser/PlaylistBrowserCategory.h <span style="color: grey">(9198d43)</span></li>
<li>src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp <span style="color: grey">(0c2f9c1)</span></li>
<li>src/browsers/playlistbrowser/PlaylistBrowserView.cpp <span style="color: grey">(9c4236d)</span></li>
<li>src/browsers/playlistbrowser/PlaylistMergedTreeItemDelegate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/playlistbrowser/PlaylistMergedTreeItemDelegate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/playlistbrowser/PlaylistNormalTreeItemDelegate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/playlistbrowser/PlaylistNormalTreeItemDelegate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/playlistbrowser/PlaylistTreeItemDelegate.h <span style="color: grey">(3a094b0)</span></li>
<li>src/browsers/playlistbrowser/PlaylistTreeItemDelegate.cpp <span style="color: grey">(bc76551)</span></li>
<li>src/browsers/playlistbrowser/PlaylistsByProviderProxy.h <span style="color: grey">(941268c)</span></li>
<li>src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp <span style="color: grey">(12f2676)</span></li>
<li>src/browsers/playlistbrowser/PlaylistsInFoldersProxy.h <span style="color: grey">(9a01dbe)</span></li>
<li>src/browsers/playlistbrowser/PlaylistsInFoldersProxy.cpp <span style="color: grey">(4268a82)</span></li>
<li>src/browsers/playlistbrowser/PodcastCategory.cpp <span style="color: grey">(1c353dc)</span></li>
<li>src/browsers/playlistbrowser/PodcastMergedTreeItemDelegate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/playlistbrowser/PodcastMergedTreeItemDelegate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/playlistbrowser/PodcastModel.h <span style="color: grey">(e88f4a1)</span></li>
<li>src/browsers/playlistbrowser/PodcastModel.cpp <span style="color: grey">(18334f6)</span></li>
<li>src/browsers/playlistbrowser/PodcastNormalTreeItemDelegate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/playlistbrowser/PodcastNormalTreeItemDelegate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/playlistbrowser/UserPlaylistCategory.cpp <span style="color: grey">(b48a55f)</span></li>
<li>src/core-impl/podcasts/sql/SqlPodcastMeta.h <span style="color: grey">(42ad039)</span></li>
<li>src/core-impl/podcasts/sql/SqlPodcastMeta.cpp <span style="color: grey">(1c3bdf4)</span></li>
<li>src/core/podcasts/PodcastMeta.h <span style="color: grey">(679f7ac)</span></li>
<li>src/core/podcasts/PodcastMeta.cpp <span style="color: grey">(b9851f7)</span></li>
<li>src/widgets/TrackSelectWidget.cpp <span style="color: grey">(5bd5059)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103603/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/103603/s/420/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/01/27/collectionBrowser_400x100.jpeg" style="border: 1px black solid;" alt="CollectionBrowser" /></a>
<a href="http://git.reviewboard.kde.org/r/103603/s/423/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/01/29/podcastBrowser_400x100.jpeg" style="border: 1px black solid;" alt="PodcastBrowser" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>