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

On Tue, Aug 9, 2011 at 1:17 AM, Jonas Sicking <jonas at sicking.cc> wrote:

> 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:
> >> 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.
>

I've talked about this with Alex, and we both agreed that having
apply/reapply split is cleaner because in many cases you'd like to know
whether you're in redo or not.  i.e. more work is done in reapply than in
apply.

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 :) ).


I also talked about this with Alex.  We both agreed that the tricky part
will be when we add the transaction event.  It appears that the listener of
such an event wants to see a transaction object and access properties of it.
 Having to call methods on undoManager in the listeners seem unnatural.

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

A big advantage I see with my proposal is that it explicitly creates an
object, and makes it easier for scripts to add more properties.  In the case
of manual transactions, apply functions most definitely need to add some
properties on the transaction object in order to remember what has been
removed or added.

But I do agree that having to do "new ManagedTransaction" every time you
create a transaction may be too verbose.  Alex proposes that we make the
transaction object duck-typed.  So we'll have something like:

myEditor.undoManager.managedTransaction(
 {apply: function() {...}, label: "..."});
myEditor.undoManager.manualTransaction(
 {apply: function() {...}, unapply: function() {...}, reapply: function()
{...}});

It's much less verbose than my proposal but provides a way for scripts to
add arbitrary objects of their choice.

- Ryosuke

Received on Tuesday, 9 August 2011 12:03:13 UTC