Open circleb opened 9 years ago
Thanks for the report. Though since this is just a demo, I don't have plans for fixing this issue. I'd review a PR and merge it, but that's about it.
Ok, thanks for the reply, I'm also reviewing Arthur's Undo Manager. If that ends up being a dead end, I may see what I can do as far as fixing yours.
If you find something that works better than this proof of concept, I'd be happy to point everyone to that instead (in the readme).
@jzaefferer This is a very nice undo plugin. but there is still a bug just like what @circleb told. After doing some action and I tried to undo the first time, the "stackPosition" didn't went down by one (should be stackPosition--) but instead it increased by one (stackPosition++) and the number of array in "command" went up (the array number should be the same). The problem came when I tried to redo it. The state is quite messy after that I just realized that after implementing quite a lot of code with this plugin.. that's too bad
Sorry to hear that, but my previous comments still apply. PRs are welcome, but I can't help beyond that.
Yeah, that's fine. I'll try to take a look at your code later. Anyway thank you for making it
Hi,
I tested this plugin and I also discovered a bug in the demo, of the same type as the one explained by circleb.
The problem comes from the variable "Startvalue": this variable is updated when the text changes but it is not updated when you click on the "Undo" button ...
So after a "undo" the variable "startvalue" is not updated so when you modify the text you record in the undo stack an "EditCommand" with a bad value for the property "oldValue".
1- You can solve the first problem in several ways, here is a way: Replace this code:
$(".bold").click(function() {
document.execCommand("bold", false);
var newValue = text.html();
stack.execute(new EditCommand(text, startValue, newValue));
startValue = newValue;
});
by :
$(".bold").click(function () {
document.execCommand("bold", false);
var newValue = text.html();
startValue = stack.commands[stack.stackPosition]?.newValue || startValue
stack.execute(new EditCommand(text, startValue, newValue));
//startValue = newValue;
});
2- You can solve the second problem by replacing this code:
$("#text").bind("keyup", function() {
// a way too simple algorithm in place of single-character undo
clearTimeout(timer);
timer = setTimeout(function() {
var newValue = text.html();
// ignore meta key presses
if (newValue != startValue) {
// this could try and make a diff instead of storing snapshots
stack.execute(new EditCommand(text, startValue, newValue));
startValue = newValue;
}
}, 250);
});
by :
$("#text").bind("keyup", function () {
// a way too simple algorithm in place of single-character undo
clearTimeout(timer);
timer = setTimeout(function () {
var newValue = text.html();
startValue = stack.commands[stack.stackPosition]?.newValue || startValue // ajout
// ignore meta key presses
if (newValue != startValue) {
// this could try and make a diff instead of storing snapshots
stack.execute(new EditCommand(text, startValue, newValue));
//startValue = newValue; // modif
}
}, 250);
});
There appears to be a bug with the contenteditable demo, here's how it goes:
I've tried hitting save at all of the various intervals in the sequence but this is very persistent...