diff --git a/packages/super-editor/src/extensions/comment/comments-plugin.js b/packages/super-editor/src/extensions/comment/comments-plugin.js index b739ead610..393d46bb65 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.js @@ -408,14 +408,24 @@ export const CommentsPlugin = Extension.create({ ({ state, editor }) => { const { from } = findRangeById(state.doc, id) || {}; if (from != null) { - state.tr.setSelection(TextSelection.create(state.doc, from)); - if (options.preferredActiveThreadId) { - state.tr.setMeta(CommentsPluginKey, { + const tr = state.tr; + tr.setSelection(TextSelection.create(state.doc, from)); + if (options.activeCommentId) { + tr.setMeta(CommentsPluginKey, { + type: 'setActiveComment', + activeThreadId: options.activeCommentId, + forceUpdate: true, + }); + } else if (options.preferredActiveThreadId) { + tr.setMeta(CommentsPluginKey, { type: 'setCursorById', preferredActiveThreadId: options.preferredActiveThreadId, }); } - if (editor.view && typeof editor.view.focus === 'function') { + // Skip view.focus() when activating from the sidebar (activeCommentId set). + // Focusing the hidden PM view can trigger a DOM selection sync transaction + // that overwrites the activeThreadId via position-based detection. + if (!options.activeCommentId && editor.view && typeof editor.view.focus === 'function') { editor.view.focus(); } return true; @@ -443,7 +453,6 @@ export const CommentsPlugin = Extension.create({ decorations: DecorationSet.empty, allCommentPositions: {}, allCommentIds: [], - changedActiveThread: false, trackedChanges: {}, }; }, @@ -473,7 +482,6 @@ export const CommentsPlugin = Extension.create({ return { ...pluginState, activeThreadId: newActiveThreadId, - changedActiveThread: true, }; } @@ -525,7 +533,7 @@ export const CommentsPlugin = Extension.create({ } } - return pluginState; + return { ...pluginState }; }, }, }; diff --git a/packages/super-editor/src/extensions/comment/comments-plugin.test.js b/packages/super-editor/src/extensions/comment/comments-plugin.test.js index 7725197754..97fc239e07 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.test.js @@ -383,6 +383,36 @@ describe('CommentsPlugin commands', () => { expect(editor.view.focus).not.toHaveBeenCalled(); }); + it('sets the active thread without focusing the hidden view when requested', () => { + const schema = createCommentSchema(); + const commentMark = schema.marks[CommentMarkName].create({ commentId: 'c-10', internal: true }); + const paragraph = schema.node('paragraph', null, [schema.text('Hello', [commentMark])]); + const doc = schema.node('doc', null, [paragraph]); + const { commands } = createEditorEnvironment(schema, doc); + + const tr = { + setSelection: vi.fn(), + setMeta: vi.fn(), + }; + const editor = { + view: { focus: vi.fn() }, + }; + + const result = commands.setCursorById('c-10', { activeCommentId: 'thread-1' })({ state: { doc, tr }, editor }); + + expect(result).toBe(true); + expect(tr.setSelection).toHaveBeenCalled(); + expect(tr.setMeta).toHaveBeenCalledWith( + CommentsPluginKey, + expect.objectContaining({ + type: 'setActiveComment', + activeThreadId: 'thread-1', + forceUpdate: true, + }), + ); + expect(editor.view.focus).not.toHaveBeenCalled(); + }); + describe('addCommentReply', () => { it('emits commentsUpdate event with parentCommentId', () => { const schema = createCommentSchema(); @@ -609,7 +639,6 @@ describe('CommentsPlugin state', () => { const pluginState = CommentsPluginKey.getState(view.state); expect(pluginState.activeThreadId).toBe('thread-1'); - expect(pluginState.changedActiveThread).toBe(true); }); it('stores decorations provided through metadata', () => { diff --git a/packages/super-editor/src/extensions/comment/comments.test.js b/packages/super-editor/src/extensions/comment/comments.test.js index 02ec04929d..70fb61c1af 100644 --- a/packages/super-editor/src/extensions/comment/comments.test.js +++ b/packages/super-editor/src/extensions/comment/comments.test.js @@ -880,7 +880,6 @@ describe('comments plugin pm plugin', () => { }; const nextState = plugin.spec.state.apply(tr, pluginState, state, state); expect(nextState.activeThreadId).toBe('comment-5'); - expect(nextState.changedActiveThread).toBe(true); const stateWithPlugin = EditorState.create({ schema, doc: state.doc, plugins: [plugin] }); expect(plugin.props.decorations(stateWithPlugin)).toBeInstanceOf(DecorationSet); }); diff --git a/packages/super-editor/src/extensions/types/comment-commands.ts b/packages/super-editor/src/extensions/types/comment-commands.ts index 03a231515f..578e68afca 100644 --- a/packages/super-editor/src/extensions/types/comment-commands.ts +++ b/packages/super-editor/src/extensions/types/comment-commands.ts @@ -204,11 +204,12 @@ export interface CommentCommands { * Set cursor position to a comment by ID * @param id - The comment ID to navigate to * @param options - Optional navigation settings + * @param options.activeCommentId - Explicitly activate this thread in the same transaction * @param options.preferredActiveThreadId - Preserve this thread as active when overlapping marks exist * @example * editor.commands.setCursorById('comment-123') */ - setCursorById: (id: string, options?: { preferredActiveThreadId?: string }) => boolean; + setCursorById: (id: string, options?: { activeCommentId?: string; preferredActiveThreadId?: string }) => boolean; /** * Add a reply to an existing comment or tracked change diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js index 48e12ad28f..a221fece5f 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js @@ -174,7 +174,8 @@ const mountDialog = async ({ baseCommentOverrides = {}, extraComments = [], prop ], activeEditor: { commands: { - setCursorById: vi.fn(), + setCursorById: vi.fn().mockReturnValue(true), + setActiveComment: vi.fn(), rejectTrackedChangeById: vi.fn(), acceptTrackedChangeById: vi.fn(), setCommentInternal: vi.fn(), @@ -226,9 +227,8 @@ describe('CommentDialog.vue', () => { const { wrapper, baseComment, superdocStub } = await mountDialog(); await nextTick(); - expect(baseComment.setActive).toHaveBeenCalledWith(superdocStub); expect(superdocStub.activeEditor.commands.setCursorById).toHaveBeenCalledWith(baseComment.commentId, { - preferredActiveThreadId: baseComment.commentId, + activeCommentId: baseComment.commentId, }); expect(commentsStoreStub.activeComment.value).toBe(baseComment.commentId); @@ -494,7 +494,8 @@ describe('CommentDialog.vue', () => { const headers = wrapper.findAllComponents(CommentHeaderStub); headers[1].vm.$emit('overflow-select', 'edit'); expect(commentsStoreStub.editingCommentId.value).toBe(childComment.commentId); - expect(commentsStoreStub.setActiveComment).toHaveBeenCalledWith(superdocStub, childComment.commentId); + // Edit activates the root thread (props.comment), not the individual child being edited + expect(commentsStoreStub.setActiveComment).toHaveBeenCalledWith(superdocStub, baseComment.commentId); commentsStoreStub.currentCommentText.value = '

Updated

'; await nextTick(); diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue index 28803c189e..1c8f4e11d3 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue @@ -89,15 +89,6 @@ const isPendingNewComment = computed(() => { return pendingComment.value && pendingComment.value.commentId === props.comment.commentId; }); -const showButtons = computed(() => { - return ( - !getConfig.readOnly && - isActiveComment.value && - !props.comment.resolvedTime && - editingCommentId.value !== props.comment.commentId - ); -}); - const showSeparator = computed(() => (index) => { const visible = visibleComments.value; if (showInputSection.value && index === visible.length - 1) return true; @@ -105,12 +96,7 @@ const showSeparator = computed(() => (index) => { }); const showInputSection = computed(() => { - return ( - !getConfig.readOnly && - isActiveComment.value && - !props.comment.resolvedTime && - editingCommentId.value !== props.comment.commentId - ); + return !getConfig.readOnly && isActiveComment.value && !props.comment.resolvedTime && !isEditingAnyComment.value; }); // Reply pill → expanded editor toggle @@ -276,6 +262,11 @@ const isInternalDropdownDisabled = computed(() => { const isEditingThisComment = computed(() => (comment) => editingCommentId.value === comment.commentId); +const isEditingAnyComment = computed(() => { + if (!editingCommentId.value) return false; + return comments.value.some((c) => c.commentId === editingCommentId.value); +}); + const shouldShowInternalExternal = computed(() => { if (!proxy.$superdoc.config.isInternal) return false; return !suppressInternalExternal.value && !props.comment.trackedChange; @@ -295,25 +286,27 @@ const setFocus = () => { clearInstantSidebarAlignment(); } - // Only set as active if not resolved (resolved comments can't be edited) + // Update Vue store immediately for responsive UI if (!props.comment.resolvedTime) { activeComment.value = props.comment.commentId; - props.comment.setActive(proxy.$superdoc); } - // Always allow scrolling to the comment location, even for resolved comments + // Move cursor to the comment location and set active comment in a single PM + // transaction. This prevents a race where position-based comment detection in the + // plugin clears the activeThreadId before the setActiveComment meta is processed. if (editor) { - // For resolved comments, use commentId since prepareCommentsForImport rewrites - // commentRangeStart/End nodes' w:id to the internal commentId (not importedId) const cursorId = props.comment.resolvedTime ? props.comment.commentId : props.comment.importedId || props.comment.commentId; if (props.comment.resolvedTime) { editor.commands?.setCursorById(cursorId); } else { - editor.commands?.setCursorById(cursorId, { preferredActiveThreadId: cursorId }); + const activeCommentId = props.comment.commentId; + const didScroll = editor.commands?.setCursorById(cursorId, { activeCommentId }); + if (!didScroll) { + editor.commands?.setActiveComment({ commentId: activeCommentId }); + } } - const presentation = props.comment.fileId ? PresentationEditor.getInstance(props.comment.fileId) : null; if (presentation && Number.isFinite(targetClientY)) { const fallbackThreadId = props.comment.commentId; @@ -377,6 +370,8 @@ const handleAddComment = () => { const comment = commentsStore.getPendingComment(options); addComment({ superdoc: proxy.$superdoc, comment }); + isReplying.value = false; + nextTick(() => emit('resize')); }; const handleReject = () => { @@ -439,7 +434,7 @@ const handleOverflowSelect = (value, comment) => { switch (value) { case 'edit': currentCommentText.value = comment?.commentText?.value ?? comment?.commentText ?? ''; - activeComment.value = comment.commentId; + activeComment.value = props.comment.commentId; editingCommentId.value = comment.commentId; commentsStore.setActiveComment(proxy.$superdoc, activeComment.value); nextTick(() => { @@ -469,7 +464,7 @@ const handleInternalExternalSelect = (value) => { const getSidebarCommentStyle = computed(() => { const style = {}; - if (isActiveComment.value || isPendingNewComment.value) { + if (isActiveComment.value || isPendingNewComment.value || isEditingAnyComment.value) { style.zIndex = 50; } @@ -483,6 +478,7 @@ const getProcessedDate = (timestamp) => { const handleCancel = (comment) => { editingCommentId.value = null; + isReplying.value = false; cancelComment(proxy.$superdoc); }; @@ -667,17 +663,26 @@ watch(editingCommentId, (commentId) => { editorCommentPositions[comment.importedId !== undefined ? comment.importedId : comment.commentId]?.bounds }} -
- -
- - +
+
+ +
+
+ +
{ max-width: var(--sd-comment-max-width, 300px); min-width: var(--sd-comment-min-width, 200px); width: 100%; + overflow-wrap: break-word; + word-break: break-word; } .comments-dialog:not(.is-active) { cursor: pointer; @@ -1011,21 +1018,7 @@ watch(editingCommentId, (commentId) => { margin-bottom: 10px; } -.comment-footer { - margin: 5px 0 5px; - display: flex; - justify-content: flex-end; - width: 100%; -} -.comment-footer .sd-button { - font-size: 12px; - margin-left: 5px; -} - -.comment-editing { - padding-bottom: 10px; -} -.comment-editing button { - margin-left: 5px; +.internal-dropdown { + display: inline-block; } diff --git a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue index 5099343d97..fa465fa963 100644 --- a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue +++ b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue @@ -74,7 +74,8 @@ const { clearInstantSidebarAlignment, } = commentsStore; -const { getFloatingComments, activeComment, editorCommentPositions, pendingComment } = storeToRefs(commentsStore); +const { getFloatingComments, activeComment, editorCommentPositions, pendingComment, editingCommentId } = + storeToRefs(commentsStore); const { activeZoom } = storeToRefs(superdocStore); const floatingCommentsContainer = ref(null); @@ -371,6 +372,31 @@ watch(activeCommentKey, (newKey, oldKey) => { }); }); +// Re-measure when editing state changes. Entering/exiting edit mode changes +// the dialog height (CommentInput + action buttons vs static text). +// We remeasure all visible dialogs because the editing comment's parent dialog +// might not be the activeComment (e.g., dropdown interaction deactivated it). +watch(editingCommentId, () => { + // Cancel stale timers from previous edit state change + remeasureTimers.forEach(clearTimeout); + remeasureTimers = []; + + const remeasure = () => { + for (const pos of allPositions.value) { + const el = placeholderRefs.value[pos.id]; + if (!el) continue; + const dialog = el.querySelector('.comments-dialog'); + if (!dialog) continue; + storeHeight(pos.id, dialog.getBoundingClientRect().height); + } + }; + + nextTick(() => { + remeasureTimers.push(setTimeout(remeasure, 50)); + remeasureTimers.push(setTimeout(remeasure, 350)); + }); +}); + // Align the active comment bubble with the same on-screen Y position as its // document anchor by translating the inner sidebar layer. watch(activeComment, () => { diff --git a/packages/superdoc/src/stores/comments-store.js b/packages/superdoc/src/stores/comments-store.js index 8718349a55..536c94e634 100644 --- a/packages/superdoc/src/stores/comments-store.js +++ b/packages/superdoc/src/stores/comments-store.js @@ -785,12 +785,19 @@ export const useCommentsStore = defineStore('comments', () => { * @returns {void} */ const removePendingComment = (superdoc) => { + const hadPending = !!pendingComment.value; currentCommentText.value = ''; pendingComment.value = null; - activeComment.value = null; superdocStore.selectionPosition = null; - superdoc.activeEditor?.commands?.removeComment({ commentId: 'pending' }); + // Only clear active comment when removing an actual pending comment. + // Replies and edits also call this to reset currentCommentText, but + // clearing activeComment would deactivate the thread (SD-2035). + if (hadPending) { + activeComment.value = null; + } + + superdoc?.activeEditor?.commands?.removeComment({ commentId: 'pending' }); }; /** diff --git a/packages/superdoc/src/stores/comments-store.test.js b/packages/superdoc/src/stores/comments-store.test.js index 9483f16bdf..8482e12d9d 100644 --- a/packages/superdoc/src/stores/comments-store.test.js +++ b/packages/superdoc/src/stores/comments-store.test.js @@ -189,6 +189,47 @@ describe('comments-store', () => { expect(store.activeComment).toBeNull(); }); + it('keeps the current active thread when removePendingComment is used for edit cleanup', () => { + const removeCommentSpy = vi.fn(); + const superdoc = { + activeEditor: { + commands: { + removeComment: removeCommentSpy, + }, + }, + }; + + store.activeComment = 'comment-2'; + store.pendingComment = null; + store.currentCommentText = '

Draft

'; + + store.removePendingComment(superdoc); + + expect(store.activeComment).toBe('comment-2'); + expect(store.currentCommentText).toBe(''); + expect(removeCommentSpy).toHaveBeenCalledWith({ commentId: 'pending' }); + }); + + it('clears the active thread when an actual pending comment is removed', () => { + const removeCommentSpy = vi.fn(); + const superdoc = { + activeEditor: { + commands: { + removeComment: removeCommentSpy, + }, + }, + }; + + store.activeComment = 'pending-thread'; + store.pendingComment = { commentId: 'pending' }; + + store.removePendingComment(superdoc); + + expect(store.activeComment).toBeNull(); + expect(store.pendingComment).toBeNull(); + expect(removeCommentSpy).toHaveBeenCalledWith({ commentId: 'pending' }); + }); + it('still syncs editor active comment when store was pre-updated by caller', () => { const setActiveCommentSpy = vi.fn(); const superdoc = {