reaflow
reaflow copied to clipboard
Undo doesn't handle mass undos, history gets partially lost
I'm submitting a...
[x] Bug report
Current behavior
When using undo/redo, when using the cmd+z shortcut, if we keep pressing on it then the history gets messed up somehow and we cannot do cmd+shift+z more than once/twice.
When doing cmd+z multiple time (not spamming) then it works as expected.
Expected behavior
It should work the same way whether spamming cmd+z or not.
Environment
reaflow 3.0.5
Couldn't reproduce the issue with the SB demo at https://reaflow.dev/?path=/story/demos-undo-redo--simple
Might be related to the shared store I'm using? (Recoil)
I tried throttling the undo/redo, but it didn't yield good results.
I couldn't locate the root of the issue. I feel like applying too many undos too fast corrupts the history or something similar.
I created an online demo: https://poc-nextjs-reaflow-iqs5cxpy3.vercel.app/
Add 2 nodes by dragging the ports, then use undo/redo through cmd+z and while the undos will work, it won't be possible to redo them when using the shortcuts. Using the undo/redo buttons works fine though.
I tried removing most of my business logic (custom nodes, recoil) and use something as basic as possible and the issue still persists.
https://poc-nextjs-reaflow-mzxz3a5h0.vercel.app/
Added a "Add node" button (top right). When keeping cmd+z pressed after adding nodes, the "redo" action cannot be done. Doing undos one by one works correctly though (as before).
I also noticed when the issue happens, and it happens even when clicking manually.
Video: https://youtu.be/4llZSog66O0
👉 At this point, I believe the issue is from reaflow undo/redo implementation. When "undoing" from the "initial state", it looses the whole history.
Okay, I think I've found the root cause.
It's because I automatically add a "start" node if there is no "start" node. And when I do the last "undo", it removes the "start" node, which in turn recreates the start nodes, which counts as a new action and removes the "redo" history.
Okay, so that was one of the reasons it didn't work properly. Fixed by https://github.com/Vadorequest/poc-nextjs-reaflow/commit/7dd9256bb5fbcae668e5bd381a2e7e79e3485f04
But now, I'm back at square one, when I first noticed the issue with undo/redo, which only allows partial redo when undoing too much at once using the cmd+z shortcut. (the above issue I just fixed was added in the meantime and wasn't why I opened this issue in the first place)
@amcdnl After much time spent on this, I'm still unsure of the root issue. It seems to work fine in the demo, but doesn't on my app.
I'm thinking about adding a few options to the useUndoRedo built-in utility, such as:
- Disable binding shortcuts (make them optional)
- Exposing more internal stuff (like ref, etc. Basically everything that's needed to build our own shortcuts)
Does that make sense to you?
Can you make a PR for the disable?
You should be able to get the refs.
I'll make a PR to allow disabling default shortcuts binding then.
The callbackRef
and callbackRef
aren't exposed. I think that's the most straightforward way to do that. I don't see how I can use the ref at this time.
https://github.com/reaviz/reaflow/blob/0c15d993798d390f372f7e763df7658b852ef3d9/src/helpers/useUndo.ts#L200-L209