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

Undo/Redo #1008

Open
aboodman opened this issue Jul 31, 2022 · 9 comments
Open

Undo/Redo #1008

aboodman opened this issue Jul 31, 2022 · 9 comments
Labels
Future Something we want to fix but is not blocking next release

Comments

@aboodman
Copy link
Contributor

aboodman commented Jul 31, 2022

Undo/redo is a very common feature request in collaborative applications and doing it correctly is somewhat subtle (e.g., see posts by Figma and Liveblocks).

Many Replicache users have created undo/redo on their own without too much trouble but it would be good to provide a solid, tested library analogous to https://github.com/rocicorp/fractional-indexing and other helper libraries we've released.

@arv
Copy link
Contributor

arv commented Oct 20, 2022

What else needs to be done here? Is https://github.com/rocicorp/undo enough to close this?

@cesara

@arv arv added the Future Something we want to fix but is not blocking next release label Oct 20, 2022
@cesara
Copy link

cesara commented Oct 21, 2022

@arv there is a particular scenario that undo is not being handled correctly.

Reference our Redoing undo notion doc for a clearer understanding of the issue.

We can open a new Issue to handle the invariant.

@arv
Copy link
Contributor

arv commented Oct 21, 2022

I see. This is still future work then.

@aboodman aboodman moved this from In Progress to Planned in Replicache Roadmap Nov 2, 2022
@heymartinadams
Copy link

Can we use this library already in production, or should we hold off until you launch this feature as part of the official Replicache? Undo/redo is a highly requested feature for us.

@aboodman
Copy link
Contributor Author

Here is the notion doc @cesara mentions: https://replicache.notion.site/Redoing-undo-c0183b91d12c4272a3482e3233cc2890?pvs=4.

It describes the edge case which this library doesn't handle.

@isaacHagoel
Copy link

I think the notion doc misinterprets the "Figma principle" by ignoring the "and nothing else has happened in between" bit.

if the user presses undo n times, then redo n times, and nothing else has happened in between, then the document should end up how it was before pressing undo.

I think it would be more correct to think about the problem in the following way (everything I say below is in line with the Figma principle):
The purpose of Undo is to allow the user to experiment safely and roll-back mistakes. Destroying work by another user (and potentially corrupting the entire "document") shouldn't be a possible outcome. Therefore Undo should only be allowed if it is "conflicts free" (meaning - the unit on which the undo is about to operate was has the same value that the undoing user put there).
Redo can be thought of as Undo of the last Undo. If undo is only allowed when it's conflicts free, a redo just mean re-applying the original mutation. It should also only be allowed as long as it's conflict free (== no one else changed the value after the undo that's about to be reverted).

When these conditions are not met the corresponding undo/redo operations should be skipped (and canUndo/canRedo should reflect it if there are not other actions the user can legally undo/redo).

Otherwise, it leads to nonsensical results (user A clicks redo and that behaves as if user B clicked undo, confusing both)
I've been reading some academic paper on the subject (this is an interesting one, is talks about an API that contains transpose and hasConflict functions) and planning to do more research and play with some existing systems to see what they do.
Putting this here in case it is helpful and/or in case someone wants to point out some error in my thinking.

@aboodman
Copy link
Contributor Author

What you describe is very different than what Figma or any other collaborative editor I've ever used does, in particular:

Therefore Undo should only be allowed if it is "conflicts free" (meaning - the unit on which the undo is about to operate was has the same value that the undoing user put there).

and:

When these conditions are not met the corresponding undo/redo operations should be skipped (and canUndo/canRedo should reflect it if there are not other actions the user can legally undo/redo).

You can easily experiment in Figma and see that it doesn't behave this way. The video I pasted above shows one example.

Our goal is to match the Figma behavior. There are some edge cases where we don't currently. The Notion doc describes the problem and a potential solution.

@isaacHagoel
Copy link

it's actually very normal to lose the ability to Redo once the state has diverged from the state that the "matching" undo operation created. Every app I can think of does that. Here is a quick example from Canva (notice how typing after undoing disables the redo button):
https://github.com/rocicorp/replicache/assets/20507787/b0141313-1397-44aa-bde5-a7a1458efef3
I tried to apply the same principle to conflicting edits done by other users (skipping might be a bad idea 😄).
I don't think any multiplayer app 100% figured out the general-case best-practices yet, not even the mighty FIgma.

Sounds like you have a clear goal though, and I respect that. I think you guys are awesome. All the best!

@isaacHagoel
Copy link

Just for future reference in case anyone finds it useful. I've dug into the subject, created a POC implementation and wrote this: https://dev.to/isaachagoel/you-dont-know-undoredo-4hol

One of the readers pointed me to the undo/redo implementation by ProseMirror that implements similar ideas:
https://github.com/ProseMirror/prosemirror-history/tree/master/src

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Future Something we want to fix but is not blocking next release
Projects
Status: In Progress
Development

No branches or pull requests

5 participants