Review Request: reasonable applets size in bigger (probably vertical) panels

[Aaron J. Seigo]

On Sunday 03 February 2008, Marco Martin wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://matt.rogers.name/r/49/
> -----------------------------------------------------------
>
> Review request for Plasma.

unfortunately this has already been submitted so i can't coment on review 
request anymore, but this patch is incorrect in several interesting ways. =)

first the obvious stuff:

* this will not work in any applet which reimplements contraintsUpdated since 
Applet::constraintsUpdated won't be called. instead, such code should go into 
flushUpdatedConstraints (there's already some stuff like there in there) i'll 
add a comment in Applet::constraintsUpdated so that this doesn't get repeated 
in the future, as i can see how it's easy to end up with such a bug as it's 
not immediately obvious.

in this particular case, one could try and say that any applet that 
re-implements constraintsUpdated() shouldn't have its size changed ... but 
that's simply not true. it is perfectly valid for an applet to reimplement 
constraintsUpdated() for reasons that have nothing to do with form factor or 
geometry changes.

* the patch violated the style guide in a few places due to lacking 
whitespace.

* this is still going to result in gigantic icons for applets such as the 
battery in vertical applets due to the check for d->square. not great. i do 
see that it would break in certain situations, so i've fixed 
contentSizeHint() for squaring, taking maximumContentSize into consideration.

now the non-obvious stuff that we probably need to discuss:

* i now have tiny little icons for, e.g. the device notifier. ditto for the 
kickoff button. but for other icons. this new inconsistency, dependant on 
d->square, is not great.

* the code is not run if d->square, but d->square impacts 
expandingDirections() so isn't necessary as a check. an update of the widget 
*is* necessary however.

these two items can be resolved by simply removing the check for d->square. 
that, however, leads to a new world of hurt.

* why rely on the size of an icon for this? what about applets which aren't 
icons and have no relation to them? putting the code in 
flushUpdatedConstraints higlights this problem very nicely with, e.g., the 
pager which is now cut off and the battery applet which is now squished up.

* the resulting icons in Horizontal layouts is overly tiny.

so ... looking at the problem some more ... it seems that the real issue is 
two fold:

* it only applies to certain classes of applets which should conserve space.
* it is commonly related to the size of icons, but probably to any item which 
behaves similarly (e.g. a fixed representation that simply grows to fill the 
space)
* it doesn't really apply when the size of the constrained containment has 
a "reasonable" height:width ratio. it's when the containment becomes more 
square that the problem really shows up.

so a proper solution could take one of two forms:

* doing the right thing in each affected applet. this has the upside of always 
being right and the downside of requiring that we make the adjust in each 
applet.

* provide a way for applets to advertise themselves needing help with 
conserving space, doing so only when constrained and above a certain h/w 
ratio threshhold and using some applet specified base-line (which for icons 
would use IconSize)

i'm still puzzling over what the exact solution should be here. probably not 
helped by my 19 hours of flights across 11 tz yesterday ;)

-- 
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 Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20080204/9c13e024/attachment.pgp