clientIO / joint

A proven SVG-based JavaScript diagramming library powering exceptional UIs
https://jointjs.com
Mozilla Public License 2.0
4.73k stars 853 forks source link

[Bug]: graph.fromJson resets undo/redo in commandManager with jointJS Plus #2754

Closed mikelina closed 2 months ago

mikelina commented 2 months ago

Current versus expected behaviour

In version 3.7 I was using graph.fromJson() in my commandManager.undo() to fix the arrows in Safari:

public undo(mapData = this.graph.toJSON()) {
    paper.freeze();
  graph.fromJSON(graph.toJSON());
  paper.unfreeze({
    afterRender: function () {
      paper.el.style.display = "none";
      requestAnimationFrame(() => (paper.el.style.display = ""));
      paper.unfreeze();
    },
  });
}

Everything worked perfectly with the history in version 3.7, but after upgrading to version 4.0.1, we noticed that calling fromJSON() resets the history. Here there is an example: https://codesandbox.io/p/sandbox/dazzling-cdn-xdvfth?file=%2Findex.js%3A71%2C38 Could you please assist us with this issue? Any help would be greatly appreciated. Thanks in advance!

Steps to reproduce

  1. Go to https://codesandbox.io/p/sandbox/dazzling-cdn-xdvfth?file=%2Findex.js%3A71%2C38
  2. Change the graph and do undo/redo to see that everything works
  3. Click on fromJson
  4. See that undo/redo doesn't work anymore and the commandManager undoStack and redoStack are empty.

Version

4.0.1

What browsers are you seeing the problem on?

No response

What operating system are you seeing the problem on?

No response

kumilingus commented 2 months ago

The problem with arrows has been solved in JointJS v4.0.4 (https://github.com/clientIO/joint/pull/2677). Workaround should not be necessary if you are using the JointJS+ tgz package (just delete package-lock.json / yarn.lock and install again).

If you use the bundled JointJS+ file from the dist folder you can export the CommandManager and load it back after the reset. See commandManager.toJSON() and commandManager.fromJSON().

mikelina commented 2 months ago

Thanks for the advice about the commandManager, we have solved our problem. The arrows on the other hand even updating the version to jointjs 4.0.4 on safari we can't see them and are forced to reload the view with the workaround we sent above. For example even in this demo https://www.jointjs.com/demos/kitchen-sink-app we reproduce on safari the arrows problem. Is the Demo not updated with the latest version?

kumilingus commented 2 months ago

You are right. We fixed displaying the arrowheads on the initial load. Not when the user tries to update them.

A more efficient workaround in v4 could be forcing the link view to re-render.

graph.on('change:attrs', (cell, attrs, opt) => {
   const { propertyPath = '' } = opt;
   if (propertyPath.startsWith('attrs/line/sourceMarker') || propertyPath.startsWith('attrs/line/targetMarker')) {
       opt.dirty === true;
   } 
});
mikelina commented 2 months ago

The proposed fix works! thank you very much