fix: Fix for cells not removed - #115
Conversation
|
I think I have some idea what's happened here. Previously, many functions were quite lenient in what they accepted, and would allow for null/undefined values to be passed and return. I know I tried to reduce the number of times functions simply returned when passed null or undefined values to reduce the risk of errors happening silently, but this also may have introduced unintentional side effects. I'm not sure whether it was me or someone else in this instance, but that ...
// Adds or removes a child to the view by online invaliding
// the minimal required portions of the cache, namely, the
// old and new parent and the child.
else if (change instanceof mxChildChange)
{
var newParent = this.model.getParent(change.child);
this.view.invalidate(change.child, true, true);
if (!this.model.contains(newParent) || this.isCellCollapsed(newParent))
{
this.view.invalidate(change.child, true, true);
this.removeStateForCell(change.child);
// Handles special case of current root of view being removed
if (this.view.currentRoot == change.child)
{
this.home();
}
}
...If you look into the /**
* Function: isAncestor
*
* Returns true if the given parent is an ancestor of the given child. Note
* returns true if child == parent.
*
* Parameters:
*
* parent - <mxCell> that specifies the parent.
* child - <mxCell> that specifies the child.
*/
mxGraphModel.prototype.isAncestor = function(parent, child)
{
while (child != null && child != parent)
{
child = this.getParent(child);
}
return child == parent;
};I think this patch may correct this, as it seems to mirror the original behaviour of mxGraph. |
tbouffard
left a comment
There was a problem hiding this comment.
Thanks @mayorovad for this new contribution ❤️
✔️ @mcyph did a formal investigation to validate the implementation logic
✔️ Tested with the DynamicLoading story
development branch |
PR 115 |
|---|---|
 |
 |
Summary
This is fix for #114. There is logical error in processing
ChildChangeinGraph. When cell is removed,ChildChangecreated to change parent of removed cell tonull. But in code, if parent is null, code to destroy child cell state is not called, so cell stays rendered on canvas.maxGraph/packages/core/src/view/Graph.ts
Lines 589 to 599 in 02ea6f1
Added check for "falsy" parent so correct branch of code will be executed and cells will be destroyed.
Description for the changelog
Fixed bug with cells not removing when calling
Graph.removeCells()closes #114