[whatwg] Fixing undo on the Web - UndoManager and Transaction

[Jonas Sicking]

On Tue, Aug 9, 2011 at 12:42 AM, Ryosuke Niwa <rniwa at webkit.org> wrote:
> On Tue, Aug 9, 2011 at 12:31 AM, Jonas Sicking <jonas at sicking.cc> wrote:
>>
>> I still see UndoManager.replace in there. I still haven't heard any use
>> cases that won't be solved better with a beforeEditingAction event (and
>> solved ok simply using the undo() function until we have a
>> beforeEditingAction standardized).
>
> Right. I think we should remove it but I wanted to go talk with a bunch of
> developers first so they're still pending.
>>
>> Likewise I still haven't heard of any examples where the apply function
>> isn't simply init+reapply. So it still seems better to me to have a
>> init/apply/unapply split rather than a apply/reapply/unapply split.
>
> This is also pending for developer feedback.

Excellent. For what it's worth, apart from those two items, the only
difference between our proposals is the use of explicit Transaction objects,
which is purely a syntactical difference. I would be super interested to get
developer feedback on that difference too (though I think I know what Alex
Russell will say :) ). The difference comes down to something like this:

Creating a manual transaction with your proposal (assuming we use
init/apply/unapply):

myEditor.undoManager.transact(
  new ManualTransaction(
   function () {
      this.text = document.createTextNode('hello');
     this.nodeBefore = window.getSelection().anchorNode;
    },
   function () { this.nodeBefore.parentNode.insertBefore(this.text,
this.nodeBefore); },
    function () { this.text.parentNode.removeChild(this.text); },
 );

Creating a manual transaction with my proposal:

var text = document.createTextNode('hello');
var nodeBefore = window.getSelection().anchorNode;
myEditor.undoManager.manualTransact(
 function () { nodeBefore.parentNode.insertBefore(text, nodeBefore); },
  function () { this.text.parentNode.removeChild(this.text); }
);

The main advantage that I see with your proposal is that it makes it easy to
create a reusable subclass for a ManualTransaction:

function TextTransaction(text) {
  this = new ManualTransaction();
 this.text = document.createTextNode('hello');
  this.nodeBefore = window.getSelection().anchorNode;
 this.apply = function () {
this.nodeBefore.parentNode.insertBefore(this.text, this.nodeBefore); };
  this.unapply = function () { this.text.parentNode.removeChild(this.text);
};
 return this;
}

Which you could then use like this:

myEditor.undoManager.transact(new TextTransaction("hello"));

So it does seem like the transaction object creates a nice scoping
opportunity, as well as nice syntax when subclasses are used to create
reusable code.


As for managed transactions. The difference looks something like:

myEditor.undoManager.transact(new ManagedTransaction(
 function() {
    var nodeBefore = window.getSelection().anchorNode;
  nodeBefore.parentNode.insertBefore(document.createTextNode('hello'),
                                     nodeBefore);
 }));

vs., with my proposal:

myEditor.undoManager.transact(
 function() {
    var nodeBefore = window.getSelection().anchorNode;
  nodeBefore.parentNode.insertBefore(document.createTextNode('hello'),
                                     nodeBefore);
 });

Here the difference is much smaller. However I also can't think of an
advantage of the transaction object.

One possibility would be to use transaction objects only for manual
transactions, and a plain callback for managed ones.

/ Jonas