Fix undo/redo bug

[ecfairle]

Close #170. Forked from #174

[ecfairle]

I added onto @heruiwoniou 's PR since I have been using his branch for a while now in a project. There is a minor bug in his PR, that i believe has to do with the extra strokes being recorded on the canvas while the history is being updated. Which would explain why the undo/redo tests were failing.

I moved the history updating logic into the mouse-down / redo / undo / clear functions so there's no timing issues (basically record history on the "next" action instead of the "current" action).

Thanks for making this utility! Let me know if you have any questions or want me to make any changes.

[Copilot]

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

packages/react-sketch-canvas/src/ReactSketchCanvas/index.tsx:174

• Resetting history to an empty array and setting historyPos to -1 may break subsequent undo/redo operations. Consider resetting to the initial state (e.g., history as [[]] and historyPos as 0) to maintain consistency.

setHistory([]);

packages/react-sketch-canvas/src/ReactSketchCanvas/index.tsx:199

• [nitpick] Incrementing historyPos before adding the current stroke state via addLastStroke might capture a stale currentPaths state. Reviewing the order of operations to ensure accurate history capture is recommended.

setHistoryPos(pos => pos + 1);

[vinothpandian]

@ecfairle Thank you for the PR. I swear I’m going to take this weekend to merge these PRs and release the new version!

Can you please look at the review comments from Copilot if you have time?

[vinothpandian]

This implementation is creating a race condition when undo or redo buttons are pressed quickly, especially on large codebases. I'm trying to fix this

[ecfairle]

Ah, right. It seems like the correct solution here is to refactor the logic for history-altering events as an event queue. I think that's closer to the underlying mental model for how a canvas should work. Anything else would be kind of hack imo. What do you think about doing that?

[vinothpandian]

Yes @ecfairle. That makes sense.