[PATCH] fix KDialog to use platform native layout spacing and margins

[Parker Coates]

On Thu, Dec 4, 2008 at 18:13, Christoph Feck wrote:
> Am Thursday 04 December 2008 23:39:22 schrieb Friedrich W. H. Kossebau:
>> Hi Christoph,
>>
>> Am Samstag, 29. November 2008, um 20:21 Uhr, schrieb Christoph Feck:
>> > I noticed that KDialog uses its own hard coded sizes for spacing and
>> > margins, which I consider bad, because Qt styles can compute layouts
>> > depending on widget types and pairs, so even a configurable value would
>> > be wrong.
>>
>> Cannot review, at least nothing more than to say the patch itself looks
>> okay superficially. And I'd welcome the described behaviour. :)
>>
>> Instead questions:
>> Q1:
>> Is there a way to also support the property int KDialog::groupSpacingHint()
>> by KStyle and Qt's layout system? It was only recently added to KDE, for
>> 4.2, because the HIG [HIG] advises to use some grouping in dialogs.
>
> If I understand groupSpacingHint() correctly, it should be replaced with a
> call to QApplication::style()->layoutSpacing(QSizePolicy::GroupBox,
> QSizePolicy::GroupBox, Qt::Vertical), but I have no reference if that is the
> intended behaviour, so I did not address it in the patch.
>
> On the other hand, the value returned by groupSpacingHint() cannot be returned
> by KStyle, unless it actually implements the layoutSpacingImplementation()
> function, because it is larger than the regular spacing value.

Hello Christoph,

I was the won who originally lobbied for and added groupSpacingHint().
You can find the whole conversation here [1].

Basically, I saw that the HIG recommended using a vertical space of 16
pixels to break up dialogs, and I wanted to make use of that in my
application. I wasn't too thrilled about hardcoding that size into my
application. My thinking was that if it's going to be a KDE "standard"
there should be an easy and consistent way to get that value. So into
KDialog it went.

I wasn't aware that QLayouts has gotten smarter and that hard coding
margins and spacings was now generally a bad idea. Otherwise, I
probably wouldn't have added groupSpacingHint(). I'm not at all
knowledgeable on styles and other advance UI issues, so if there's a
more logical implementation or location for this function, then feel
free to propose it.

I just checked [2] and it seems that Killbots is currently the only
user of that function, so I think it makes sense to pull it out
quickly before the 4.2 release and it becomes part of the official
API. I'll go back to hard coding the size in Killbots, for the time
being.

Parker

[1] http://markmail.org/message/hjlxbwrotvayeonb
[2] http://lxr.kde.org/ident?i=groupSpacingHint