Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First undo after making a change does nothing #61

Open
graphics-et-al opened this issue Oct 10, 2024 · 2 comments
Open

First undo after making a change does nothing #61

graphics-et-al opened this issue Oct 10, 2024 · 2 comments

Comments

@graphics-et-al
Copy link

graphics-et-al commented Oct 10, 2024

After making a change to the canvas captured in the history, the first undo does nothing.

Probable cause is the code:

  /**
   * It pushes the state of the canvas into history stack
   */
  fabric.Canvas.prototype._historySaveAction = function (e) {
    if (this.historyProcessing) return;
    if (!e || (e.target && !e.target.excludeFromExport)) {
// HERE pushes the CURRENT state to the end of the historyUndo array. When undoing for the first time, it loads the current state.
      const json = this._historyNext();
//
      this.historyUndo.push(json);
      this.historyNextState = this._historyNext();
      this.fire('history:append', { json: json });
    }
  };

It should be (as in previous versions)

  /**
   * It pushes the state of the canvas into history stack
   */
  fabric.Canvas.prototype._historySaveAction = function (e) {
    if (this.historyProcessing) return;
    if (!e || (e.target && !e.target.excludeFromExport)) {
// HERE pushes the PREVIOUS state to the end of the historyUndo array.
      const json = this.historyNextState;
      this.historyUndo.push(json);
      this.historyNextState = this._historyNext();
      this.fire('history:append', { json: json });
    }
  };

After making this change it works for me :)

@YoniZiv
Copy link

YoniZiv commented Oct 12, 2024

Was looking for a solution for this.
Did you actually change anything in the "should be" example?
I might be tired but they seem identical

@graphics-et-al
Copy link
Author

graphics-et-al commented Oct 14, 2024

Was looking for a solution for this. Did you actually change anything in the "should be" example? I might be tired but they seem identical

Oh I'm so very sorry! Have edited the 'Should be'. It works now.

Also another gotcha I found: if you're loading canvas programmatically, it can fire events that add to the history. Here's an example of how I do it...


// load the canvas from a data object to a canvas
export const loadCanvas = (canvas_data, _canvas) => {
    return new Promise((resolve, reject) => {
        // clear the canvas
        _canvas.clear();
        // rebuild the canvas from the objects contained in the data
        fabric.util.enlivenObjects(canvas_data.objects, (objs) => {
            // add items
            objs.forEach((item) => {
                _canvas.add(item);
            });
            // Do anything else you need to here e.g. add a background 
       
            // Clear the history
            canvas.clearHistory();
            // take a snapshot
            canvas.onHistory();
            // resolve the promise
            resolve(true);
        });
    });
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants