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

Christoph Feck christoph at maxiom.de
Thu Dec 4 23:13:30 GMT 2008


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.

> Q2:
> If the Q*Layouts now use the style defaults, is there a need for the
> KDialog::*Hints() at all? So should these functions be better set to
> Deprecated? And groupSpacingHint() be removed again, before the 4.2
> release? And should all usages of these functions then be removed, as they
> only reset the margins/spacings to values they already have?

Yes, I would deprecate them, unless someone has a vision for a layouting 
system that should replace Qt's dedicated layouting. But I did not feel in 
the position to post a patch that removes functionality that was added for 
4.2 :)

Qt itself deprecated the QLayout::setMargin() function, and for the spacing, 
it has some compat stuff for applications that are not style aware.

> If you get no real answer to your patch here, perhaps you can go for some
> people more aesthetically interested and experienced with KStyle code in
> the mailinglist kde-artists. :)

Ah yes, thanks. I am not sure, but someone on IRC suggested me to post patches 
to this list. I already noticed that this one is mostly "core" and not "ui" 
related :)

Thanks,
Christoph Feck (kdepepo)




More information about the kde-core-devel mailing list