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

[Christoph Feck]

Hello all,

after some thought and research, I see no reason to deprecate or even remove 
groupSpacingHint(). In fact, it is designed to query a value for a single use 
case (space between two groups in a dialog).

However, I changed its internal implementation to not return a fixed value of 
16 pixels, but lineSpacing() value from fontMetrics() of the applications 
default font. This makes it somewhat adaptive to different DPI settings. In 
the future, it could be changed to query the style.

I also noticed that one signal is never emitted (probably was required in Qt 3 
days), if you need to react on style changes, use QEvent::StyleChange.

In my updated patch I also added the "@deprecated" comments for the 
marginHint(), spacingHint(), updateGeometry() and resizeLayout() functions, 
along with recommended replacement calls. Note that except for the 
updateGeometry() call they still work as documented. Calling updateGeometry() 
from outside KDialog is not possible (protected), it probably was only 
internally called by the signal that is never emitted :)

Full patch in attachement. Commiting if no objections.
Christoph (kdepepo)


Am Friday 05 December 2008 03:39:25 schrieb 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: layout.diff
Type: text/x-diff
Size: 8625 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20081212/bfdd1b4a/attachment.diff>