Skip to content

Ability to undo/redo multiple changes in one step#122

Open
denzo wants to merge 12 commits intooffirgolan:masterfrom
denzo:array-of-arrays
Open

Ability to undo/redo multiple changes in one step#122
denzo wants to merge 12 commits intooffirgolan:masterfrom
denzo:array-of-arrays

Conversation

@denzo
Copy link
Copy Markdown
Contributor

@denzo denzo commented Nov 11, 2017

This PR introduces new functionality without breaking the existing one.

Currently the undo/redo stack consists of list of changes. Therefore it is not possible to make multiples changes and be able to undo those in one step. This is easy to demonstrate via using setProperties for example.

tm.setProperties({
  completedAt: new Date(),
  isCompleted: true
});

Calling tm.undo() will not undo both of the changes.

In order to achieve the above this PR changes the structure of how we store the changes.

The undo/redo stack now stores a collection of changeSets which is nothing more than an Array of changes.

It looks like this:

undoStack/redoStack = [
  [Record, Record],
  [Record],
  [Record, Record, Record]
];

The current functionality of the addon stays the same. However, when multiple changes need to be recorded as one the user of the addon needs to call .startTimeMachine prior to making the changes and .stopTimeMachine when all the changes are done.

tm.startTimeMachine();
tm.setProperties({
  completedAt: new Date(),
  isCompleted: true
});
tm.stopTimeMachine();

Calling tm.undo() will undo both of the changes.

@denzo denzo mentioned this pull request Nov 11, 2017
@denzo denzo changed the title use undoStack as an Array of ChnageSets use undoStack as an Array of ChangeSets Nov 18, 2017
@offirgolan
Copy link
Copy Markdown
Owner

@denzo there are a lot of changes here. Can you explain what's going on and why these changes are needed?

@denzo denzo changed the title use undoStack as an Array of ChangeSets Ability to undo/redo multiple changes in one step Dec 30, 2017
@denzo
Copy link
Copy Markdown
Contributor Author

denzo commented Dec 30, 2017

@offirgolan Apologies for the delayed response.

I've updated the description of this PR and added tests and explanation in the code.

I hope it made it a bit more clear. Let me know what you think and if this is something you would consider including as a feature of this awesome addon.

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

Successfully merging this pull request may close these issues.

2 participants