feat: inline comments - #8901
Conversation
github-project-automation
Bot
moved this to 🧭 Planning evaluation (don't pick)
in 📝 Productivity team
mejo-
moved this from 🧭 Planning evaluation (don't pick)
to 🏗️ In progress
in 📝 Productivity team
0ecbdff to
8eda11e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
When being a guest, I can not edit previous comments before I enter a guestname, but I can delete them. Is that intentional? 
|
Good catch. I fixed this by switching to
Another good catch. Fixed by setting a
Indeed, that's inconsistent. I changed this now that deleting is only possible after adding a guest name as well for consistent UX. I also did the following:
|
There was a problem hiding this comment.
Very cool! Some first feedback:
- Showing/hiding annotations changes the text flow a little bit, which makes things look unstable/brittle. That might be because the comment emoji is aligned a bit to the top? It’s fine if it’s vertically centered, it would be more important for the text not to move vertically. :)
- The comment field is quite large / high, which makes it a bit massive and imposing. It would be ok to reduce it to the height of 3.5 lines of text, or even a single line if we use autosize.
- The "Edit comment" and "Delete comment" actions can go into a 3-dot action menu. Then they can have proper text and are not so in-your-face.
- A quick animation moving the comment field down when sending the comment would be nice. Better than an interface jump
- Right now it is only possible to show a single comment thread, right? Other solutions make it possible to view all comments at the same time, which is especially useful if there are multiple and not really many replies going on.
I would like to keep this for a follow-up, as I have to say I find it quite confusing how others display all the comments per default and I think we should think this through properly. All other comments addressed @jancborchardt |
The plugin post-processes data from footnotes plugin. As we cannot change the label parsing rules of the footnotes plugin, this seemed to be the most sensible thing to do. Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-opus-4-8
The comments container is hidden in UI for now. Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org>
The command is registered in an own Tiptap extenstion that requires a browser. In the comment node unit tests, we don't have this extension loaded. Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Avoids the bubble growing until viewport top and overlapping with viewer header and close button. Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
This is not technically necessary but creates a more consistent user experience, as all editing actions are only available after setting a name now. Signed-off-by: Jonas <jonas@freesources.org>
Use a NodeView in commentReference to allow adding a click handler. Use a button element instead of link in there to make the reference accessible via tab navigation. Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Allows to insert a comment with a straight-forward keyboard flow: 1. Insert comment via Ctrl-Alt-M 2. Type comment and submit via Ctrl-Enter 3. Close bubble via Esc 4. Continue typing in document Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Also ensures guest name is reactive everywhere, regardless of whether it got set via comment bubble or guest name dialog. Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Jonas <jonas@freesources.org> Assisted-by: OpenCode:claude-fable-5
Signed-off-by: Jonas <jonas@freesources.org>
Test idempotence via markdownThroughEditor round-trip for different unexpected/broken comment Markdown syntax. Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Jonas <jonas@freesources.org>
| function isDisabled(actionEntry, editor) { | ||
| return actionEntry.action && !actionEntry.action(editor.can(), editor) | ||
| return (actionEntry.action && !actionEntry.action(editor.can(), editor)) | ||
| || (actionEntry.isAnnotation && annotationsHidden.value) |
There was a problem hiding this comment.
minor nitpick:
I don't like this annotation specific logic in the generic menu handler.
I think the check could be moved into the action function of both footnote and comment buttons. But then I am not sure if it wouldn't be better to make annotationsHidden a part of the editor itself. That way one could also prevent creating annotations with keyboard shortcuts, when they are hidden.
| onUnmounted(() => props.editor.off('update', onUpdate)) | ||
|
|
||
| const commentNode = computed<Node | null>(() => { | ||
| void editorVersion.value |
There was a problem hiding this comment.
Never saw that syntax before. I assume it's for making the commentNode computed depend on editor version, right?
There was a problem hiding this comment.
That seems like a fairly complex way to trigger vue computed updates from prosemirror to then trigger a prosemirror processing to get the node and update the computed.
How about having a Ref<Node|null>, moving the computed function into the onUpdate and setting the ref explicitly?
There was a problem hiding this comment.
Ah... I see you also want it to depend on props.referenceId 🤔
There was a problem hiding this comment.
How about having a reactive map (either an actual Map or an object) from referenceId to the node in question that is updated for all comment nodes in the update handler and then a computed that picks the one for props.referenceId?
That way the prosemirror part would be separated from the vue reactivity part.
| .openCommentBubble(referenceId) | ||
| .run() | ||
| }, | ||
| addCommentReply: (referenceId: string, text: string) => ({ state, dispatch }) => { |
There was a problem hiding this comment.
minor nitpick:
I think I would expect this to take either the comment position or the comment itself as the first argument. The comment node is readily available in the bubble view and that way the command would not have to know about reference ids.
|
|
||
| const itemsContainer = ref<HTMLElement | null>(null) | ||
| const replyText = ref('') | ||
| const replyText = ref(sessionStorage.getItem(`${DRAFT_KEY_PREFIX}${props.referenceId}`) ?? '') |
There was a problem hiding this comment.
Is the reference id unique across different files? Or is sessionStorage somehow scoped to one file?
4 participants
📝 Summary
This PR implement inline point-anchor comments as discussed in #185.
[?]or by pressing Ctrl-Alt-MMarkdown representation
Comments are stored using the footnote syntax already supported by Text, with a comment- prefix to distinguish them from regular footnotes:
Existing Markdown editors with footnote support (Obsidian, Ghostwriter) render the comment definitions as footnotes and make them accessible.
🖼️ Screenshots
🖼️ Screencast
🚧 TODO
Regression: Undo is now always disabled in toolbarWould be great if adding/editing/deleting replies could be undoable as welledit: turns out it already is 🎉🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)🤖 AI (if applicable)