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

[Jonas Sicking]

On Tue, Aug 9, 2011 at 12:03 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

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

Could you please provide examples. I feel like I'm fighting an elusive
shadow.

I.e. please provide an example where "apply" isn't just init+reapply. "There
are many cases" isn't a particularly compelling argument unless you can show
these cases :-)

And ideally also some estimate how common that case will be compared to
"apply" simply being init+reapply. This is important since if that is very
rare, people can always implement it themselves using init+reapply semantics
by having a flag on the object which indicates if you're in the first call
or not.

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

I like this! (except for the apply/reapply/unapply split, please do answer
the question above)

The example used in this thread would become something like (independent of
if we use init/apply/unapply or apply/reapply/unapply):

myEditor.undoManager.manualTransaction(
 { apply: function () { this.nodeBefore.parentNode.insertBefore(this.text,
this.nodeBefore); },
    unapply: function () { this.text.parentNode.removeChild(this.text); },
    text: document.createTextNode('hello'),
   nodeBefore: window.getSelection().anchorNode
  });

/ Jonas