Review Request: Some changes to make Amarok appearance more pretty

Matěj Laitl matej at laitl.cz
Thu Mar 22 22:04:41 UTC 2012


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



src/browsers/AbstractTreeViewDelegate.h
<http://git.reviewboard.kde.org/r/103603/#comment9334>

    I would be more verbose here:
    
    bigFontMetrics();
    smallFontMetrics();



src/browsers/CollectionTreeItemModelBase.cpp
<http://git.reviewboard.kde.org/r/103603/#comment9336>

    Sorry, I fooled you in a bad way, I wanted to say that the height should be left at 34 pixels (i.e. untouched).


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?

- Matěj Laitl


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

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


More information about the Amarok-devel mailing list