Created an attachment (id=15811) [details]
User interaction with UMLWidget improvements

As you can see, it's a "big" patch. When I first began coding it, I kept the
existent code. But then I thought that it needed a refactoring. So I added the
new features and also redesigned this part of the code to, in my humble
opinion, a more maintenable and easier to understand code. Every method and
class is commented to help understanding it.

User interaction with UMLWidgets seemed as a complex enough matter to deserve a
class on its own, so I made it. I copied the old code and began refactoring it.
I've also wrote some parts from scratch. Some things where also modified
outside UMLWidget where it was needed. I think that code documentation is
complete enough to not to need to explain it here :) But if there's something
you want me to explain just ask.

In brief, user interaction in UMLWidget is handled in UMLWidgetController. If a
widget needs an specific behaviour (resize only in X axis, for example), a
subclass of UMLWidgetController must be made. There are some virtual protected
methods which are used by the events handler (a Template Design Pattern). In
most cases, override those protected methods will suffice.

The patch, although at this moment it works, has some pending tasks. Those are
marked with comments starting with "TODO" (without quotes). Those TODO tasks
are, in most cases, code copied from the old one that I don't know what it
does. There are also pending tasks such as refactor some code, or fix a method
that stopped working in UMLView (which, by the way, isn't used anywhere... so I
think it could be removed). I upload the patch in this state because I don't
know when I'll have time to work on it again (so maybe someone can finish it)
and because, as said, it already works (not taking into account the bugs :) ).

If I can suggest some things:
-Rename FloatingText to FloatingTextWidget, because it's the only child of
UMLWidget with a name not ending in Widget.
-Move all the widgets classes from umbrello/ to umbrello/widget/, because they
have a common purpose and it'd help to keep the files organized.
-In MessageWidget, methods getMinHeight and getMaxHeight, shouldn't they be
called getMinY and getMaxY? (I may be wrong, but I think they return a
position, not a height nor a width)

SVN commit 535784 by okellogg:

Apply attachment 15811 [details] from Daniel Calviño Sánchez, also incorporating his
suggestions of renaming MessageWidget::get{Min,Max}Height to get{Min,Max}Y
and renaming class FloatingText to FloatingTextWidget.
Thanks again for your contributions Daniel!
BUG:126391


 M  +1 -0      ChangeLog  
 M  +6 -1      umbrello/Makefile.am  
 M  +22 -14    umbrello/activitywidget.cpp  
 M  +6 -8      umbrello/activitywidget.h  
 M  +57 -57    umbrello/associationwidget.cpp  
 M  +44 -44    umbrello/associationwidget.h  
 M  +2 -2      umbrello/clipboard/umlclipboard.cpp  
 D             umbrello/floatingtext.cpp  
 D             umbrello/floatingtext.h  
 A             umbrello/floatingtextwidget.cpp   umbrello/floatingtext.cpp#535774 [License: GPL (v2+)]
 A             umbrello/floatingtextwidget.h   umbrello/floatingtext.h#535774 [License: GPL (v2+)]
 M  +19 -19    umbrello/linkwidget.h  
 M  +2 -2      umbrello/listpopupmenu.cpp  
 M  +35 -100   umbrello/messagewidget.cpp  
 M  +26 -33    umbrello/messagewidget.h  
 M  +4 -31     umbrello/notewidget.cpp  
 M  +4 -20     umbrello/notewidget.h  
 M  +5 -25     umbrello/objectwidget.cpp  
 M  +2 -10     umbrello/objectwidget.h  
 M  +27 -16    umbrello/statewidget.cpp  
 M  +10 -11    umbrello/statewidget.h  
 M  +1 -1      umbrello/toolbarstateassociation.cpp  
 M  +2 -2      umbrello/toolbarstatemessages.cpp  
 M  +3 -3      umbrello/toolbarstateother.cpp  
 M  +5 -0      umbrello/uml.cpp  
 M  +7 -0      umbrello/uml.h  
 M  +48 -45    umbrello/umlview.cpp  
 M  +13 -6     umbrello/umlview.h  
 M  +34 -259   umbrello/umlwidget.cpp  
 M  +31 -37    umbrello/umlwidget.h  

In commit 535784 all the widget controllers are missed! So it doesn't even
compile.

Maybe you applied my patch in your SVN tree and forgot to use "svn add" with
the new files? Only an idea... :)

SVN commit 535937 by okellogg:

Add files forgotten in commit 535784. Thanks Daniel for noticing.
BUG:126391


 A             floatingtextwidgetcontroller.cpp   [License: GPL (v2+)]
 A             floatingtextwidgetcontroller.h   [License: GPL (v2+)]
 A             messagewidgetcontroller.cpp   [License: GPL (v2+)]
 A             messagewidgetcontroller.h   [License: GPL (v2+)]
 A             notewidgetcontroller.cpp   [License: GPL (v2+)]
 A             notewidgetcontroller.h   [License: GPL (v2+)]
 A             objectwidgetcontroller.cpp   [License: GPL (v2+)]
 A             objectwidgetcontroller.h   [License: GPL (v2+)]
 A             umlwidgetcontroller.cpp   [License: GPL (v2+)]
 A             umlwidgetcontroller.h   [License: GPL (v2+)]

Need to reopen this because of the new bugs mentioned initially by Daniel.
Achim Spangler reports much deteriorated responsiveness with large models
and complex diagrams. Also there seem to be some quirks with multiple
selection.

SVN commit 580562 by okellogg:

Apply FixSelectionsPartly.diff by Achim Spangler as described at
 http://www.geeksoc.org/~jr/umbrello/uml-devel/9729.html
Many thanks Achim for your bug hunting.
CCBUG:126391


 M  +2 -2      associationwidget.cpp  
 M  +1 -1      messagewidget.cpp  
 M  +23 -9     toolbarstate.cpp  
 M  +28 -13    umlview.cpp  
 M  +7 -6      umlview.h  
 M  +5 -18     umlwidget.cpp  
 M  +5 -4      umlwidgetcontroller.cpp  


--- branches/KDE/3.5/kdesdk/umbrello/umbrello/associationwidget.cpp #580561:580562
@@ -2511,7 +2511,7 @@
         if( x - BOUNDARY <= p.x() && x + BOUNDARY >= p.x() &&
                 y - BOUNDARY <= p.y() && y + BOUNDARY >= p.y() ) {
             m_nMovingPoint = i;
-            i = size; //no need to check the rest
+            break; //no need to check the rest
         }//end if
     }//end for
 }
@@ -2532,7 +2532,7 @@
         m_LinePath.insertPoint( i + 1, me->pos() );
         m_nMovingPoint = i + 1;
     }
-    
+
     setSelected();
     //new position for point
     QPoint p = me->pos();
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/messagewidget.cpp #580561:580562
@@ -375,7 +375,7 @@
     calculateWidget();
     if( !m_pFText )
         return;
-    if( m_pView -> getSelectCount() > 1 )
+    if (m_pView->getSelectCount(true) > 1)
         return;
     setTextPosition();
 }
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/toolbarstate.cpp #580561:580562
@@ -1,8 +1,3 @@
-/*
- *  copyright (C) 2004
- *  Umbrello UML Modeller Authors <uml-devel@ uml.sf.net>
- */
-
 /***************************************************************************
  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
@@ -10,11 +5,17 @@
  *   the Free Software Foundation; either version 2 of the License, or     *
  *   (at your option) any later version.                                   *
  *                                                                         *
+ *  copyright (C) 2004-2006                                                *
+ *  Umbrello UML Modeller Authors <uml-devel@ uml.sf.net>                  *
  ***************************************************************************/
+
+// own header
+#include "toolbarstate.h"
+// qt/kde includes
 #include <qwmatrix.h> // need for inverseWorldMatrix.map
 #include <qevent.h>
-
-#include "toolbarstate.h"
+#include <kdebug.h>
+// app includes
 #include "umlview.h"
 #include "umlwidget.h"
 #include "messagewidget.h"
@@ -166,7 +167,6 @@
 bool ToolBarState::setSelectedWidget(QMouseEvent * me)
 {
     m_pUMLView->setMoveAssoc(NULL);
-    m_pUMLView->setOnWidget(NULL);
 
     // Check associations.
     AssociationWidgetListIt assoc_it(m_pUMLView->getAssociationList());
@@ -188,6 +188,11 @@
     for (MessageWidgetListIt mit(m_pUMLView->getMessageList()); mit.current(); ++mit) {
         MessageWidget *obj = mit.current();
         if (obj->isVisible() && obj->onWidget(me->pos())) {
+            UMLWidget *bkgnd = m_pUMLView->getOnWidget();
+            if (bkgnd && obj != bkgnd) {   // change of selected object
+                bkgnd->setSelected(false);
+                m_pUMLView->setOnWidget(NULL);
+            }
             m_pUMLView->setOnWidget( obj );
             obj->mousePressEvent( me );
             m_bWidgetSelected = true;
@@ -211,14 +216,23 @@
         }
     }
     if (smallestObj) {
+        UMLWidget *bkgnd = m_pUMLView->getOnWidget();
+        if (bkgnd && smallestObj != bkgnd) {   // change of selected object
+            bkgnd->setSelected(false);
+            m_pUMLView->setOnWidget(NULL);
+        }
         m_pUMLView->setOnWidget(smallestObj);
         smallestObj->mousePressEvent(me);
         m_bWidgetSelected = true;
         return true;
     }
 
-    m_pUMLView->setOnWidget(NULL);
+    if (m_pUMLView->getOnWidget()) {   // clear selected object
+        m_pUMLView->getOnWidget()->setSelected(false);
+        m_pUMLView->setOnWidget(NULL);
+    }
 
     m_bWidgetSelected = false;
     return false;
 }
+
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlview.cpp #580561:580562
@@ -238,12 +238,12 @@
     int marginY = pPrinter->margins().height();
 
     if(pPrinter->orientation() == KPrinter::Landscape) {
-      // we are printing in LANDSCAPE --> swap marginX and marginY
-      // this is needed, as KPrinter reports width margin strictly as printer
-      // default orientation margin - and not depend on LANDSCAPE/PORTRAIT
-      int temp = marginX;
-      marginX = marginY;
-      marginY = temp;
+        // we are printing in LANDSCAPE --> swap marginX and marginY
+        // this is needed, as KPrinter reports width margin strictly as printer
+        // default orientation margin - and not depend on LANDSCAPE/PORTRAIT
+        int temp = marginX;
+        marginX = marginY;
+        marginY = temp;
     }
 
     // The printer will probably use a different font with different font metrics,
@@ -1408,16 +1408,31 @@
     }//end while
 }
 
+int UMLView::getSelectCount(bool filterText) const {
+    if (!filterText)
+        return m_SelectedList.count();
+    int counter = 0;
+    const UMLWidget * temp = 0;
+    for (UMLWidgetListIt iter(m_SelectedList); (temp = iter.current()) != 0; ++iter) {
+        if (temp->getBaseType() == wt_Text) {
+            const FloatingTextWidget *ft = static_cast<const FloatingTextWidget*>(temp);
+            if (ft->getRole() == tr_Floating)
+                counter++;
+        } else {
+            counter++;
+        }
+    }
+    return counter;
+}
 
+
 bool UMLView::getSelectedWidgets(UMLWidgetList &WidgetList, bool filterText /*= true*/) {
-    UMLWidget * temp = 0;
-    int type;
-    for(temp=(UMLWidget *)m_SelectedList.first();temp;temp=(UMLWidget *)m_SelectedList.next()) {
-        type = temp->getBaseType();
-        if (filterText && type == wt_Text) {
-            if( ((FloatingTextWidget*)temp)->getRole() == tr_Floating ) {
+    const UMLWidget * temp = 0;
+    for (UMLWidgetListIt it(m_SelectedList); (temp = it.current()) != NULL; ++it) {
+        if (filterText && temp->getBaseType() == wt_Text) {
+            const FloatingTextWidget *ft = static_cast<const FloatingTextWidget*>(temp);
+            if (ft->getRole() == tr_Floating)
                 WidgetList.append(temp);
-            }
         } else {
             WidgetList.append(temp);
         }
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlview.h #580561:580562
@@ -451,11 +451,12 @@
     /**
      * Return the amount of widgets selected.
      *
-     * @return Return the amount of widgets selected.
+     * @param filterText  When true, do NOT count floating text widgets that
+     *                    belong to other widgets (i.e. only count tr_Floating.)
+     *                    Default: Count all widgets.
+     * @return  Number of widgets selected.
      */
-    int getSelectCount() const {
-        return m_SelectedList.count();
-    }
+    int getSelectCount(bool filterText = false) const;
 
     /**
      * Set the useFillColor variable to all selected widgets
@@ -1127,7 +1128,7 @@
      */
     QRect getDiagramRect();
 
-    
+
     /**
      * Initializes key variables.
      */
@@ -1225,7 +1226,7 @@
      * Reset to false when clicking in an empty region of the view.
      */
     bool m_bChildDisplayedDoc;
-    
+
     ToolBarStateFactory* m_pToolBarStateFactory;
     ToolBarState* m_pToolBarState;
 
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlwidget.cpp #580561:580562
@@ -609,30 +609,17 @@
     slotRemovePopupMenu();
 
     //if in a multi- selection to a specific m_pMenu for that
-    int count = m_pView -> getSelectCount();
+    // NEW: ask UMLView to count ONLY the widgets and not their floatingtextwidgets
+    int count = m_pView->getSelectCount(true);
     //a MessageWidget when selected will select its text widget and vice versa
     //so take that into account for popup menu.
 
     // determine multi state
-    bool multi = false;
+    bool multi = (m_bSelected && count > 1);
 
     // if multiple selected items have the same type
     bool unique = false;
 
-    if( m_bSelected )
-        if( m_pView -> getType() == dt_Sequence ) {
-            if( getBaseType() == wt_Message && count == 2 ) {
-                multi = false;
-            } else if( getBaseType() == wt_Text &&
-                       ((FloatingTextWidget*)this) -> getRole() == tr_Seq_Message && count == 2 ) {
-                multi = false;
-            } else if( count > 1 ) {
-                multi = true;
-            }
-        } else if( count > 1 ) {
-            multi = true;
-        }
-
     // if multiple items are selected, we have to check if they all have the same
     // base type
     if (multi == true)
@@ -712,9 +699,9 @@
 
     const QPoint pos(getX(), getY());
     UMLWidget *bkgnd = m_pView->testOnWidget(pos);
-    if (bkgnd) {
+    if (bkgnd && _select) {
         kdDebug() << "UMLWidget::setSelected: setting Z to "
-            << bkgnd->getZ() + 1 << endl;
+            << bkgnd->getZ() + 1 << ", SelectState: " << _select << endl;
         setZ( bkgnd->getZ() + 1 );
     } else {
         setZ( 0 );
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlwidgetcontroller.cpp #580561:580562
@@ -29,6 +29,7 @@
 #include "listpopupmenu.h"
 #include "classifierwidget.h"
 #include "associationwidget.h"
+#include "messagewidget.h"
 
 using namespace Uml;
 
@@ -92,9 +93,9 @@
 
     m_shiftPressed = false;
 
-    int count = m_widget->m_pView->getSelectCount();
+    int count = m_widget->m_pView->getSelectCount(true);
     if (me->button() == Qt::LeftButton) {
-        if (count > 1 && m_widget->m_bSelected == true) {
+        if (count > 1) {
             //Single selection is made in release event if the widget wasn't moved
             m_inMoveArea = true;
             return;
@@ -176,7 +177,7 @@
             widget->adjustAssocs(widget->getX(), widget->getY());
 
             //TODO check if this is needed. What it does?
-            //If needed, create an empty virtual method executed at this place 
+            //If needed, create an empty virtual method executed at this place
             //and override it in ClassifierWidgetController
 /*            if (widget->m_Type == Uml::wt_Class) {
                 ClassifierWidget *cw = static_cast<ClassifierWidget*>(widget);
@@ -203,7 +204,7 @@
                 m_leftButtonDown = false;
 
                 if (!m_moved && !m_resized) {
-                    if (!m_shiftPressed && m_widget->m_pView->getSelectCount() > 1) {
+                    if (!m_shiftPressed && (m_widget->m_pView->getSelectCount(true) > 1)) {
                         selectSingle(me);
                     } else if (!m_wasSelected) {
                         deselect(me);