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

Jonathan Marten jjm at keelhaul.me.uk
Thu May 7 09:06:42 BST 2009



> On 2009-05-07 00:07:46, Frank Osterfeld wrote:
> > 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?

I'll try that out before committing.  You pointed out some time ago that taking account of the font height was necessary when providing the size hint from the model, but it may not be necessary in the delegate because that can start with QItemDelegate's size hint which already takes account of the font.


- Jonathan


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


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/



More information about the kde-pim mailing list