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

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

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

For example, in editing apps, you have to restore selection on redo.
 Presumably, the UA can auto-restore selection in managed transactions but
restoring selection in manual selection is hard since DOM after redo might
look completely different from the DOM after the first apply.

(I've sent out a massive email to a bunch of Googlers who have worked on
RTE, but unfortunately some of them are OOO until mid or late September so
please be patient with me about developer feedbacks).

On the other hand, what are use cases that are better addressed by your
proposal?  Do you have specific examples in your mind?  Also, the following
code will provide the same functionality as your proposal:
apply: function () {init(); action();}
unapply: unapply
reapply: action

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
>   });
>

Yeah, this looks much cleaner than the current syntax.  Let's make this
change then (duck typed object).

- Ryosuke

Received on Tuesday, 9 August 2011 14:44:32 UTC