Undo/Redo "Move" not updating link positions properly

I have set up set up a diagram listener to update target bindings on undo/redo. Because without this the twoWay bound location data will not undo/redo at all:

  diagram.model.addChangedListener(function(evt) {
      if (
        evt.change === go.ChangedEvent.Transaction &&
        (evt.propertyName === 'FinishedUndo' || evt.propertyName === 'FinishedRedo')
      ) {
        setTimeout(function() {
          diagram.commit(function(_diagram) {
            _diagram.updateAllTargetBindings();
            _diagram.updateAllRelationshipsFromData();
          }, null);
        }, 1);
      }
    });

But the link endpoint positions don’t seem to be correctly captured during an undo/redo operation. As you can see in this video the link positions are correct when doing a simple move/drag operation, but when undoing/redoing that change it draws them slightly off:

Screen Recording 2023-03-16 at 1.13.12 PM

Doing that in a model Changed listener doesn’t sound right.

Could you please tell us how to reproduce the problem?
What is your link template?
Do you have any other event handlers or diagram event listeners besides the one you quoted, and if so, what do they do?

It didn’t/doesn’t feel right either, but it was the only thing I could find to kick the location undo/redo into gear. I found the solution here: Use undoManager for virtualizedForceLayout - #7 by walter

If this isn’t the right place/way to do this then what is?

I’m using the ReactDiagram, and am using the onModelChange function to update my db data layer when I make transactions through GoJS. Using a handful of other unrelated diagramEvent listeners:

ViewportBoundsChanged: switching node/link category at a specific zoom level to use a detailed vs compact template (using: setCategoryForNodeData, setCategoryForLinkData)

LostFocus: clearing selection

ClipboardChanged: monitoring clipboard contents

The link template is:

export const defaultLinkTemplate = hideContextMenu =>
  GraphObject.make(
    Link,
    {
      corner: 6,
      selectionAdorned: false,
    },
    new Binding('curve', 'type', type => (type === 'revert' ? Link.Bezier : Link.JumpGap)),
    new Binding('curviness', 'type', type => (type === 'revert' ? 100 : 0)),
    new Binding('isShadowed', 'isSelected').ofObject(),
    // the "line"
    GraphObject.make(
      Shape,
      { strokeWidth: 2, opacity: 1 },
      new Binding('stroke', '', ({ data }) => LINK_COLORS[data.state]).ofObject(),
      new Binding('strokeDashArray', 'type', type => (type === 'revert' ? [6, 6] : [])),
      new AnimationTrigger('stroke', { duration: 300 }),
    ),
    {
      contextMenu: GraphObject.make(HTMLInfo, { show: () => {}, hide: hideContextMenu }),
    },
    // the "from" end arrowhead
    GraphObject.make(
      Shape,
      { fromArrow: 'Circle', stroke: null, scale: 1.2, strokeWidth: 2 },
      new Binding('fill', '', ({ data, isSelected }) =>
        isSelected ? '#FFFFFF' : LINK_COLORS[data.state],
      ).ofObject(),
      new AnimationTrigger('fill', { duration: 300 }),
      new Binding('stroke', '', ({ data }) => LINK_COLORS[data.state]).ofObject(),
    ),
    // the "to" end arrowhead
    GraphObject.make(
      Shape,
      { toArrow: 'RoundedTriangle', stroke: null, scale: 1.2, strokeWidth: 2 },
      new Binding('fill', '', ({ data, isSelected }) =>
        isSelected ? '#FFFFFF' : LINK_COLORS[data.state],
      ).ofObject(),
      new AnimationTrigger('fill', { duration: 300 }),
      new Binding('stroke', '', ({ data }) => LINK_COLORS[data.state]).ofObject(),
    ),
    {
      shadowColor: 'grey',
      shadowBlur: 12,
      shadowOffset: new Point(0, 0),
      mouseHover(e, node) {
        node.isShadowed = true;
      },
      mouseLeave(e, node) {
        if (!node.isSelected) node.isShadowed = false;
      },
    },
  );

First, an unrelated suggestion. Three of your Bindings are like:

new go.Binding('stroke', '', ({ data }) => LINK_COLORS[data.state]).ofObject(),

I believe this could more clearly be written:

new go.Binding('stroke', 'state', state => LINK_COLORS[state]),

Second, I used your link template in a copy of the Minimal sample and did not encounter any issues with undo/redo:

Here’s my complete test sample:

<!DOCTYPE html>
<html>
<head>
  <title>Minimal GoJS Sample</title>
  <!-- Copyright 1998-2023 by Northwoods Software Corporation. -->
</head>
<body>
  <div id="myDiagramDiv" style="border: solid 1px black; width:100%; height:600px"></div>

  <script src="https://unpkg.com/gojs"></script>
  <script id="code">
const $ = go.GraphObject.make;

const myDiagram =
  $(go.Diagram, "myDiagramDiv",
    {
      "undoManager.isEnabled": true
    });

myDiagram.nodeTemplate =
  $(go.Node, "Auto",
    $(go.Shape,
      { fill: "white" },
      new go.Binding("fill", "color")),
    $(go.TextBlock,
      { margin: 8 },
      new go.Binding("text"))
  );

const LINK_COLORS = ["black", "red", "green", "blue"];
myDiagram.linkTemplate =
go.GraphObject.make(
  go.Link,
    {
      corner: 6,
      selectionAdorned: false,
    },
    new go.Binding('curve', 'type', type => (type === 'revert' ? go.Link.Bezier : go.Link.JumpGap)),
    new go.Binding('curviness', 'type', type => (type === 'revert' ? 100 : 0)),
    new go.Binding('isShadowed', 'isSelected').ofObject(),
    // the "line"
    go.GraphObject.make(
      go.Shape,
      { strokeWidth: 2, opacity: 1 },
      //new go.Binding('stroke', '', ({ data }) => LINK_COLORS[data.state]).ofObject(),
      new go.Binding('stroke', 'state', state => LINK_COLORS[state]),
      new go.Binding('strokeDashArray', 'type', type => (type === 'revert' ? [6, 6] : [])),
      new go.AnimationTrigger('stroke', { duration: 300 }),
    ),
    {
      //??? contextMenu: GraphObject.make(HTMLInfo, { show: () => {}, hide: hideContextMenu }),
    },
    // the "from" end arrowhead
    go.GraphObject.make(
      go.Shape,
      { fromArrow: 'Circle', stroke: null, scale: 1.2, strokeWidth: 2 },
      new go.Binding('fill', '', ({ data, isSelected }) =>
        isSelected ? '#FFFFFF' : LINK_COLORS[data.state],
      ).ofObject(),
      new go.AnimationTrigger('fill', { duration: 300 }),
      //new go.Binding('stroke', '', ({ data }) => LINK_COLORS[data.state]).ofObject(),
      new go.Binding('stroke', 'state', state => LINK_COLORS[state]),
    ),
    // the "to" end arrowhead
    go.GraphObject.make(
      go.Shape,
      { toArrow: 'RoundedTriangle', stroke: null, scale: 1.2, strokeWidth: 2 },
      new go.Binding('fill', '', ({ data, isSelected }) =>
        isSelected ? '#FFFFFF' : LINK_COLORS[data.state],
      ).ofObject(),
      new go.AnimationTrigger('fill', { duration: 300 }),
      //new go.Binding('stroke', '', ({ data }) => LINK_COLORS[data.state]).ofObject(),
      new go.Binding('stroke', 'state', state => LINK_COLORS[state]),
    ),
    {
      shadowColor: 'grey',
      shadowBlur: 12,
      shadowOffset: new go.Point(0, 0),
      mouseHover(e, node) {
        node.isShadowed = true;
      },
      mouseLeave(e, node) {
        if (!node.isSelected) node.isShadowed = false;
      },
    },
  );

myDiagram.model = new go.GraphLinksModel(
[
  { key: 1, text: "Alpha", color: "lightblue" },
  { key: 2, text: "Beta", color: "orange" },
  { key: 3, text: "Gamma", color: "lightgreen" },
  { key: 4, text: "Delta", color: "pink" }
],
[
  { from: 1, to: 2, type: "revert" },
  { from: 1, to: 3, state: 2 },
  { from: 2, to: 2 },
  { from: 3, to: 4 },
  { from: 4, to: 1 }
]);
  </script>
</body>
</html>

Note that I commented out the context menu, since I don’t know it and it probably is irrelevant to your issue.

So I suspect that there’s something else going on in your app to cause the problem.

I tried to build out as full but still simple recreation of our use-case and I wasn’t able to get it to happen exactly the same. I’m not yet sure what I’m missing… But you can use this example to see a very similar break. Try this: move a node, undo, then redo (note: the state initialization isn’t perfect since its all ripped out of our main app, so to get it to draw just make a small change to the code to force a re-render, something simple like a newline works fine)

That sample complains about being unable to read localStorage.
Then when I make a code change as you suggest, there is an error:

Cannot add node 1 because a node with that id is already in the Store.

But indeed two nodes show up, connected by a link.

When I move one of the nodes, and then undo, and then redo, everything works as I would expect.
Nodes move, and links stay “connected”, just as I think you would expect.

I even tried making additional changes to the diagram. Repeated undo’s and redo’s work well.

Hmm… This is what I see when repeating those steps:

  • newline to render
  • move node
  • undo (fine)
  • redo (not ok)

(any repeated undo/redos result the same)

Maybe browser related? :shrug: I’m using Chrome (109.0.5414.119) Edit: I just tried on FF & Safari. FF is the same result as chrome, but it works as-expected in Safari! So maybe it is browser related bug of some sort? … Although I also checked my full app against Safari and I’m still seeing the bug there

Screen Recording 2023-03-17 at 2.53.26 PM (1)

Check, if you can, to make sure that there aren’t any changes to the diagram or model being made from your (react) code during either any undo or any redo. I’m wondering if an undo, for example, might be causing a change to your app state that is being reflected as a “real” change to the model, even though that should never happen. Such a change could be construed by the UndoManager as a real change, which would clear out any future “history” instead of leaving the UndoManager.history unmodified to permit redo’s.

Still haven’t been able to completely solve the issue, but we have found an improvement.
Adding

_diagram.links.each(link => {
  link.invalidateRoute();
});

right above out calls to updateAllTargetBindings and updateAllRelationshipsFromData seems to resolve the issue of the links not fully moving back to the correct position. But the links still occasionally disappear from the diagram visualization entirely, the data for them still exists, they just fail to render seemingly randomly after a series of undo and redo actions.

If you run with the debug version of the library, go-debug.js, you will find that some bindings have problems. I have rewritten them, guessing what you were intending. Again, I cannot reproduce any problems with undo and redo with various selection on both Firefox and Chrome.

<!DOCTYPE html>
<html>
<head>
  <title>Minimal GoJS Sample</title>
  <!-- Copyright 1998-2023 by Northwoods Software Corporation. -->
</head>
<body>
  <div id="myDiagramDiv" style="border: solid 1px black; width:100%; height:600px"></div>

  <script src="https://unpkg.com/gojs/release/go-debug.js"></script>
  <script id="code">
const $ = go.GraphObject.make;

const myDiagram =
  $(go.Diagram, "myDiagramDiv",
    {
      "undoManager.isEnabled": true
    });

myDiagram.nodeTemplate =
  $(go.Node, "Auto",
    $(go.Shape,
      { fill: "white" },
      new go.Binding("fill", "color")),
    $(go.TextBlock,
      { margin: 8 },
      new go.Binding("text"))
  );

const LINK_COLORS = ["black", "red", "green", "blue"];
myDiagram.linkTemplate =
go.GraphObject.make(
  go.Link,
    {
      corner: 6,
      selectionAdorned: false,
    },
    new go.Binding('curve', 'type', type => (type === 'revert' ? go.Link.Bezier : go.Link.JumpGap)),
    new go.Binding('curviness', 'type', type => (type === 'revert' ? 100 : 0)),
    new go.Binding('isShadowed', 'isSelected').ofObject(),
    // the "line"
    go.GraphObject.make(
      go.Shape,
      { strokeWidth: 2, opacity: 1 },
      new go.Binding('stroke', 'state', state => LINK_COLORS[state]),
      new go.Binding('strokeDashArray', 'type', type => (type === 'revert' ? [6, 6] : [])),
      new go.AnimationTrigger('stroke', { duration: 300 }),
    ),
    // the "from" end arrowhead
    go.GraphObject.make(
      go.Shape,
      { fromArrow: 'Circle', stroke: null, scale: 1.2, strokeWidth: 2 },
      new go.Binding('fill', 'isSelected', (isSelected, shape) =>
        isSelected ? '#FFFFFF' : LINK_COLORS[shape.part.data.state || 0],
      ).ofObject(),
      new go.AnimationTrigger('fill', { duration: 300 }),
      new go.Binding('stroke', 'state', state => LINK_COLORS[state]),
    ),
    // the "to" end arrowhead
    go.GraphObject.make(
      go.Shape,
      { toArrow: 'RoundedTriangle', stroke: null, scale: 1.2, strokeWidth: 2 },
      new go.Binding('fill', 'isSelected', (isSelected, shape) =>
        isSelected ? '#FFFFFF' : LINK_COLORS[shape.part.data.state || 0],
      ).ofObject(),
      new go.AnimationTrigger('fill', { duration: 300 }),
      new go.Binding('stroke', 'state', state => LINK_COLORS[state]),
    ),
    {
      shadowColor: 'grey',
      shadowBlur: 12,
      shadowOffset: new go.Point(0, 0),
      mouseHover(e, node) {
        node.isShadowed = true;
      },
      mouseLeave(e, node) {
        if (!node.isSelected) node.isShadowed = false;
      },
    },
  );

myDiagram.model = new go.GraphLinksModel(
[
  { key: 1, text: "Alpha", color: "lightblue" },
  { key: 2, text: "Beta", color: "orange" },
  { key: 3, text: "Gamma", color: "lightgreen" },
  { key: 4, text: "Delta", color: "pink" }
],
[
  { from: 1, to: 2, type: "revert" },
  { from: 1, to: 3, state: 2 },
  { from: 2, to: 2 },
  { from: 3, to: 4 },
  { from: 4, to: 1 }
]);
  </script>
</body>
</html>

We tried using the debug library early on but encountered issues due to the fact that we’re using the gojs-react package and didn’t quite understand how to best load the debug lib for both. But after finding this post: How to import go-debug jointly with gojs-react? I was able to get the debug lib up and running and was able to see various issues that it was complaining about and after squashing all of the errors and most of the warnings this issue appears to be resolved. I wanted to wait long enough to have multiple people test against it and its not come back up so I think I can say that the last piece of the puzzle was squashing those silent errors/warnings that we were unaware of by only using the main lib. Thank you!