Review Request: Plasma::IconWidget: Add support for constrained size hints

[Ingomar Wesp]

> On 2010-10-12 20:19:23, Marco Martin wrote:
> > just looking at the code seems sensible so far, did you tested it extensively? did you find any misbehaviour? 
> > (will do it tomorrow anyways)
> > if it appear to behave well i think it could be tried to make it go in

> did you tested it extensively? did you find any misbehaviour? 

I've played around with a default plasma-desktop and tested the constrained size hint with a local version of the quicklaunch applet that makes use of it. Unfortunately I haven't found the time for a more systematic and thorough testing …

> if it appear to behave well i think it could be tried to make it go in

Ready when you are :)


- Ingomar


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


On 2010-10-11 13:06:24, Ingomar Wesp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5580/
> -----------------------------------------------------------
> 
> (Updated 2010-10-11 13:06:24)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> I poked around the IconWidget class and came up with the following patch that
> implements handling of constraints in sizeHint(...). It basically pulls the 
> code that determines the icon size for a given widget size out of 
> layoutIcons(...) into an the a new iconSizeForWidgetSize(...) method so that 
> it can be reused by sizeHint(...).
> 
> I've also changed the signatures of elidedText(...) and layoutText(...) by 
> removing the need to pass a QStyleOptionGraphicsItem*, since it wasn't used 
> there anyway. If there's some reason why this could be a bad idea, I'll put it 
> back in.
> 
> The patch changes the behavior of sizeHint(...) in the following way:
> 
> - If there is no constraint, the behavior remains unchanged.
> 
> - Likweise, the minimum and maximum sizes are unaffected by constraints. I 
>   don't see how limiting the available width/height would affect either one?
>   
> - The preferred size computation now uses the constraint to compute the 
>   expected size of the icon and adjusts the returned value accordingly.
> 
> I must admit that I'm not entirely happy with the solution as it has (at
> least ;) the following issues:
> 
> - The reported preferredSize is not bounded by the maximumSize. Unfortunately 
>   I can't call maximumSize() and its derivatives from within sizeHint, since
>   this might cause QGraphicsLayoutItem to update all sizeHints, which might in
>   turn lead to unbound recursion, since it apparently also caches the 
>   previously passed constraints :/
>   
>   This is why I'm setting any unconstrained dimension to QWIDGETSIZE_MAX for 
>   the purpose of determining the icon size that fits within. 
> 
> - I'm not very fond of having to initialize and pass around
>   QStyleOptionGraphicsItem objects, especially since they are apparently only 
>   used for determining whether we've got an RTL layout. 
> 
> Any testing / feedback would be greatly appreciated.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/widgets/iconwidget.cpp 1182894 
>   /trunk/KDE/kdelibs/plasma/widgets/iconwidget_p.h 1182894 
> 
> Diff: http://svn.reviewboard.kde.org/r/5580/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ingomar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101012/0ab421ea/attachment.htm