Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Way to bundle change updates, so they get undo-ed together #1456

Closed
stochris opened this issue Dec 7, 2017 · 9 comments
Closed

Way to bundle change updates, so they get undo-ed together #1456

stochris opened this issue Dec 7, 2017 · 9 comments

Comments

@stochris
Copy link

stochris commented Dec 7, 2017

Do you want to request a feature or report a bug?

I would to request a feature

What's the current behavior?

Currently, there is no way to bundle change operation together so they get undo-ed at the same time.

What's the expected behavior?

It would be useful, especially when using a table, to have operation done on multiple cells be undo-ed all at the same time ( for example, when you delete text from multiple cells, undo would undo all the deletes, not just the first )

@Nantris
Copy link
Contributor

Nantris commented Dec 7, 2017

Was just asking about this. Would love a method for this.

@thesunny
Copy link
Collaborator

I haven't looked into the internals but just wanted to start a discussion of how the developer might bundle actions together.

In my opinion, it should work something like this:

  • By default, all operations submitted at the same time get bundled together into a single undo. Usually this is a reasonable default. For example, a "delete column in table" action would be executed at once so all its operations would be grouped togehter. Can't think of too many situations where this isn't true.
  • If operations need to be broken down further, use something like change.doSomething().doSomethingElse().bundle().andDoThis().bundle().andFinallyDoThis(). There is an implicit bundle() at the start and end. The in between bundles separate other groups of operations.

I like this because by default the editor should normally just do the right thing (I think).

@ianstormtaylor
Copy link
Owner

Doesn't it merge everything right now? There's also a merge operation flag that might be used to get this functionality.

@thesunny
Copy link
Collaborator

LOL! Yeah, it does exactly that. Because of the comment earlier about deleting multiple texts, I assumed it was saying that it chunked them more granularly by default.

@stochris
Copy link
Author

stochris commented Jan 8, 2018

@ianstormtaylor could you provide a link to the merge flag implementation or to the docs? I couln't find anything about that.

@stochris
Copy link
Author

stochris commented Jan 8, 2018

I'm using the deleteAtRange method for every cell content which in turn which calls change.snapshotSelection(). That creates a slight problem, since every change operation is seen as a separate action. Is there any way to add a flag that skips this ?

@maddrag0n
Copy link

@ianstormtaylor any news on this one?

@zGrav
Copy link
Collaborator

zGrav commented Jan 22, 2018

A workaround has been made for this, but it would still be interesting that deleteAtRange undoing would act more like removeNodeByKey undoing.

@ianstormtaylor
Copy link
Owner

I believe that this may be fixed by #3093, which has changed a lot of the logic in Slate and slate-react especially. I'm going to close this out, but as always, feel free to open a new issue if it persists for you. Thanks for understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants