Review Request: Plasma::IconWidget: Add support for constrained size hints
Marco Martin
notmart at gmail.com
Tue Oct 12 22:19:21 CEST 2010
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5580/#review8102
-----------------------------------------------------------
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
- Marco
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/8774aaa4/attachment.htm
More information about the Plasma-devel
mailing list