-
Notifications
You must be signed in to change notification settings - Fork 3
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
override redo function #2
Comments
You're right that this is how it's designed. Could you give an example of some code where it's impossible for the undo to have its own undo? |
The case arises when undoing built-in functions (which obviously dont have undoables registered in them ). In the case I'm working on, I have a lot of DOM manipulation. The undo operation for an insertBefore is a removeChild and the redo operation is the insertBefore itself. Now if I give removeChild as the undo operation through undo-able, then no undo-able is registered in the removeChild for the redo operation. So, redo will not work. If have to undo an insertBefore, I would have to create a new function which assigns the undoable and then calls the insertBefore. The undo-able in this function would take insertBefore as the undo function. This would mean that I have to replace every instance of the code and also create similar functions for the rest of the DOM manipulating functions which introduces more possibility of error due to modification of code. Another way is to replace the built-in functions with my own methods with the undo-able but this could affect the rest of the page. But, if I'm able to give an optional redo operation, then I could specify removeChild as the the undo operation and insertBefore as the redo operation and then I could implement undo/redo without modifying my original code but by simply adding codes that will not directly affect the functionality/working. I think this would prompt more response for the library. |
Okay I understand what you mean. This design choice led me to need code like the following: set: function(variable, value) {
this[variable] = value;
},
uSet: function(variable, value) {
this.undoable('set ' + variable, this.uSet, [variable, this[variable]]);
this.set(variable, value);
}, in a recent project. This was a design decision on my part as I think it's much easier to just remember that every method needs to point to the reverse of itself and all of the undo/redo functionality works automatically. In my own projects the tradeoff of having to write some wrappers for dom manipulation are reasonable because the project grows so large that the additional complications of having to specify both undo and redo every time an annoyance. However I do see your point that it could get annoying, especially in projects where these wrapper functions could take up a substantial amount of the code. I will look into a way to make this sort of operation easier. |
As I understand, currently redo works by calling the undo of the function that was called to perform the undo operation. But this may not be always possible such as when native code is directly used ( in which case the undo operation itself doesn't have its own undo code).
So it would be useful to add an optional redo function too.
Correct me if I'm wrong.
The text was updated successfully, but these errors were encountered: