Review Request: Some changes to make Amarok appearance more pretty

[Lucas Gomes]

> On Jan. 30, 2012, 8:17 a.m., Bart Cerneels wrote:
> > The screenshots look good. Perhaps some font effects (italics, bold) can be added to highlight channels with new episodes to make it even more intuitive?
> > 
> > Have not take a better look at the code yet. My remark on an early version was that it was quite invasive (new abstract class for Collection and PodcastBrowser) and would prefer if it's mainly changes in the podcast view.

It's a good ideia to add font effects for podcast with new episodes.

I understand your concern, but I done this changes because the overall code design of the way PlaylistBrowser and CollectionBrowser used delegates was too much restrictive. For instance, I couldn't apply a delegate to any of the merged view modes. 

So I created a abstract class (AbstractTreeViewDelegate) and setted PlaylistBrowser/CollectionBrowser to accept two of them, one for the normal view mode and one for the merged view mode, in the initializeView() function.

I also removed some repetitive code related to font creation in these delegates.

In my point of view, these changes were necessary to keep or not extending the code capability, that is something desirable in every piece of code.


- Lucas


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


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/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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120130/d4c6dc2e/attachment.html>