Dialog: Simplify handling of min/max width/height changed

When the minimumWidth/Height of the attached Layout of the mainItem
would change. The following events would happen -

- updateMinimumWidth is called
--> results in resizeEvent being called
--> results in syncMainItemToSize
--> results in slots connected to mainItem widthChanged
---> syncMainItemToSize + syncToMainItemSize being called a few more
times. It's not entirely apparent why at thist point.

This kind of logic is quite hard to follow and more importantly because
of the timers in the middle, an extra paint event is called. This means
the user can first see the window resize and then the item getting
resized.

This patch introduces a little bit of code duplication (can be fixed in
future commits) to clearly establish what updateMinimumWidth should be
doing -
* disconnect signals to make sure mainItem's widthChange is not triggered
* update window size + item size + borders
* reposition if required

The repositioning is useful as currently if a dialog becomes wider if
will not reposition itself and will overflow. With this patch we always
make sure the entire dialog is shown.

Minor Point: On testing without the patch the dialog does reposition
itself if it is not already overflowing. I suspect this is kwin moving
the window.

A test called dialog_minWidthHeighRepositioning.qml can be used to see
how the change occurs before and after.

src/plasmaquick/dialog.cpp

@@ -2,6 +2,7 @@
  *   Copyright 2011 Marco Martin <mart@kde.org>                            *
  *   Copyright 2013 Sebastian Kügler <sebas@kde.org>                       *
  *   Copyright 2014 Martin Gräßlin <mgraesslin@kde.org>                    *
+ *   Copyright 2014 Vishesh Handa <vhanda@kde.org>                         *
  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
  *   it under the terms of the GNU General Public License as published by  *
@@ -84,6 +85,14 @@ public:
     void updateMaximumWidth();
     void updateMaximumHeight();
 
+    /**
+     * This function checks the current position of the dialog and repositions
+     * it so that no part of it is not on the screen
+     */
+    void repositionIfOffScreen();
+
+    void slotMainItemSizeChanged();
+
     void syncMainItemToSize();
     void syncToMainItemSize();
     void requestSizeSync(bool delayed = false);
@@ -146,13 +155,6 @@ void DialogPrivate::syncBorders()
 
     if (frameSvgItem->enabledBorders() != (Plasma::FrameSvg::EnabledBorder)borders) {
         frameSvgItem->setEnabledBorders((Plasma::FrameSvg::EnabledBorder)borders);
-
-        if (mainItemLayout) {
-            updateMinimumWidth();
-            updateMinimumHeight();
-            updateMaximumWidth();
-            updateMaximumHeight();
-        }
     }
 }
 
@@ -259,68 +261,137 @@ void DialogPrivate::updateVisibility(bool visible)
 
 void DialogPrivate::updateMinimumWidth()
 {
-    if (mainItemLayout) {
+    Q_ASSERT(mainItem);
-        int oldWidth = q->width();
+    Q_ASSERT(mainItemLayout);
-        q->setMinimumWidth(mainItemLayout.data()->property("minimumWidth").toInt() + frameSvgItem->margins()->left() + frameSvgItem->margins()->right());
-        //Sometimes setMinimumWidth doesn't actually resize: Qt bug?
-        resizeOrigin = DialogPrivate::Window;
-        q->setWidth(qMax(q->width(), q->minimumWidth()));
 
-        if (location == Plasma::Types::RightEdge) {
+    mainItem.data()->disconnect(q);
-            q->setX(q->x() + (oldWidth - q->size().width()));
+
-        }
+    syncBorders();
-    } else {
+    updateTheme();
-        q->setMinimumWidth(-1);
+
+    int minimumWidth = mainItemLayout.data()->property("minimumWidth").toInt();
+    auto margin = frameSvgItem->margins();
+
+    int oldWidth = q->width();
+
+    q->setMinimumWidth(minimumWidth + margin->left() + margin->right());
+    q->setWidth(qMax(q->width(), q->minimumWidth()));
+
+    mainItem.data()->setWidth(q->width() - margin->left() - margin->right());
+    frameSvgItem->setWidth(q->width());
+
+    if (location == Plasma::Types::RightEdge) {
+        q->setX(q->x() + (oldWidth - q->size().width()));
     }
+    repositionIfOffScreen();
+
+    QObject::connect(mainItem.data(), SIGNAL(widthChanged()), q, SLOT(slotMainItemSizeChanged()));
+    QObject::connect(mainItem.data(), SIGNAL(heightChanged()), q, SLOT(slotMainItemSizeChanged()));
 }
 
 void DialogPrivate::updateMinimumHeight()
 {
-    if (mainItemLayout) {
+    Q_ASSERT(mainItem);
-        int oldHeight = q->height();
+    Q_ASSERT(mainItemLayout);
-        q->setMinimumHeight(mainItemLayout.data()->property("minimumHeight").toInt() + frameSvgItem->margins()->top() + frameSvgItem->margins()->bottom());
-        //Sometimes setMinimumHeight doesn't actually resize: Qt bug?
-        resizeOrigin = DialogPrivate::Window;
-        q->setHeight(qMax(q->height(), q->minimumHeight()));
 
-        if (location == Plasma::Types::BottomEdge) {
+    mainItem.data()->disconnect(q);
-            q->setY(q->y() + (oldHeight - q->size().height()));
+
-        }
+    syncBorders();
-    } else {
+    updateTheme();
-        q->setMinimumHeight(-1);
+
+    int minimumHeight = mainItemLayout.data()->property("minimumHeight").toInt();
+    auto margin = frameSvgItem->margins();
+
+    int oldHeight = mainItem.data()->height();
+
+    q->setMinimumHeight(minimumHeight + margin->left() + margin->right());
+    q->setHeight(qMax(q->height(), q->minimumHeight()));
+
+    mainItem.data()->setHeight(q->height() - margin->top() - margin->bottom());
+    frameSvgItem->setHeight(q->height());
+
+    if (location == Plasma::Types::BottomEdge) {
+        q->setY(q->y() + (oldHeight - q->size().height()));
     }
+    repositionIfOffScreen();
+
+    QObject::connect(mainItem.data(), SIGNAL(widthChanged()), q, SLOT(slotMainItemSizeChanged()));
+    QObject::connect(mainItem.data(), SIGNAL(heightChanged()), q, SLOT(slotMainItemSizeChanged()));
 }
 
 void DialogPrivate::updateMaximumWidth()
 {
-    if (mainItemLayout) {
+    Q_ASSERT(mainItem);
-        const int hint = mainItemLayout.data()->property("maximumWidth").toInt();
+    Q_ASSERT(mainItemLayout);
-        resizeOrigin = DialogPrivate::Window;
+
-        if (hint > 0) {
+    mainItem.data()->disconnect(q);
-            q->setMaximumWidth(hint + frameSvgItem->margins()->left() + frameSvgItem->margins()->right());
+
-            q->setWidth(qMin(q->width(), q->maximumWidth()));
+    syncBorders();
-        } else {
+    updateTheme();
-            q->setMaximumWidth(DIALOGSIZE_MAX);
+
-        }
+    int maximumWidth = mainItemLayout.data()->property("maximumWidth").toInt();
-    } else {
+    maximumWidth = maximumWidth ? maximumWidth : DIALOGSIZE_MAX;
-        q->setMaximumWidth(DIALOGSIZE_MAX);
+    auto margin = frameSvgItem->margins();
-    }
+
+    q->setMaximumWidth(maximumWidth + margin->left() + margin->right());
+    q->setWidth(qBound(q->minimumWidth(), q->width(), q->maximumWidth()));
+
+    mainItem.data()->setWidth(q->width() - margin->left() - margin->right());
+    frameSvgItem->setWidth(q->width());
+
+    repositionIfOffScreen();
+
+    QObject::connect(mainItem.data(), SIGNAL(widthChanged()), q, SLOT(slotMainItemSizeChanged()));
+    QObject::connect(mainItem.data(), SIGNAL(heightChanged()), q, SLOT(slotMainItemSizeChanged()));
 }
 
 void DialogPrivate::updateMaximumHeight()
 {
-    if (mainItemLayout) {
+    Q_ASSERT(mainItem);
-        const int hint = mainItemLayout.data()->property("maximumHeight").toInt();
+    Q_ASSERT(mainItemLayout);
-        resizeOrigin = DialogPrivate::Window;
+
-        if (hint > 0) {
+    mainItem.data()->disconnect(q);
-            q->setMaximumHeight(hint + frameSvgItem->margins()->top() + frameSvgItem->margins()->bottom());
+
-            q->setHeight(qMin(q->height(), q->maximumHeight()));
+    syncBorders();
-        } else {
+    updateTheme();
-            q->setMaximumHeight(DIALOGSIZE_MAX);
+
-        }
+    int maximumHeight = mainItemLayout.data()->property("maximumHeight").toInt();
-    } else {
+    maximumHeight = maximumHeight ? maximumHeight : DIALOGSIZE_MAX;
-        q->setMaximumHeight(DIALOGSIZE_MAX);
+    auto margin = frameSvgItem->margins();
+
+    q->setMaximumHeight(maximumHeight + margin->left() + margin->right());
+    q->setHeight(qBound(q->minimumHeight(), q->height(), q->maximumHeight()));
+
+    mainItem.data()->setHeight(q->height() - margin->top() - margin->bottom());
+    frameSvgItem->setHeight(q->height());
+
+    repositionIfOffScreen();
+
+    QObject::connect(mainItem.data(), SIGNAL(widthChanged()), q, SLOT(slotMainItemSizeChanged()));
+    QObject::connect(mainItem.data(), SIGNAL(heightChanged()), q, SLOT(slotMainItemSizeChanged()));
+}
+
+void DialogPrivate::repositionIfOffScreen()
+{
+    const QRect avail = q->screen()->availableGeometry();
+
+    int x = q->x();
+    int y = q->y();
+
+    if (x < avail.left()) {
+        x = avail.left();
+    } else if (x + q->width() > avail.right()) {
+        x = avail.right() - q->width() + 1;
     }
+
+    if (y < avail.top()) {
+        y = avail.top();
+    } else if (y + q->height() > avail.bottom()) {
+        y = avail.bottom() - q->height() + 1;
+    }
+
+    q->setX(x);
+    q->setY(y);
 }
 
 void DialogPrivate::updateInputShape()
@@ -394,6 +465,8 @@ void DialogPrivate::syncToMainItemSize()
         return;
     }
 
+    syncBorders();
+
     const QSize s = QSize(mainItem.data()->width(), mainItem.data()->height()) +
                     QSize(frameSvgItem->margins()->left() + frameSvgItem->margins()->right(),
                           frameSvgItem->margins()->top() + frameSvgItem->margins()->bottom());
@@ -414,7 +487,6 @@ void DialogPrivate::syncToMainItemSize()
         q->resize(s);
     }
 
-    syncBorders();
     mainItem.data()->setX(frameSvgItem->margins()->left());
     mainItem.data()->setY(frameSvgItem->margins()->top());
 
@@ -513,20 +585,9 @@ void Dialog::setMainItem(QQuickItem *mainItem)
             mainItem->setParent(contentItem());
             mainItem->setProperty("parent", QVariant::fromValue(contentItem()));
 
-            if (mainItem->metaObject()->indexOfSignal("widthChanged")) {
+            connect(mainItem, SIGNAL(widthChanged()), this, SLOT(slotMainItemSizeChanged()));
-                connect(mainItem, &QQuickItem::widthChanged, [ = ]() {
+            connect(mainItem, SIGNAL(heightChanged()), this, SLOT(slotMainItemSizeChanged()));
-                    d->resizeOrigin = DialogPrivate::MainItem;
+            d->slotMainItemSizeChanged();
-                    d->syncTimer->start(0);
-                });
-            }
-            if (mainItem->metaObject()->indexOfSignal("heightChanged")) {
-                connect(mainItem, &QQuickItem::heightChanged, [ = ]() {
-                    d->resizeOrigin = DialogPrivate::MainItem;
-                    d->syncTimer->start(0);
-                });
-            }
-            d->resizeOrigin = DialogPrivate::MainItem;
-            d->requestSizeSync();
 
             //Extract the representation's Layout, if any
             QObject *layout = 0;
@@ -569,6 +630,12 @@ void Dialog::setMainItem(QQuickItem *mainItem)
     }
 }
 
+void DialogPrivate::slotMainItemSizeChanged()
+{
+    resizeOrigin = DialogPrivate::MainItem;
+    syncTimer->start(0);
+}
+
 QQuickItem *Dialog::visualParent() const
 {
     return d->visualParent.data();
@@ -754,19 +821,9 @@ void Dialog::adjustGeometry(const QRect &geom)
     setGeometry(geom);
 }
 
-void Dialog::resizeEvent(QResizeEvent *re)
+void Dialog::resizeEvent(QResizeEvent* re)
 {
-    d->frameSvgItem->setX(0);
-    d->frameSvgItem->setY(0);
-    d->frameSvgItem->setWidth(re->size().width());
-    d->frameSvgItem->setHeight(re->size().height());
-
     QQuickWindow::resizeEvent(re);
-
-    if (d->resizeOrigin == DialogPrivate::Undefined) {
-        d->resizeOrigin = DialogPrivate::Window;
-    }
-    d->requestSizeSync(true);
 }
 
 void Dialog::setType(WindowType type)

src/plasmaquick/dialog.h

@@ -222,6 +222,8 @@ private:
     Q_PRIVATE_SLOT(d, void updateMinimumHeight())
     Q_PRIVATE_SLOT(d, void updateMaximumWidth())
     Q_PRIVATE_SLOT(d, void updateMaximumHeight())
+
+    Q_PRIVATE_SLOT(d, void slotMainItemSizeChanged())
 };
 
 }