Abstraction for multi-line decorations

[hanspagel]

Describe the bug We’re trying to set up annotations with tiptap 2 + y-prosemirror and good news first: It’s working.

We’re storing the annotations and their relative position in a Y.js map. For changes within the same editor we just apply the ProseMirror mapping to the deocoration. When Y.js updates the document, we’re recalculating the absolute position with relativePositionToAbsolutePosition.

Unfortunately, there are few cases where that functions returns unexpected results. I’m not really sure if we’re using it wrong, or if it’s a bug in y-prosemirror.

Steps to reproduce the behavior

1. Go to https://sbdbm.csb.app/
2. Add a selection to the above editor
3. Click on the “comment“ button and add some text
4. The editor and the one below should both have the same annotation
5. Click before or in the middle of the annotation and hit Enter
6. Both editors have different annotations.

Expected behavior Somehow relativePositionToAbsolutePosition doesn’t seem to work well when the nodes are splitted. I’d expect to get the position inside of the newly created node, but all results seem to be stuck inside the first node.

GIF

Environment Information

• Chrome
• y-prosemirror 1.0.7
• yjs 13.5.4

Additional context Here is how we store the relative positions of an annotation: https://codesandbox.io/s/tiptap-collaboration-annotation-sbdbm?file=/src/extension/AnnotationState.ts:1157-1713

And here is how we create decorations from those relative positions: https://codesandbox.io/s/tiptap-collaboration-annotation-sbdbm?file=/src/extension/AnnotationState.ts:2224-2767

I hope you’ll just say we need to use another utility function and it’s working fine. :) Anyway, I know it’s pretty complex, so let me know what I can do to make debugging easier for you.

[dmonad]

In Yjs, every character can be uniquely identified. The "relative positions" point to the unique ids of the character in the text. When a paragraph is split, y-prosemirror copies the tail of the first paragraph to a new paragraph. For technical reasons, we can't retain the unique ids of the copied characters. So the expected outcome is that the decoration ends at the end of the first paragraph (as it is still pointing to the deleted characters that are just hidden).

You re-render the decorations correctly when you receive remote changes. However, when creating a local change, you also need to re-render the decorations. Once the remote client also applied a change, you re-render the local selection and all clients end up with the same decorations.

Relative positions provide a very nice solution that just works in most of all cases. But for this specific case - if you want to support multi-line comments - you need to adjust the ending position when the paragraph is split. There is, unfortunately, no way around it.

I dislike the y-prosemirror API for relative positions. Even the Yjs API for relative positions is hard to understand. I think it would make sense to provide an abstraction for multi-line decorations that rerenders automatically, that adjusts the position when the paragraph is split.

Thanks for providing a sandbox for the TipTap code, this is super useful :)

[SamDuvall]

@hanspagel,

Sorry, I'm a bit late to the party here and I'm not 100% sure if this is the correct place for a TipTap-related discussion, but what made you decide to store the position within the comment record? I was thinking about using a Y.Map from commentId to comment (data) as well, but I was thinking about storing the commentId(s) as a comment Mark instead. Is this something you considered and rejected? If so, I'd love to know why so I can understand the solution you came up with a little better.

Thanks!

[hanspagel]

Making it a Mark would make things easier, yes.

You’d probably expect a different behaviour in some cases though, for example when copying text. Decorations won’t be copied, Marks will.

[jessicalc]

Hi @dmonad !

However, when creating a local change, you also need to re-render the decorations.

We've been trying to do this and inserted a this.createDecorations call in the plugin state's apply method here (which we forked from the csb @hanspagel linked): https://codesandbox.io/s/tiptap-collaboration-annotation-forked-2stod?file=/src/App.vue:491-501

Are we missing something as we try to re-render the decorations? Thanks a lot!

[dmonad]

@jessicalc This is quite a lot of code. It seems that you are correctly transforming ranges to and from RelativePositions. However, it seems that you are not rerendering all decorations when a decoration is added. You are still using the ProseMirror mapping approach:

    this.decorations = this.decorations.map(
      transaction.mapping,
      transaction.doc
    );

I meant that you need to recreate the decorations from relative positions to make sure that everyone ends up with the same content.

[jamesopti]

@dmonad @hanspagel - I'm trying to determine if this is conceptually any different from the collaboration cursor behavior.

This GIF shows the editors staying in sync for cursor selection decorations as the paragraphs are broken up. The last paragraph getting broken out and losing selection does happen, but it is in sync across both.

Would it be possible to use the same mapping logic for annotation decorations?

[jpobley]

I have a fork of the original sandbox that @hanspagel put together, and it has the desired effect, but it's a little rough around the edges and I'm unsure if it's correct. Here's the code.

Also, it's specific to this Annotations plugin--I have no idea what an abstraction would look like or where it would go.

As @dmonad pointed out, once we call:

this.decorations = this.decorations.map(
  transaction.mapping,
  transaction.doc
);

ProseMirror will have done the work of mapping the decorations to their new positions, and we just need use those new absolute positions to update the relative positions in our YMap:

this.options.map.doc.transact(() => {
  this.decorations.find().forEach((deco) => {
    const {from, to} = deco;

    const newFrom = absolutePositionToRelativePosition(
      from,
      ystate.type,
      ystate.binding.mapping
    );

    const newTo = absolutePositionToRelativePosition(
      to,
      ystate.type,
      ystate.binding.mapping
    );

    const {id} = deco.spec;
    const annotation = this.options.map.get(id);

    annotation.from = newFrom;
    annotation.to = newTo;

    this.options.map.set(id, annotation);
  });
}, AnnotationPluginKey);

Some issues:

1. Once a decoration is "split", you'll notice there's a delay before the decorations redraw for each client. There needs to be an event published either by the ysync-plugin or by a ProseMirror transaction, but I'm not sure which. Probably ysync-plugin since all the clients reach the desired state once an awareness update is published.
2. We need to handle removed decorations. DecorationSet.map() supports an onRemove callback for this case but thought needs to be put into what to do with a "detached" annotation.
3. Related to the previous point, undo and redo have weird behavior.

[jamesopti]

@jpobley - Great find! I'm using that update here to try and implement "block" level annotations via Decoration.Widget.

Its close, but the syncing isn't perfect (sometimes doesnt render correctly on the remote editor when Backspace deletes a paragraph).

https://codesandbox.io/s/elastic-waterfall-721bj?file=/src/extension/AnnotationState.ts

[joacub]

this is not gonna work with versions, best approach is using marks, im already using and works nice and with snapshot, this approach is too hard to keep in sync with the snapshots and cursors, the behavior of copying and keeping marks is very easy to skip just making a handler when paste and cleaning this marks if that behaviors is not wanted.