Extender Polishing time! The following problems have been adressed:

* Far more correct spacer implementation. This avoids the spacer jumping around while dragging an item. Also adjustSizeHints is now only called once/spacer move.
* Avoid spacer related memleak.
* Only load extenderItems that are actually detached. This way, attached items won't linger around in case of a plasma crash.
* Use utilities-desktop-extra as icon for items with no saved icon (e.g. items where the icon is set using setIcon(QIcon) instead of setIcon(QString)). Sure beats the questionmark.
* Update mask when offscreen extender items are resized when being dragged to avoid screwed up masks (white borders).
* Start the drag only aften being moved a minimum of QApplication::startDragDistance().
* Correct transformation for calls to showDropZone.
* Use the mouse position for positioning items in extenders or panels, the topleft corner for positioning in a desktop containment. This feels the most natural.
* Move items back to the extender they came from when they're dropped into nowhere.
* Some small code style fixes.



svn path=/trunk/KDE/kdelibs/; revision=890249

extender.cpp

@@ -211,24 +211,27 @@ void Extender::itemHoverEnterEvent(ExtenderItem *item)
 
 void Extender::itemHoverMoveEvent(ExtenderItem *item, const QPointF &pos)
 {
-    int insertIndex = d->insertIndexFromPos(pos);
+    if (d->spacerWidget && d->spacerWidget->geometry().contains(pos)) {
-
-    if ((insertIndex == d->currentSpacerIndex) || (insertIndex == -1)) {
-        //relayouting is resource intensive, so don't do that when not necesarry.
         return;
     }
 
     //Make sure we remove any spacer that might already be in the layout.
-    itemHoverLeaveEvent(item);
+    if (d->spacerWidget) {
+        d->layout->removeItem(d->spacerWidget);
+    }
 
-    d->currentSpacerIndex = insertIndex;
+    int insertIndex = d->insertIndexFromPos(pos);
 
     //Create a widget that functions as spacer, and add that to the layout.
-    QGraphicsWidget *widget = new QGraphicsWidget(this);
+    if (!d->spacerWidget) {
-    widget->setMinimumSize(QSizeF(item->minimumSize().width(), item->size().height()));
+        QGraphicsWidget *widget = new QGraphicsWidget(this);
-    widget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Fixed);
+        widget->setMinimumSize(item->minimumSize());
-    d->spacerWidget = widget;
+        widget->setPreferredSize(item->preferredSize());
-    d->layout->insertItem(insertIndex, widget);
+        widget->setMaximumSize(item->maximumSize());
+        widget->setSizePolicy(item->sizePolicy());
+        d->spacerWidget = widget;
+    }
+    d->layout->insertItem(insertIndex, d->spacerWidget);
 
     //Make sure we remove any 'no detachables' label that might be there, and update the layout.
     d->updateEmptyExtenderLabel();
@@ -357,38 +360,46 @@ void ExtenderPrivate::loadExtenderItems()
 
         bool temporarySourceApplet = false;
 
-        //find the source applet.
+        kDebug() << "applet id = " << applet->id();
-        Corona *corona = applet->containment()->corona();
+        kDebug() << "sourceappletid = " << sourceAppletId;
-        Applet *sourceApplet = 0;
+
-        foreach (Containment *containment, corona->containments()) {
+        if (sourceAppletId != applet->id()) {
-            foreach (Applet *applet, containment->applets()) {
+            //find the source applet.
-                if (applet->id() == sourceAppletId) {
+            Corona *corona = applet->containment()->corona();
-                    sourceApplet = applet;
+            Applet *sourceApplet = 0;
+            foreach (Containment *containment, corona->containments()) {
+                foreach (Applet *applet, containment->applets()) {
+                    if (applet->id() == sourceAppletId) {
+                        sourceApplet = applet;
+                    }
                 }
             }
-        }
 
-        //There is no source applet. We just instantiate one just for the sake of creating
+            //There is no source applet. We just instantiate one just for the sake of creating
-        //detachables.
+            //detachables.
-        if (!sourceApplet) {
+            if (!sourceApplet) {
-            kDebug() << "creating a temporary applet as factory";
+                kDebug() << "creating a temporary applet as factory";
-            sourceApplet = Applet::load(appletName);
+                sourceApplet = Applet::load(appletName);
-            temporarySourceApplet = true;
+                temporarySourceApplet = true;
-            //TODO: maybe add an option to applet to indicate that it shouldn't be deleted after
+                //TODO: maybe add an option to applet to indicate that it shouldn't be deleted after
-            //having used it as factory.
+                //having used it as factory.
-        }
-
-        if (!sourceApplet) {
-            kDebug() << "sourceApplet is null? appletName = " << appletName;
-            kDebug() << "                      extenderItemId = " << extenderItemId;
-        } else {
-            ExtenderItem *item = new ExtenderItem(q, extenderItemId.toInt());
-            item->setName(extenderItemName);
-            sourceApplet->initExtenderItem(item);
-
-            if (temporarySourceApplet) {
-                delete sourceApplet;
             }
+
+            if (!sourceApplet) {
+                kDebug() << "sourceApplet is null? appletName = " << appletName;
+                kDebug() << "                      extenderItemId = " << extenderItemId;
+            } else {
+                ExtenderItem *item = new ExtenderItem(q, extenderItemId.toInt());
+                sourceApplet->initExtenderItem(item);
+
+                if (temporarySourceApplet) {
+                    delete sourceApplet;
+                }
+            }
+        } else {
+            //this entry is still here, probably because plasma crashed, but it isn't detached and
+            //should be reinstantiated. Just delete the entry.
+            cg.deleteGroup(pair.second);
         }
     }
 
@@ -411,9 +422,9 @@ void ExtenderPrivate::adjustSizeHints()
     //FIXME: what happens in this function are some nasty workarounds for a bug in qt4.4's QGL.
     //Alexis has told me they are working on a fix for qt4.5, so this can be removed once the bug
     //has been fixed in Qt.
-    if (q->layout()) {
+    if (layout) {
-        q->layout()->updateGeometry();
+        layout->updateGeometry();
-        q->setMinimumSize(q->layout()->preferredSize());
+        q->setMinimumSize(layout->preferredSize());
     }
 
     if (applet->layout()) {

extenderitem.cpp

@@ -79,14 +79,18 @@ ExtenderItem::ExtenderItem(Extender *hostExtender, uint extenderItemId)
         //The item is new
         dg.writeEntry("sourceAppletPluginName", hostExtender->d->applet->pluginName());
         dg.writeEntry("sourceAppletId", hostExtender->d->applet->id());
-        dg.writeEntry("sourceIconName", hostExtender->d->applet->icon());
+        dg.writeEntry("extenderIconName", hostExtender->d->applet->icon());
         d->sourceApplet = hostExtender->d->applet;
         d->collapseIcon = new IconWidget(KIcon(hostExtender->d->applet->icon()), "", this);
     } else {
         //The item already exists.
         d->name = dg.readEntry("extenderItemName", "");
         d->title = dg.readEntry("extenderTitle", "");
-        d->collapseIcon = new IconWidget(KIcon(dg.readEntry("extenderIconName", "")), "", this);
+        QString iconName = dg.readEntry("extenderIconName", "utilities-desktop-extra");
+        if (iconName.isEmpty()) {
+            iconName = "utilities-desktop-extra";
+        }
+        d->collapseIcon = new IconWidget(KIcon(iconName), "", this);
 
         //Find the sourceapplet.
         Corona *corona = hostExtender->d->applet->containment()->corona();
@@ -126,6 +130,10 @@ ExtenderItem::ExtenderItem(Extender *hostExtender, uint extenderItemId)
 
 ExtenderItem::~ExtenderItem()
 {
+    //make sure the original mousepointer always get's restored.
+    if (d->mouseOver) {
+        QApplication::restoreOverrideCursor();
+    }
     delete d;
 }
 
@@ -470,6 +478,14 @@ void ExtenderItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *opti
     painter->drawPixmap(d->titleRect().topLeft(), pixmap);
 }
 
+void ExtenderItem::moveEvent(QGraphicsSceneMoveEvent *event)
+{
+    if (d->toplevel) {
+        d->toplevel->setSceneRect(sceneBoundingRect());
+        update();
+    }
+}
+
 void ExtenderItem::resizeEvent(QGraphicsSceneResizeEvent *event)
 {
     qreal width = event->newSize().width();
@@ -497,6 +513,12 @@ void ExtenderItem::resizeEvent(QGraphicsSceneResizeEvent *event)
     d->repositionToolbox();
 
     update();
+
+    if (d->toplevel) {
+        d->toplevel->setSceneRect(sceneBoundingRect());
+        d->toplevel->setMask(d->background->mask());
+        update();
+    }
 }
 
 void ExtenderItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
@@ -506,30 +528,33 @@ void ExtenderItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
         return;
     }
 
-    d->mousePressed = true;
-    d->deltaScene = pos();
-    d->mousePos = event->pos().toPoint();
-    d->hostApplet()->raise();
-    setZValue(d->hostApplet()->zValue());
-
-    QPointF mousePos = d->scenePosFromScreenPos(event->screenPos());
-
-    if (!mousePos.isNull()) {
-        d->extender->itemHoverMoveEvent(this, d->extender->mapFromScene(mousePos));
-    }
-
-    d->extender->d->removeExtenderItem(this);
     QApplication::setOverrideCursor(Qt::ClosedHandCursor);
+
+    d->mousePos = event->pos().toPoint();
+    d->deltaScene = pos();
 }
 
 void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
 {
-    if (!d->mousePressed) {
+    if ((d->mousePos - event->pos().toPoint()).manhattanLength()
-        return;
+                       >= QApplication::startDragDistance()) {
+        d->mousePressed = true;
+        //start the drag:
+        //set the zValue to the maximum.
+        d->hostApplet()->raise();
+        setZValue(d->hostApplet()->zValue());
+
+        QPointF mousePos = d->scenePosFromScreenPos(event->screenPos());
+        if (!mousePos.isNull()) {
+            d->extender->itemHoverMoveEvent(this, d->extender->mapFromScene(mousePos));
+        }
+        d->extender->d->removeExtenderItem(this);
+
+        d->themeChanged();
     }
 
-    if (d->background->enabledBorders() != FrameSvg::AllBorders) {
+    if (!d->mousePressed) {
-        d->themeChanged();
+        return;
     }
 
     //keep track of the movement in scene coordinates. we use this to set the position of the
@@ -551,10 +576,10 @@ void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
         if (!d->toplevel) {
             //FIXME: duplication from applethandle
             //create a toplevel view and aim it at the applet.
-            d->toplevel = new QGraphicsView(corona, 0);
-
             corona->addOffscreenWidget(this);
 
+            d->toplevel = new QGraphicsView(scene(), 0);
+
             d->toplevel->setWindowFlags(
                 Qt::ToolTip | Qt::FramelessWindowHint | Qt::WindowStaysOnTopHint);
             d->toplevel->setFrameShape(QFrame::NoFrame);
@@ -575,20 +600,16 @@ void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
             d->toplevel->show();
         }
 
-        //move the toplevel view.
         d->toplevel->setSceneRect(sceneBoundingRect());
         d->toplevel->setGeometry(screenRect);
-        update();
     } else {
         corona->removeOffscreenWidget(this);
         setParentItem(d->hostApplet());
         setPos(d->deltaScene);
 
        //remove the toplevel view.
-       if (d->toplevel) {
+        delete d->toplevel;
-            delete d->toplevel;
+        d->toplevel = 0;
-            d->toplevel = 0;
-        }
     }
 
     //let's insert spacers in applets we're hovering over for some useful visual feedback.
@@ -596,7 +617,6 @@ void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
     //position in scene coordinates (event->scenePos won't work, since it doesn't take in
     //consideration that you're leaving the current view).
     QPointF mousePos = d->scenePosFromScreenPos(event->screenPos());
-    QPointF topleft = d->scenePosFromScreenPos(event->screenPos() - d->mousePos);
 
     //find the extender we're hovering over.
     Extender *targetExtender = 0;
@@ -618,8 +638,8 @@ void ExtenderItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
                 }
             }
 
-            if (containment->sceneBoundingRect().contains(topleft) && !targetExtender) {
+            if (containment->sceneBoundingRect().contains(mousePos) && !targetExtender) {
-                containment->showDropZone(event->screenPos() - d->mousePos);
+                containment->showDropZone(containment->mapFromScene(mousePos).toPoint());
             } else {
                 containment->showDropZone(QPoint());
             }
@@ -696,7 +716,7 @@ void ExtenderItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
 
         //find the extender we're hovering over.
         Extender *targetExtender = 0;
-        Corona *corona = qobject_cast<Corona*>(scene());
+        Corona *corona = qobject_cast<Corona*>(d->extender->d->applet->scene());
 
         corona->removeOffscreenWidget(this);
 
@@ -722,19 +742,38 @@ void ExtenderItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
             }
         } else {
             //apparently, it is not, so instantiate a new ExtenderApplet.
-            //TODO: maybe we alow the user to choose a default extenderapplet.
+            bool extenderCreated = false;
-            kDebug() << "Instantiate a new ExtenderApplet";
+            mousePos = d->scenePosFromScreenPos(event->screenPos());
-            mousePos = d->scenePosFromScreenPos(event->screenPos() - d->mousePos);
             if (!mousePos.isNull()) {
                 foreach (Containment *containment, corona->containments()) {
                     if (containment->sceneBoundingRect().contains(mousePos)) {
+                        kDebug() << "Instantiate a new ExtenderApplet";
+
+                        //when on a desktopcontainment, we use the widgets topleft corner to
+                        //position the applet in the containment, on other containments, we use the
+                        //actual mouse position.
+                        QPointF position;
+                        if (containment->type() == Plasma::Containment::DesktopContainment) {
+                            position = containment->mapFromScene(
+                                       d->scenePosFromScreenPos(event->screenPos() - d->mousePos));
+                        } else {
+                            position = containment->mapFromScene(mousePos);
+                        }
+
                         Applet *applet = containment->addApplet("internal:extender",
                                 QVariantList(),
                                 QRectF(containment->mapFromScene(mousePos), size()));
                         setExtender(applet->d->extender);
+                        extenderCreated = true;
                     }
                 }
             }
+
+            //if no containment is at the position where the item is dropped, just move the item
+            //back to where it came from.
+            if (!extenderCreated) {
+                setExtender(extender());
+            }
         }
 
         QApplication::restoreOverrideCursor();
@@ -995,8 +1034,11 @@ void ExtenderItemPrivate::sourceAppletRemoved()
 
 qreal ExtenderItemPrivate::iconSize()
 {
+    //read the icon size hint, and enforce a minimum size of 16x16
     QSizeF size = dragger->elementSize("hint-preferred-icon-size");
+    size = size.expandedTo(QSizeF(16,16));
 
+    //return the size of the text, if that is larger then the recommended icon size
     Plasma::Theme *theme = Plasma::Theme::defaultTheme();
     QFont font = theme->font(Plasma::Theme::DefaultFont);
     QFontMetrics fm(font);

extenderitem.h

@@ -232,6 +232,7 @@ class PLASMA_EXPORT ExtenderItem : public QGraphicsWidget
     protected:
         void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget);
 
+        void moveEvent(QGraphicsSceneMoveEvent *event);
         void resizeEvent(QGraphicsSceneResizeEvent *event);
 
         void mousePressEvent(QGraphicsSceneMouseEvent *event);

private/extenderapplet.cpp

@@ -27,7 +27,6 @@
 ExtenderApplet::ExtenderApplet(QObject *parent, const QVariantList &args)
     : Plasma::PopupApplet(parent, args)
 {
-    setPopupIcon("utilities-desktop-extra");
 }
 
 ExtenderApplet::~ExtenderApplet()
@@ -36,6 +35,8 @@ ExtenderApplet::~ExtenderApplet()
 
 void ExtenderApplet::init()
 {
+    setPopupIcon("utilities-desktop-extra");
+
     QGraphicsLinearLayout *layout = new QGraphicsLinearLayout(this);
     layout->setSpacing(0);
     setLayout(layout);
@@ -47,7 +48,7 @@ void ExtenderApplet::init()
             this, SLOT(itemDetached(Plasma::ExtenderItem*)));
 
     layout->addItem(extender());
-    updateGeometry();
+    //updateGeometry();
 }
 
 void ExtenderApplet::itemDetached(Plasma::ExtenderItem *)