On March 22, 2012, 10:04 p.m., Lucas Gomes wrote:
> > 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?

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?


- Lucas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103603/#review11771
-----------------------------------------------------------


On Jan. 29, 2012, 6:42 p.m., Lucas Gomes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103603/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2012, 6:42 p.m.)
> 
> 
> Review request for Amarok and Bart Cerneels.
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   src/browsers/collectionbrowser/CollectionBrowserTreeView.cpp 35a8222 
>   src/browsers/collectionbrowser/CollectionMergedTreeItemDelegate.h 
> PRE-CREATION 
>   src/browsers/collectionbrowser/CollectionMergedTreeItemDelegate.cpp 
> PRE-CREATION 
>   src/browsers/collectionbrowser/CollectionNormalTreeItemDelegate.h 
> PRE-CREATION 
>   src/browsers/collectionbrowser/CollectionNormalTreeItemDelegate.cpp 
> PRE-CREATION 
>   src/browsers/CollectionTreeItemModelBase.cpp e7f8e62 
>   ChangeLog 70dd420 
>   src/CMakeLists.txt 4241e69 
>   src/browsers/AbstractTreeViewDelegate.h PRE-CREATION 
>   src/browsers/AbstractTreeViewDelegate.cpp PRE-CREATION 
>   src/browsers/collectionbrowser/CollectionTreeItemDelegate.h 8a189e6 
>   src/browsers/collectionbrowser/CollectionTreeItemDelegate.cpp 755be00 
>   src/browsers/collectionbrowser/CollectionWidget.cpp ac1c26d 
>   src/browsers/playlistbrowser/PlaylistBrowserCategory.h 9198d43 
>   src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp 0c2f9c1 
>   src/browsers/playlistbrowser/PlaylistBrowserView.cpp 9c4236d 
>   src/browsers/playlistbrowser/PlaylistMergedTreeItemDelegate.h PRE-CREATION 
>   src/browsers/playlistbrowser/PlaylistMergedTreeItemDelegate.cpp 
> PRE-CREATION 
>   src/browsers/playlistbrowser/PlaylistNormalTreeItemDelegate.h PRE-CREATION 
>   src/browsers/playlistbrowser/PlaylistNormalTreeItemDelegate.cpp 
> PRE-CREATION 
>   src/browsers/playlistbrowser/PlaylistTreeItemDelegate.h 3a094b0 
>   src/browsers/playlistbrowser/PlaylistTreeItemDelegate.cpp bc76551 
>   src/browsers/playlistbrowser/PlaylistsByProviderProxy.h 941268c 
>   src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp 12f2676 
>   src/browsers/playlistbrowser/PlaylistsInFoldersProxy.h 9a01dbe 
>   src/browsers/playlistbrowser/PlaylistsInFoldersProxy.cpp 4268a82 
>   src/browsers/playlistbrowser/PodcastCategory.cpp 1c353dc 
>   src/browsers/playlistbrowser/PodcastMergedTreeItemDelegate.h PRE-CREATION 
>   src/browsers/playlistbrowser/PodcastMergedTreeItemDelegate.cpp PRE-CREATION 
>   src/browsers/playlistbrowser/PodcastModel.h e88f4a1 
>   src/browsers/playlistbrowser/PodcastModel.cpp 18334f6 
>   src/browsers/playlistbrowser/PodcastNormalTreeItemDelegate.h PRE-CREATION 
>   src/browsers/playlistbrowser/PodcastNormalTreeItemDelegate.cpp PRE-CREATION 
>   src/browsers/playlistbrowser/UserPlaylistCategory.cpp b48a55f 
>   src/core-impl/podcasts/sql/SqlPodcastMeta.h 42ad039 
>   src/core-impl/podcasts/sql/SqlPodcastMeta.cpp 1c3bdf4 
>   src/core/podcasts/PodcastMeta.h 679f7ac 
>   src/core/podcasts/PodcastMeta.cpp b9851f7 
>   src/widgets/TrackSelectWidget.cpp 5bd5059 
> 
> Diff: http://git.reviewboard.kde.org/r/103603/diff/
> 
> 
> Testing
> -------
> 
> This patch should build. Everything is working as expected and there aren't 
> any known issues.
> 
> 
> Screenshots
> -----------
> 
> CollectionBrowser
>   http://git.reviewboard.kde.org/r/103603/s/420/
> PodcastBrowser
>   http://git.reviewboard.kde.org/r/103603/s/423/
> 
> 
> Thanks,
> 
> Lucas Gomes
> 
>

_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Reply via email to