osuresearch / annotator Goto Github PK

I misinterpreted the spec at some point, this should be document IRI not a field within the document (as I was using it previously). Requires a bit of refactoring of how I'm re-associating threads to targets.

This also brings up a potential issue with text position selectors not knowing which particular field to target. May require changing the IRI to something like UUID#field

Outstanding issues with the comment threading feature for me to track, in no particular order:

Bugs

• Fix new comment views not being linkable to threads (somehow the updated Redux state of threads isn't available to focus on?)
• Delete comment-view marks when threads are deleted
• Restore comment-view marks when threads are restored
• osuresearch/ripple#2
• Better context tracking. Prose has bookmarks (not through Tiptap) that might be useful: https://prosemirror.net/docs/ref/#state.Selection.getBookmark but it may also be a simplified version (e.g. just matching text to text)
• Omit reply box when creating a new thread

Features

• #6
• Jump the page to the comment-view of an active thread in list mode
• Smoother resizing of threads when focused
• Highlight comment-view mark when the thread has focus
• Block-level comment anchors on Field and collection instances
• #5
• comment-view restoration when loading in new comments (ties into the delete/restore bug)
• #4
• osuresearch/ripple#3

Documents that have anchors going to external websites (or linking to documents) without opening in a new tab will cause the iframe to attempt to load cross-domain content, which of course crashes the whole thing.

Either the links need to be sanitized or disabled to ensure they can't open in the same frame when parsing a source document.

Reviewable external documents need to autogenerate a TOC from heading levels to reach feature parity with the legacy document review component. Docusaurus uses a third party library for this, might be able to leverage the same.

Add a set of feature flags we can toggle in the main provider. Ideas so far:

• Ability to mark a thread as resolved
• Ability to ask questions instead of just leaving comments (may be tied with the above)
• Ability to delete threads that already have replies

That below block shouldn't be visible. It's redundant in this edge case.

The packing algo in useCellList will execute ~4x each time we click a NoteAnchor due to various focus changes, container height changes, and cell position changes that all happen in various render cycles. Since there's a sort in there as well, performance isn't going to scale well.

Throttle the packing algo, or figure out some other way to reduce call count through some memoization magicks.

My notes / TODO on the integration with Adobe's DC viewer

• Delete annotations in the PDF when our thread is removed
• Migrate thread data structure to W3C's annotation spec
• Focus annotation in the PDF when the thread is focused
• Reapply annotations in the PDF when our thread is reapplied (undo) and during initial mount
• Remove unsupported features from the PDF reader (drawing tools, strikethrough, exports, etc)

Typical double rendering issue - anchors end up duplicating in the useAnchors item list which causes various components to render twice into portals in the iframe DOM.

I've spent a good amount of time trying to trace this down, but no luck so far.

Repro:

• Select some text
• Start a new thread
• Do this over and over until you run into the issue (lol).

Basically, since the thread's Tiptap instance has keyboard focus on mount and the thread is usually in mid transit down the page due to our translateY rule telling to hurry down to the target anchor - there's a chance that the browser will attempt to focus on the element when it's somewhere up higher on the page before the animation has settled.

Lazy solution: Focus delay that's longer than the animation + packing speed.

Better solution: TBD.

Really no longer need the custom RUIAnnoSelector type. Anything we can annotate can conform to W3C now. The only annotator-specific data is attached as a body to each annotation.

Thus, use the following selectors instead:

RUIAnnoSelector.subtype == 'highlight' will use a TextPositionSelector instead:

"target": {
  "source": "XXXX-XXX-XXX-XXXX",
  "selector": {
    "type": "TextPositionSelector",
    "start": 412,
    "end": 795
  }
}

RUIAnnoSelector.subtype == 'note' to be:

"target": {
  "source": "XXXX-XXX-XXX-XXXX",
  "selector": {
    "type": "FragmentSelector",
    "value": "Field5",
  }
}

Instead of using source to denote which field is being annotated, target.source will now specify the source document being annotated to better conform to the standard and allow us to combine multiple annotation sources together without conflict. E.g. annotations against multiple instances of the same form, or annotations that are for a parent form + any attached documents, each with their own IRI.