feat(collaboration): emit comments-update event for remotely-added comments#3079
feat(collaboration): emit comments-update event for remotely-added comments#3079mattConnHarbour wants to merge 2 commits intomainfrom
Conversation
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
|
📦 Preview published: npm install superdoc@pr-3079 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
hey @mattConnHarbour, direction's right :) one bug inline plus a question about event type.
| if (currentUser.name === user.name && currentUser.email === user.email) return; | ||
|
|
||
| // Capture existing comment IDs before loading new state | ||
| const existingIds = new Set(superdoc.commentsStore.commentsList?.map((c) => c.commentId || c.importedId) || []); |
There was a problem hiding this comment.
the loader and this code disagree on which id wins when a comment has both. loader (line 19) prefers importedId, here commentId wins. so if an imported comment later gets a commentId, or two people import the same docx and pick different commentIds, this code treats it as new and fires add for a comment that's already there. fix: match the loader at lines 104 and 112.
| const existingIds = new Set(superdoc.commentsStore.commentsList?.map((c) => c.commentId || c.importedId) || []); | |
| const existingIds = new Set(superdoc.commentsStore.commentsList?.map((c) => c.importedId ?? c.commentId) || []); |
| newComments.forEach((comment) => { | ||
| const commentValues = typeof comment.getValues === 'function' ? comment.getValues() : comment; | ||
| superdoc.emit('comments-update', { | ||
| type: comments_module_events.ADD, |
There was a problem hiding this comment.
comments-store.js emits 'add', CommentsLayer.vue emits 'new' for UI creation. does the listener that scrolls to remote comments use 'add'? if it keys off 'new', it won't fire.
3 participants
No description provided.