Bug 126391 - User interaction with UMLWidget improvements
Summary: User interaction with UMLWidget improvements
Status: RESOLVED FIXED
Alias: None
Product: umbrello
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: unspecified Linux
: HI wishlist
Target Milestone: ---
Assignee: Umbrello Development Group
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-04-28 05:46 UTC by Daniel Calviño Sánchez
Modified: 2013-11-19 13:11 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
User interaction with UMLWidget improvements (122.25 KB, patch)
2006-04-28 05:56 UTC, Daniel Calviño Sánchez
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Calviño Sánchez 2006-04-28 05:46:24 UTC
Version:           Latest SVN (using KDE KDE 3.4.2)
Installed from:    00

I thought that user interaction with UMLWidgets could be improved, so I made a patch for it. Here it is a description of what the patch mades:

New features:
-Move and resize can be cancelled.

-Move and resize can be constrained to X or Y axis using shift and control buttons when moving/resizing the widget.

-The widgets being moved are considered as a block, so if a widget in the selection is going beyond the limits of the canvas, the whole selection can't be moved further in that direction (before, it was only checked for individual widgets instead of selection as a whole)

-The way the widgets are adjusted to the grid was slightly changed. Now, they're adjusted using the upper left corner, instead of center or bottom right corner (depending on the code, the adjustment was made using one or other policy). This way, if component size is also adjusted to the grid, the widgets will fit completly in the grid, instead of being between some grid points.


Bugs fixed:
There were various little bugs in the old code, some of them minor as two double clicks over a widget made it to be deselected, and other more importants as not being able to resize ObjectWidgets. I have forget to document all the fixed bugs... but, well, there were some :) And I've possibly added new ones, however :P


Drawbacks:
-A lot more CPU charge than before (as selection limits are calculated in every move event).


Bugs:
-If a widget is being moved or resized but the cursor isn't over the widget (for example, if the movement is limited in an axis), new mouse release events aren't delivered to the widget, thus preventing movement cancel. However, the next release event over the widget will cancel the movement (as it didn't received the other).

It could be fixed if, in ToolbarStateArrow, when a widget received a press button event, the next release button is always for it, no matter over which widget the button was released (or if it was released in an empty space). However, to do this, the parent class ToolbarState should be modified to reflect this behaviour, and I don't know if it could have some side effects with other ToolbarStates.

-Select a note and a message widget, drag the note and cancel the move shows (only sometimes) a strange behaviour in the final position of the note. I've no idea of why it happens. May it be a problem related to ToolbarState and the way the events are delivered?

-Select a floating text of an association and another widget and drag the widget. The text is constrained badly (because constraining in FloatingTextWidgetController::moveWidgetBy is designed for square constraining instead of circular). It isn't very critical ;)
Comment 1 Daniel Calviño Sánchez 2006-04-28 05:56:29 UTC
Created attachment 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)
Comment 2 Oliver Kellogg 2006-04-30 15:05:16 UTC
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  
Comment 3 Daniel Calviño Sánchez 2006-04-30 18:50:14 UTC
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... :)
Comment 4 Oliver Kellogg 2006-04-30 20:54:55 UTC
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+)]
Comment 5 Oliver Kellogg 2006-07-19 21:32:20 UTC
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.
Comment 6 Oliver Kellogg 2006-09-03 23:51:38 UTC
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);
Comment 7 Ralf Habacker 2013-11-19 13:11:19 UTC
Seem to be fixed long time ago