[Kde-pim] Review Request: Fix site favicons being squashed together in Akregator's feed list

[Frank Osterfeld]

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

Ship it!


Looks good, commit :)
But is the font part when calculating the height actually needed? Shouldn't it be enough to return the padded icon height there, and let the base class care about the font?

- Frank


On 2009-05-06 09:36:09, Jonathan Marten wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/668/
> -----------------------------------------------------------
> 
> (Updated 2009-05-06 09:36:09)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Add some line spacing between Akregator's feed list rows, so that the site favicons do not run into each other.
> Particularly noticeable in this application, because many sites' favicons are square with no margin.
> 
> Calculate the row height as the maximum of the current icon theme's "Small" size, and the font height
> for the "General" font.  Return this, plus a 1-pixel margin, as the SizeHintRole for the list view model.
> Recalculate this on a font or theme settings change.
> 
> 
> This addresses bug 178821.
>     https://bugs.kde.org/show_bug.cgi?id=178821
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/akregator/src/subscriptionlistmodel.cpp 964216 
>   /trunk/KDE/kdepim/akregator/src/subscriptionlistmodel.h 964216 
>   /trunk/KDE/kdepim/akregator/src/subscriptionlistdelegate.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/akregator/src/CMakeLists.txt 964216 
>   /trunk/KDE/kdepim/akregator/src/subscriptionlistdelegate.h PRE-CREATION 
>   /trunk/KDE/kdepim/akregator/src/subscriptionlistview.cpp 964216 
> 
> Diff: http://reviewboard.kde.org/r/668/diff
> 
> 
> Testing
> -------
> 
> Built and run Akregator application with these changes, checked on a variety of feed sites.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/