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
Merge transactions in undo manager #592
Comments
You should set |
How would that help? Then the state diff to select my shape would not be captured and not reverted |
Sorry for my misunderstood. First of all, the selection state should be in the awarenessStore, not in CRDT. In the undoManager, there is a |
It could be an idea to move the selection to the awareness store, but I am not sure if it makes my life easier. I use redux for my application and bind the redux store to yjs models with a custom binding that I have built: https://github.com/SebastianStehle/yjs-redux. The yjs stuff is just a recent addition and the redux store has existed way before that. Many operations also require the selection state to be changed. For example, when you add a new item you want this to be selected. When you ungroup a group you want to children to be selected. Therefore it was easy to move this to the main state and just implement it in the reducer. Ofc it could be also modeled in a middleware and then synced to awareness state or something like that, but anyway. Some other operations like deletion are also simpler with the current setup. So there are a wide range of reasons not to put the selection into the awareness state. No idea how deleteFilter would help. |
As @himself65 said, with
Now, do you want to ignore the selection state ( If you manipulate the I guess that what you are asking is that there should be a configuration option to disable tracking of all fields that are named Maybe there could be an option However, I agree with @himself65 that the selection state should be tracked using the awareness. 1. every user should be able to have a different selection. 2. you probably don't want to persist the selection state across sessions (e.g. when you reload the window). |
Perhaps my description was not precise and confusing. I want to "ignore" selection changes in a way that it does not add something to the undo stack but it gets merged with the next update. I have achieved it the following way: https://github.com/mydraft-cc/ui/blob/collaboration/src/wireframes/collaboration/services/extended-undo-manager.ts#L33 I don't see how I could implement it in another way. |
Ah okay, then you need to do something like you did. It is also possible to merge stack-items in Y.UndoManager. But currently this can only be done via If your solution works, then I'd like to close this ticket. I don't think we can cover really all use-cases in a single UndoManager, so it's nice to see that the current API helped to implement the desired behavior. |
Checklist
[ ] Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/ [ ] Try to report your issue in the correct repository. Yjs consists of many modules. When in doubt, report it to https://github.com/yjs/yjs/issues/
Is your feature request related to a problem? Please describe. I am using yjs for a diagram editor or I am working on the integration of yjs into this editor. In my editor the selection state is also part of the model and I would like to ignore that in the undo redo operations, or to be more precise, it should not be added to the stack.
Lets consider a very basic example with 2 transactions and the initial state 0:
Describe the solution you'd like If I just ignore transaction, then A would be selected after undo operation, which is not the same state as before. Therefore what I would like to achieve is that the transaction 1 is ignored but added to the undo stack after transaction 2 is completed.
Describe alternatives you've considered I think I can also implement it on top of the undo manager or implement it by adding additional metadata to the stack item as described in the docs, but I think this makes the most sense for me
Additional context Add any other context or screenshots about the feature request here.
Huly®: YJS-403
The text was updated successfully, but these errors were encountered: