Review Request: Improved extender theming

Aaron J. Seigo aseigo at kde.org
Sat Oct 18 22:40:31 CEST 2008


On Saturday 18 October 2008, Rob Scheepmaker wrote:
> > > - Added a virtual enabledBorderForItem(ExtenderItem*) to Extender.
> > > Depending on the setting of appearance, and the position of the item,
> > > this returns which background borders to enable.
> >
> > why is it virtual?
>
> So that subclasses of extender can do something sane here. Else it would
> only work if your custom extender uses a vertical layout.

hm.. ok.. i still have my doubts anyone will create custom extenders, but hey 
.. we'll find out =)

> > would be interested in seeing this part of the patch in particular ..
>
> here it is :)

+ ? ? ? ?//FIXME: for some reason the preferred size hint get's set correctly,
+ ? ? ? ?//but the minimumSizeHint doesn't. Investigate why.
+ ? ? ? ?q->setMinimumSize(q->layout()->preferredSize());

that could possibly be because the minimumSize being set is larger than the 
maximumSize?

(yes, it should probably bump up the max size in that case, but iirc it 
doesn't)

+ ? ? ? ?enum Appearance {

is there any use for an AllBorders?


it looks ok on reading, but the patch is a bit harder to read due to also 
including a large amount of changes due to moving the Private class to its own 
header.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Software

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20081018/ea8d898e/attachment.sig 


More information about the Plasma-devel mailing list