y-prosemirror
y-prosemirror copied to clipboard
fix (sync plugin)[Issue #43]: Restore relativeSelection using a NodeSelection if appropriate
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 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)
}
}
}
@dmonad Any chance we could get this merged soon?
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 :sweat_smile:
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.
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
Just bumped into this as well. @jamesopti anything left before it can be merged?
I have the same issue https://github.com/yjs/y-prosemirror/issues/43. I try this commit and it does not work, but https://github.com/yjs/y-prosemirror/issues/43#issuecomment-1253668582 works.