Fix vanishing transitions when zooming to state

The main design issue in this plugin is that
there is no clear separation between model (which
pretent to be ScxmlDocument and his childern of
ScxmlTag type) and its views (GraphicsScene
and its children of BaseItem subclasses).
When the "Zoom to State" action is invoked,
the new view is being created, showing just
a part of the ScxmlDocument model. However,
some validation is done only on the view part,
checking that BaseItems belong to the common
GraphicsScene. In case the transition was defined
from internal state node to somewhere outside,
we don't have for it an BaseItem on the gui side,
as we just show the part of the scene. That's why
the validation vanishes the transition when
viewing a zoom.

In general, all the validations should be moved
from gui to the ScxmlDocument / ScxmlTag part.
This in general would require really big refactoring,
or even rewrite.

This patch moves some checks into a model side and
disables direct modifications of the model when
it's not desired.

Fixes: QTCREATORBUG-21676
Change-Id: Ica0771f637a9802dcc0fb87ad04b0ee77a21cda2
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Jarek Kobus
2021-01-12 12:12:56 +01:00
parent 484d40258a
commit 2b2cacfc09
7 changed files with 40 additions and 10 deletions

View File

@@ -649,6 +649,14 @@ ScxmlTag *ScxmlDocument::scxmlRootTag() const
return tag; return tag;
} }
ScxmlTag *ScxmlDocument::tagForId(const QString &id) const
{
if (id.isEmpty())
return nullptr;
ScxmlTag *root = scxmlRootTag();
return root ? root->tagForId(id) : nullptr;
}
ScxmlTag *ScxmlDocument::rootTag() const ScxmlTag *ScxmlDocument::rootTag() const
{ {
return m_rootTags.isEmpty() ? 0 : m_rootTags.last(); return m_rootTags.isEmpty() ? 0 : m_rootTags.last();

View File

@@ -163,6 +163,7 @@ public:
*/ */
ScxmlTag *rootTag() const; ScxmlTag *rootTag() const;
ScxmlTag *scxmlRootTag() const; ScxmlTag *scxmlRootTag() const;
ScxmlTag *tagForId(const QString &id) const;
void pushRootTag(ScxmlTag *tag); void pushRootTag(ScxmlTag *tag);
ScxmlTag *popRootTag(); ScxmlTag *popRootTag();

View File

@@ -485,6 +485,20 @@ bool ScxmlTag::isRootTag() const
return m_document->rootTag() == this; return m_document->rootTag() == this;
} }
ScxmlTag *ScxmlTag::tagForId(const QString &id) const
{
for (ScxmlTag *child : m_childTags) {
const TagType type = child->tagType();
const bool typeOK = type == State || type == Parallel || type == History || type == Final;
if (typeOK && child->attribute("id") == id)
return child;
ScxmlTag *foundTag = child->tagForId(id);
if (foundTag)
return foundTag;
}
return nullptr;
}
QVector<ScxmlTag*> ScxmlTag::allChildren() const QVector<ScxmlTag*> ScxmlTag::allChildren() const
{ {
return m_childTags; return m_childTags;

View File

@@ -112,6 +112,8 @@ public:
int childIndex(const ScxmlTag *child) const; int childIndex(const ScxmlTag *child) const;
int index() const; int index() const;
bool isRootTag() const; bool isRootTag() const;
ScxmlTag *tagForId(const QString &id) const;
/** /**
* @brief writeXml - write tag's content with the QXMLStreamWriter. Call writeXml-function for all children too. * @brief writeXml - write tag's content with the QXMLStreamWriter. Call writeXml-function for all children too.

View File

@@ -708,7 +708,7 @@ void TransitionItem::setEndPos(const QPointF &endPos, bool snap)
} }
} }
void TransitionItem::setEndItem(ConnectableItem *item) void TransitionItem::setEndItem(ConnectableItem *item, bool fixValue)
{ {
if (item) { if (item) {
@@ -725,7 +725,7 @@ void TransitionItem::setEndItem(ConnectableItem *item)
} }
updateZValue(); updateZValue();
updateTarget(); updateTarget(fixValue);
} }
QPointF TransitionItem::loadPoint(const QString &name) QPointF TransitionItem::loadPoint(const QString &name)
@@ -984,9 +984,10 @@ void TransitionItem::updateEditorInfo(bool allChilds)
m_pen.setColor(stateColor.isValid() ? stateColor : qRgb(0x12, 0x12, 0x12)); m_pen.setColor(stateColor.isValid() ? stateColor : qRgb(0x12, 0x12, 0x12));
} }
void TransitionItem::updateTarget() void TransitionItem::updateTarget(bool fixValue)
{ {
setTagValue("target", m_endItem ? m_endItem->itemId() : QString()); if (fixValue)
setTagValue("target", m_endItem ? m_endItem->itemId() : QString());
if (m_endItem) if (m_endItem)
m_endItem->checkInitial(true); m_endItem->checkInitial(true);
} }
@@ -1002,7 +1003,7 @@ void TransitionItem::updateAttributes()
m_endItem = nullptr; m_endItem = nullptr;
findEndItem(); findEndItem();
updateTarget(); updateTarget(false);
updateZValue(); updateZValue();
} }
@@ -1123,7 +1124,7 @@ void TransitionItem::findEndItem()
if (items[i]->type() >= FinalStateType) { if (items[i]->type() >= FinalStateType) {
auto item = qgraphicsitem_cast<ConnectableItem*>(items[i]); auto item = qgraphicsitem_cast<ConnectableItem*>(items[i]);
if (item && item->itemId() == targetId) { if (item && item->itemId() == targetId) {
setEndItem(item); setEndItem(item, false);
break; break;
} }
} }

View File

@@ -79,7 +79,7 @@ public:
void disconnectItem(ConnectableItem *item); void disconnectItem(ConnectableItem *item);
void setStartItem(ConnectableItem *item); void setStartItem(ConnectableItem *item);
void setEndItem(ConnectableItem *item); void setEndItem(ConnectableItem *item, bool fixValue = true);
void startTransitionFrom(ConnectableItem *item, const QPointF &mouseScenePos); void startTransitionFrom(ConnectableItem *item, const QPointF &mouseScenePos);
void setEndPos(const QPointF &endPos, bool snap = true); void setEndPos(const QPointF &endPos, bool snap = true);
@@ -95,7 +95,7 @@ public:
void init(ScxmlTag *tag, BaseItem *parentItem = nullptr, bool initChildren = true, bool blockUpdates = false) override; void init(ScxmlTag *tag, BaseItem *parentItem = nullptr, bool initChildren = true, bool blockUpdates = false) override;
void updateEditorInfo(bool allChilds = true) override; void updateEditorInfo(bool allChilds = true) override;
void updateAttributes() override; void updateAttributes() override;
void updateTarget(); void updateTarget(bool fixValue = true);
void finalizeCreation() override; void finalizeCreation() override;
void checkVisibility(double scaleFactor) override; void checkVisibility(double scaleFactor) override;
bool containsScenePoint(const QPointF &p) const override; bool containsScenePoint(const QPointF &p) const override;

View File

@@ -143,8 +143,12 @@ SetAttributeCommand::SetAttributeCommand(ScxmlDocument *doc, ScxmlTag *tag, cons
void SetAttributeCommand::doAction(const QString &key, const QString &value) void SetAttributeCommand::doAction(const QString &key, const QString &value)
{ {
emit m_document->beginTagChange(ScxmlDocument::TagAttributesChanged, m_tag, m_tag->attribute(key)); emit m_document->beginTagChange(ScxmlDocument::TagAttributesChanged, m_tag, m_tag->attribute(key));
m_tag->setAttribute(key, value);
emit m_document->endTagChange(ScxmlDocument::TagAttributesChanged, m_tag, value); QString newValue = value;
if (m_tag->tagType() == Transition && key == "target" && !m_document->tagForId(value))
newValue = QString();
m_tag->setAttribute(key, newValue);
emit m_document->endTagChange(ScxmlDocument::TagAttributesChanged, m_tag, newValue);
} }
int SetAttributeCommand::id() const int SetAttributeCommand::id() const