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

Readme section for undo/redo, proposed addToHistory command #111

Merged
merged 1 commit into from
May 14, 2022

Conversation

thatsjonsense
Copy link
Contributor

The proposed API mirrors prosemirror-history, for more info see https://github.com/ProseMirror/prosemirror-history/blob/master/src/history.js#L380

@dmonad
Copy link
Member

dmonad commented May 14, 2022

Thanks 👍
I implemented the API and will release it in a bit.

@dmonad dmonad merged commit be34a36 into yjs:master May 14, 2022
@thatsjonsense
Copy link
Contributor Author

Hey @dmonad , thanks so much for pushing this change. I've been testing it locally though and I'm still not seeing the intended behavior. It looks like when ProseMirror sends multiple transactions in a row, y-prosemirror is batching them together and only checking this meta key for the last one. This means that the whole batch is still actually getting captured.

image

Is there any way to prevent this?

@dmonad
Copy link
Member

dmonad commented May 25, 2022

I think this is fixed in v1.1.1, can you please verify?

@thatsjonsense
Copy link
Contributor Author

Hey @dmonad unfortunately not. I think the issue remains that when multiple prosemirror transactions happen in short sequence, and Yjs batches them together into its own transaction, only the last prosemirror transaction's addToHistory value is used:

image

@thatsjonsense
Copy link
Contributor Author

thatsjonsense commented May 25, 2022

I haven't tested this code, but I suspect a solution would look something like:

image

dmonad added a commit that referenced this pull request May 27, 2022
@dmonad
Copy link
Member

dmonad commented May 27, 2022

@thatsjonsense I think the next release should fix this issue.

@dmonad
Copy link
Member

dmonad commented May 27, 2022

If not, please write a test case so I can reproduce it better.

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

Successfully merging this pull request may close these issues.

2 participants