Bug 376804 - "Else" part outside of alternative combined fragment on creation
Summary: "Else" part outside of alternative combined fragment on creation
Status: RESOLVED FIXED
Alias: None
Product: umbrello
Classification: Applications
Component: general (show other bugs)
Version: 2.21.2 (KDE Applications 16.12.2)
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Umbrello Development Group
URL:
Keywords: junior-jobs
Depends on:
Blocks:
 
Reported: 2017-02-22 14:56 UTC by Ralf Habacker
Modified: 2017-07-18 04:39 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 2.22.90 (KDE Applications 17.07.90)
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ralf Habacker 2017-02-22 14:56:55 UTC
After creating an alternative combined fragment the "[else]" part is located below the combined fragment. Instead it should be inside the box as shown at 
http://www.uml-diagrams.org/sequence-diagrams-combined-fragment.html#operator-alt
Comment 1 Ralf Habacker 2017-02-22 15:50:43 UTC
The implementation of the FloatingDashLineWidget movement in CombinedFragmentWidget seems to be broken. It may be fixed and simplified by making FloatingDashLineWidget instance a QGraphicsItem child of the related CombinedFragmentWidget instance. Movement of both widgets are then handled by QGraphicsScene. The remaining stuff will be to limit vertical movement of the FloatingDashLineWidget instance.
Comment 2 Ralf Habacker 2017-07-12 17:10:33 UTC
How to debug: 
1. Compile umbrello with debug informations (the easiest way is on linux)
2. Set a breakpoint at the line mentioned below
3. start umbrello
4. add a sequence diagram
5. add a combined fragment, choose "Alternative" and press okay
6. the debugger will stop at the mentioned breakpoint and you can inspect what happens with the coordinate settings of the related m_dashLines. 

You need to get familiar with QGraphicsScene and its coordinate system and handling to be able to understand what goes wrong.

void CombinedFragmentWidget::setCombinedFragmentType(CombinedFragmentType combinedfragmentType)
{
    m_CombinedFragment = combinedfragmentType;
    UMLWidget::m_resizable =  true ; //(m_CombinedFragment == Normal);
    // creates a dash line if the combined fragment type is alternative or parallel
    if (m_CombinedFragment == Alt  && m_dashLines.isEmpty())
...< here set breakpoint
Comment 3 Alessandro Stranieri 2017-07-14 12:08:34 UTC
I have debugged this flow around the suggested location in the code. The problem
is that the method CombinedFragmentWidget::setCombinedFragmentType() is
called after the selection of the combinedFragmentType but before the call
to UMLScene::setupNewWidget() which sets the position and the geometry of the
widget. This means that at that moment, any call to methods like y() or height() won't return useful values. The solution would be for me to post-pone the set-up of the FloatingDashLineWidget within the UMLScene::setupNewWidget().
Comment 4 Ralf Habacker 2017-07-15 08:17:19 UTC
(In reply to Alessandro Stranieri from comment #3)
> I have debugged this flow around the suggested location in the code.
Good work :-)
> This means that at that moment, any call to methods like y() or
> height() won't return useful values. The solution would be for me to
> post-pone the set-up of the FloatingDashLineWidget within the
> UMLScene::setupNewWidget().

setupNewWidget() calls a virtual method UMLWidget::activate() which could be used for that purpose. 

1. CombinedFragmentWidget need to have a new virtual method activate() which setup up positions according to the current fragementtype 
2. setCombinedFragmentType() saves only the wanted state and calls activate() only if already activated e.g. widget is full constructed
Comment 5 Ralf Habacker 2017-07-18 04:32:29 UTC
Git commit 12abb14ecef117cea6a8f622d16975f2beb8cd47 by Ralf Habacker, on behalf of Alessandro Stranieri.
Committed on 18/07/2017 at 04:31.
Pushed by habacker into branch 'arcpatch-D6761'.

Fix else section of alternative combined fragment outside widget box

With minor indention fix by Ralf Habacker.
FIXED-IN:2.22.80 (KDE Applications 17.07.80)
Differential Revision: https://phabricator.kde.org/D6761

M  +29   -11   umbrello/umlwidgets/combinedfragmentwidget.cpp
M  +5    -1    umbrello/umlwidgets/combinedfragmentwidget.h

https://commits.kde.org/umbrello/12abb14ecef117cea6a8f622d16975f2beb8cd47
Comment 6 Ralf Habacker 2017-07-18 04:39:13 UTC
Git commit e80bdbbb07fb9c316e4fe68b23231c96ba8fc452 by Ralf Habacker, on behalf of Alessandro Stranieri.
Committed on 18/07/2017 at 04:33.
Pushed by habacker into branch 'Applications/17.08'.

Fix else section of alternative combined fragment outside widget box

With minor indention fix by Ralf Habacker.
FIXED-IN:2.22.90 (KDE Applications 17.07.90)
Differential Revision: https://phabricator.kde.org/D6761

M  +29   -11   umbrello/umlwidgets/combinedfragmentwidget.cpp
M  +5    -1    umbrello/umlwidgets/combinedfragmentwidget.h

https://commits.kde.org/umbrello/e80bdbbb07fb9c316e4fe68b23231c96ba8fc452