search

yjs / y-prosemirror

ProseMirror editor binding for Yjs
https://demos.yjs.dev/prosemirror/prosemirror.html
MIT License
355 stars 124 forks source link

Cursor is placed in a strange position after changing a node type and undoing it. #101

Open vitmf opened 2 years ago

vitmf commented 2 years ago

Describe the bug Cursor is placed in a strange position after changing a node type and undoing it.

To Reproduce Steps to reproduce the behavior:

  1. type some text in the first line.
  2. type some text in the second line.
  3. change the second line type to heading.
  4. press ctrl+z.
  5. the cursor is placed at the beginning of the second line.
  6. press backspace.
  7. the border of the first line turns to blue.
  8. press backspace again.
  9. the first line is deleted.

Expected behavior After ctrl+z, the cursor should stay at the end of the second line. Even if the cursor is placed at the beginning of the line, backspace should not create the blue border.

Screenshots https://user-images.githubusercontent.com/640041/160831412-2a79ee9d-0ad1-4790-844f-3357ac42e4d0.mp4

Environment Information Chrome 98.0.4758.102 on the demo page.

Additional context Testing in a local project, it seems the cursor is placed between nodes.

vitmf commented 2 years ago

I don't understand yjs very well, but I think I have some clues.

  1. Shouldn't this ystate initialization be inside the event callbacks? https://github.com/yjs/y-prosemirror/blob/e3755b12afca45f2253aee44b3f3bf23803dc9b9/src/plugins/undo-plugin.js#L73-L80
  2. After an undo, the stack-item-popped event seems to be triggered after the document has been restored, so binding.beforeTransactionSelection is only updated after the selection has already been restored.
  3. As I understand the undo plugin, after a transaction prevSel is saved in the plugin state so it can be added to the meta of the undo stack item. However, if UnderManager merges the edits, won't prevSel refer to a wrong position? (edit: reading more about yjs, this does not seem to be a problem, but more generally, could prevSel refer to the wrong position?)