Prosemirror SelectedNode Issue

[akashkamboj]

I checked everywhere and there is no solution to it.

• [x] 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/

Describe the bug Prosemirror provides the ability to create NodeSelection. But when content is updated through YJS. The node selection gets removed and become the TextSelection.

To Reproduce Steps to reproduce the behavior:

1. Go to Y-Prosemirror Demo
2. Insert an Image and click on the Image, the Image shows a border, indication of the NodeSelection.
3. Now if The other collaborator does any change in the document. The border goes away and it creates a TextSelection of that image.
4. This behaviour is very weird because I am showing a toolbar on Image Selection and that gets disappear every time.

Expected behavior Ideally it should retain the NodeSelection and not convert it to TextSelection.

Environment Information Everywhere

Additional context I can detect the changes and move TextSelection back to NodeSelection, that requires an additional update to the state and behaviour will not be smooth at all. That doesn't seem the right way to me. Ideally y-prosemirror should handle this scenario itself.

[akashkamboj]

I believe we can solve this here:

const restoreRelativeSelection = (tr, relSel, binding) => {
  if (relSel !== null && relSel.anchor !== null && relSel.head !== null) {
    const anchor = relativePositionToAbsolutePosition(binding.doc, binding.type, relSel.anchor, binding.mapping)
    const head = relativePositionToAbsolutePosition(binding.doc, binding.type, relSel.head, binding.mapping)
    if (anchor !== null && head !== null) {
      tr = tr.setSelection(TextSelection.create(tr.doc, anchor, head))
    }
  }
}

[dmonad]

Hi @akashkamboj, could you please create a PR with the change? If it doesn't break anything, I'm happy to merge it after I was able to test it.

[jamesopti]

We are seeing an error in this same function that is causing YJS to pickup an empty change and effectively wipe out our document by persisting it (TipTap + YJS client with Hocuspocus YJS server).

Any chance these are related? We've noticed this more when someone has a tab open with the document and comes back online after closing their laptop lid.

[jamesopti]

@akashkamboj Did you ever find a workaround to this?

We're seeing issues with NodeSelections turned into TextSelections as well, notably our drag handle implementation that lets you move an entire node.

We set the NodeSelection so that Prosemirror-view's drop handler will correctly delete the node on drop, but the NodeSelection turns into a TextSelection if someone else edits while youre dragging.

[milesingrams]

Have the same issue. I think on any change all node selections are converted to text selections. Not sure how to fix but assume it's related to the restoreRelativeSelection function. Perhaps if the original selection is a node then leave it as is?

[jevakallio]

@jamesopti I'm seeing the same issue with the same setup (TipTap + YJS client with Hocuspocus YJS server).

Did you have any luck fixing this?

For now I've patched y-prosemirror to not call tr.setSelection which avoid crashes and data loss, but I'm investigating the root cause and looking for a more appropriate fix.

[jamesopti]

@dmonad Any thoughts on this one? We've realized this makes collaborative editing with atom nodes (like an image) impossible as whenever user1 types, user2 loses their selection of the image.

[jamesopti]

Hi @akashkamboj, could you please create a PR with the change? If it doesn't break anything, I'm happy to merge it after I was able to test it.

I just threw up #118 after testing locally and verifying that my selected NodeView remains selected when another user types.

[akashbit]

hi @jamesopti I also did same kind of code to fix it temporarily . But this is not sufficient. Because apart from TextSelection and NodeSelection I also have CellSelection etc. And that becomes a bit complicated to calculate anchor and head.

[mweidner037]

hi @jamesopti I also did same kind of code to fix it temporarily . But this is not sufficient. Because apart from TextSelection and NodeSelection I also have CellSelection etc. And that becomes a bit complicated to calculate anchor and head.

For CellSelection, what worked for me is:

// oldState is old EditorState, tr is Transaction dispatched by y-prosemirror
if (oldState.selection instanceof CellSelection) {
    tr.setSelection(CellSelection.create(tr.doc, tr.selection.$anchor.before(), tr.selection.$head.before()));
}

[hamflx]

here's my solution: https://github.com/hamflx/y-prosemirror/commit/2ed27a029e47f292b388ac950a3fbdf5fdf8f30b

features:

1. no switch/case
2. no additional parameters

Use selection.map to record pos and convert to relative position when we need to save position.

Use the recorded pos to execute selection.map when we need to restore the position.

For custom SomeSelection, you only need to implement a proper map method to make the recovery of the selection work properly.

[akashkamboj]

@hamflx awesome work, it's working great. They should really merge it.

One more question can we utilize the same in cursors as well somehow. So that if a node is selected the collaborator can see the selection as an overlay over the node?