fix (sync plugin)[Issue #43]: Restore relativeSelection using a NodeSelection if appropriate

[jamesopti]

Changes

• Update getRelativeSelection to capture whether or not the current selection is a NodeSelection
• Update restoreRelativeSelection to use a node selection if restoring one

Context

Attempts to fix #43

[dmonad]

Could you please add a test-case for this?

[dmonad]

Can you please change this to type: 'NodeSelection' | 'TextSelection'? This would make it possible to potentially add more types of selections.

[dmonad]

How does this behave when multiple nodes are selected?

[jamesopti]

How does this behave when multiple nodes are selected?

Hmmm good question! I'm actually not sure how to test this.

[jamesopti]

According to the Prosemirror docs, a Node Selection is a selection that points at a single node.

I believe a range that points to mulitple nodes or ranges is still considered a Text Selection, however, I don't know how or if it is possible to select multiple specific nodes at once, so I cant say for sure.

[jamesopti]

@dmonad I think the complete solution here to handle custom selections (like Cell Selection for tables) is to accept an optional function parameter restoreSelection(tr, relSel) that a consumer could implement to handle their own selection restoration.

If you think that makes sense to do now, I'm happy to do it as part of this PR or as a follow up.

E.g.

const defaultRestoreSelection = (tr, anchor, head) => {
  const selection = relSel.type === 'node'
    ? NodeSelection.create(tr.doc, Math.min(anchor, head))
    : TextSelection.create(tr.doc, anchor, head)
  tr = tr.setSelection(selection)
}


export const ySyncPlugin = (yXmlFragment, {
  colors = defaultColors,
  colorMapping = new Map(),
  permanentUserData = null,
  onFirstRender = () => {},
  restoreSelection = defaultRestoreSelection
} = {}) => {

  ...

})

const restoreRelativeSelection = (tr, relSel, binding, restoreSelection) => {
  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) {
      restoreSelection(tr, anchor, head)
    }
  }
}

[jamesopti]

@dmonad Any chance we could get this merged soon?

[dmonad]

FYI this is pretty low on my priority list. I just took some time off. Now I need to catch up on quite a lot of stuff. The good thing is that it is on some kind of priority list I guess 😅

[amilich]

FYI this is pretty low on my priority list. I just took some time off. Now I need to catch up on quite a lot of stuff. The good thing is that it is on some kind of priority list I guess 😅

Hey all! Just noting that we could use this fix as well. Would help us out fixing some other selection types.

[amilich]

FYI this is pretty low on my priority list. I just took some time off. Now I need to catch up on quite a lot of stuff. The good thing is that it is on some kind of priority list I guess 😅

Hey all! Just noting that we could use this fix as well. Would help us out fixing some other selection types.

Checking in briefly on this

[rebolyte]

Just bumped into this as well. @jamesopti anything left before it can be merged?

[Alecyrus]

I have the same issue #43. I try this commit and it does not work, but #43 (comment) works.