[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    [PATCH] fix KDialog to use platform native layout spacing and margins
From:       Christoph Feck <christoph () maxiom ! de>
Date:       2008-11-29 19:21:14
Message-ID: 200811292021.15901.christoph () maxiom ! de
[Download RAW message or body]

Hello,

this is my first post to this list, so please be gentle with my mistakes :)

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.

This patch should "move" those fixed values to KStyle, meaning any style 
automatically inherits the KDE defaults (should they ever change).

Additionally, KDialog no longer sets these attributes on it's TopLayout, so 
that it can respect the style's layoutSpacingImplementation() method, which 
results in better platform native appearance e.g. on QMacStyle.

I already posted this to kde.bugs.org as bug #176473, since I was not 
subscribed to this list before, and am looking for someone who could review 
this patch. But I am adding it here, too, as I do not know if you prefer it 
here.

Thanks,
Christoph Feck (kdepepo on #kde-devel)

["layout.diff" (text/x-diff)]

Index: kernel/kstyle.cpp
===================================================================
--- kernel/kstyle.cpp	(Revision 890630)
+++ kernel/kstyle.cpp	(Arbeitskopie)
@@ -141,6 +141,8 @@
 {
     //Set up some default metrics...
     setWidgetLayoutProp(WT_Generic, Generic::DefaultFrameWidth, 2);
+    setWidgetLayoutProp(WT_Generic, Generic::DefaultLayoutSpacing, 6);
+    setWidgetLayoutProp(WT_Generic, Generic::DefaultLayoutMargin, 9);

     setWidgetLayoutProp(WT_PushButton, PushButton::ContentsMargin, 5);
     setWidgetLayoutProp(WT_PushButton, PushButton::FocusMargin,    3);
@@ -2439,6 +2441,33 @@
             else
                 return widgetLayoutProp(WT_Generic, Generic::DefaultFrameWidth, \
option, widget);

+        case PM_DefaultChildMargin:
+        case PM_DefaultTopLevelMargin:
+            return widgetLayoutProp(WT_Generic, Generic::DefaultLayoutMargin, \
option, widget); +
+        case PM_LayoutHorizontalSpacing:
+        case PM_LayoutVerticalSpacing:
+            // use layoutSpacingImplementation
+            return -1;
+
+        case PM_DefaultLayoutSpacing:
+            return widgetLayoutProp(WT_Generic, Generic::DefaultLayoutSpacing, \
option, widget); +
+        case PM_LayoutLeftMargin:
+        case PM_LayoutTopMargin:
+        case PM_LayoutRightMargin:
+        case PM_LayoutBottomMargin:
+        {
+            PixelMetric marginMetric;
+            if ((option && (option->state & QStyle::State_Window))
+                || (widget && widget->isWindow())) {
+                marginMetric = PM_DefaultTopLevelMargin;
+            } else {
+                marginMetric = PM_DefaultChildMargin;
+            }
+            return pixelMetric(marginMetric, option, widget);
+        }
+
         case PM_ButtonMargin:
             return 0; //Better not return anything here since we already
             //incorporated this into SE_PushButtonContents
@@ -2627,6 +2656,14 @@
     return QCommonStyle::pixelMetric(metric, option, widget);
 }

+int KStyle::layoutSpacingImplementation(QSizePolicy::ControlType control1, \
QSizePolicy::ControlType control2, Qt::Orientation orientation, const QStyleOption \
*option, const QWidget *widget) const +{
+    Q_UNUSED(control1); Q_UNUSED(control2); Q_UNUSED(orientation);
+
+    return pixelMetric(PM_DefaultLayoutSpacing, option, widget);
+}
+
+
 bool KStyle::isVerticalTab(const QStyleOptionTab* tbOpt) const
 {
     switch (tbOpt->shape)
Index: kernel/kstyle.h
===================================================================
--- kernel/kstyle.h	(Revision 890628)
+++ kernel/kstyle.h	(Arbeitskopie)
@@ -463,7 +463,9 @@
          */
         enum LayoutProp
         {
-            DefaultFrameWidth   ///< The FrameWidth used by LineEdit, etc..., \
default is \b 2 [sets QStyle::PM_DefaultFrameWidth] +            DefaultFrameWidth,   \
///< The FrameWidth used by LineEdit, etc..., default is \b 2 [sets \
QStyle::PM_DefaultFrameWidth] +            DefaultLayoutSpacing, ///< The spacing \
used by layouts, unless the style implements layoutSpacingImplementation(), default \
is \b 6 [sets QStyle::PM_DefaultLayoutSpacing] +            DefaultLayoutMargin   \
///< The margin used by layouts, default is \b 9 [sets QStyle::PM_DefaultChildMargin \
and QStyle::PM_DefaultTopLevelMargin]  };

         /**
@@ -1620,6 +1622,11 @@
     QPixmap generatedIconPixmap(QIcon::Mode iconMode, const QPixmap &pixmap,
                                    const QStyleOption *opt) const;
     bool eventFilter(QObject *, QEvent *);
+
+protected Q_SLOTS:
+    int layoutSpacingImplementation(QSizePolicy::ControlType control1,
+                    QSizePolicy::ControlType control2, Qt::Orientation orientation,
+                    const QStyleOption *option, const QWidget *widget) const;
 //@}
 private:
     KStylePrivate * const d;
Index: dialogs/kdialog_p.h
===================================================================
--- dialogs/kdialog_p.h	(Revision 890628)
+++ dialogs/kdialog_p.h	(Arbeitskopie)
@@ -76,8 +76,6 @@
         QHash<int, KPushButton*> mButtonList;
         QSignalMapper mButtonSignalMapper;

-        static int mMarginSize;
-        static int mSpacingSize;
         static int mGroupSpacingSize;

     protected Q_SLOTS:
Index: dialogs/kdialog.cpp
===================================================================
--- dialogs/kdialog.cpp	(Revision 890628)
+++ dialogs/kdialog.cpp	(Arbeitskopie)
@@ -32,6 +32,7 @@
 #include <QHBoxLayout>
 #include <QHideEvent>
 #include <QPointer>
+#include <QStyle>
 #include <QTimer>
 #include <QVBoxLayout>
 #include <QWhatsThis>
@@ -49,8 +50,6 @@
 #include <netwm.h>
 #endif

-int KDialogPrivate::mMarginSize = 9;
-int KDialogPrivate::mSpacingSize = 6;
 int KDialogPrivate::mGroupSpacingSize = 16;

 void KDialogPrivate::setupLayout()
@@ -78,9 +77,6 @@
   else
         mTopLayout = new QHBoxLayout(q);

-  mTopLayout->setMargin( KDialog::marginHint() );
-  mTopLayout->setSpacing( KDialog::spacingHint() );
-
   if ( mUrlHelp )
     mTopLayout->addWidget( mUrlHelp, 0, Qt::AlignRight );

@@ -395,12 +391,12 @@

 int KDialog::marginHint()
 {
-    return KDialogPrivate::mMarginSize;
+    return QApplication::style()->pixelMetric( QStyle::PM_DefaultTopLevelMargin );
 }

 int KDialog::spacingHint()
 {
-    return KDialogPrivate::mSpacingSize;
+    return QApplication::style()->pixelMetric( QStyle::PM_DefaultLayoutSpacing );
 }

 int KDialog::groupSpacingHint()



[prev in list] [next in list] [prev in thread] [next in thread]