From 026328fdaf3d8761943faec554407b8c58bc6d0a Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Tue, 3 Mar 2026 18:37:55 -0300 Subject: [PATCH 1/2] fix(comments): resolve double-click activation and edit mode issues (SD-2035) Combine cursor move and active comment activation into a single PM transaction to prevent position-based detection from clearing the active thread. Skip view.focus() for sidebar-initiated activations to avoid DOM selection sync overrides. Add changedActiveThread suppression flag in plugin apply to handle residual focus transactions. Also fix reply input staying open after send, edit mode UI using consistent reply-expanded styles, and removePendingComment only clearing activeComment when an actual pending comment existed. --- .../src/extensions/comment/comments-plugin.js | 27 ++++-- .../CommentsLayer/CommentDialog.test.js | 9 +- .../CommentsLayer/CommentDialog.vue | 87 +++++++++---------- .../CommentsLayer/FloatingComments.vue | 28 +++++- .../superdoc/src/stores/comments-store.js | 9 +- 5 files changed, 101 insertions(+), 59 deletions(-) diff --git a/packages/super-editor/src/extensions/comment/comments-plugin.js b/packages/super-editor/src/extensions/comment/comments-plugin.js index 00ae95ac00..2292c31f7b 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.js @@ -404,12 +404,23 @@ export const CommentsPlugin = Extension.create({ return true; }, setCursorById: - (id) => + (id, options) => ({ state, editor }) => { const { from } = findRangeById(state.doc, id) || {}; if (from != null) { - state.tr.setSelection(TextSelection.create(state.doc, from)); - if (editor.view && typeof editor.view.focus === 'function') { + const tr = state.tr; + tr.setSelection(TextSelection.create(state.doc, from)); + if (options?.activeCommentId) { + tr.setMeta(CommentsPluginKey, { + type: 'setActiveComment', + activeThreadId: options.activeCommentId, + forceUpdate: true, + }); + } + // 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; @@ -492,9 +503,13 @@ export const CommentsPlugin = Extension.create({ ); } - // Check for changes in the actively selected comment + // Check for changes in the actively selected comment. + // Skip position-based detection if the previous transaction explicitly set the + // active comment (changedActiveThread flag). This prevents focus-induced DOM + // selection sync transactions from overriding a sidebar-initiated activation. const trChangedActiveComment = meta?.type === 'setActiveComment'; - if ((!tr.docChanged && tr.selectionSet) || trChangedActiveComment) { + const suppressPositionDetection = pluginState.changedActiveThread && !trChangedActiveComment; + if ((!tr.docChanged && tr.selectionSet && !suppressPositionDetection) || trChangedActiveComment) { const { selection } = tr; let currentActiveThread = getActiveCommentId(newEditorState.doc, selection); if (trChangedActiveComment) currentActiveThread = meta.activeThreadId; @@ -513,7 +528,7 @@ export const CommentsPlugin = Extension.create({ } } - return pluginState; + return { ...pluginState, changedActiveThread: false }; }, }, diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js index 43c28a499e..7fc639e9ef 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js @@ -227,8 +227,10 @@ describe('CommentDialog.vue', () => { const { wrapper, baseComment, superdocStub } = await mountDialog(); await nextTick(); - expect(baseComment.setActive).toHaveBeenCalledWith(superdocStub); - expect(superdocStub.activeEditor.commands.setCursorById).toHaveBeenCalledWith(baseComment.commentId); + // setFocus combines cursor move and active comment into a single PM transaction + expect(superdocStub.activeEditor.commands.setCursorById).toHaveBeenCalledWith(baseComment.commentId, { + activeCommentId: baseComment.commentId, + }); expect(commentsStoreStub.activeComment.value).toBe(baseComment.commentId); // Click the reply pill to expand the editor @@ -476,7 +478,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 fa912cabe6..82c05360ab 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue @@ -82,12 +82,7 @@ const isPendingNewComment = computed(() => { }); const showButtons = computed(() => { - return ( - !getConfig.readOnly && - isActiveComment.value && - !props.comment.resolvedTime && - editingCommentId.value !== props.comment.commentId - ); + return !getConfig.readOnly && isActiveComment.value && !props.comment.resolvedTime && !isEditingAnyComment.value; }); const showSeparator = computed(() => (index) => { @@ -97,12 +92,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 @@ -268,6 +258,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; @@ -280,20 +275,20 @@ const hasTextContent = computed(() => { const setFocus = () => { const editor = proxy.$superdoc.activeEditor; - // 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; - editor.commands?.setCursorById(cursorId); + const activeCommentId = !props.comment.resolvedTime ? props.comment.commentId : null; + editor.commands?.setCursorById(cursorId, { activeCommentId }); } }; @@ -349,6 +344,8 @@ const handleAddComment = () => { const comment = commentsStore.getPendingComment(options); addComment({ superdoc: proxy.$superdoc, comment }); + isReplying.value = false; + nextTick(() => emit('resize')); }; const handleReject = () => { @@ -411,7 +408,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(() => { @@ -441,7 +438,7 @@ const handleInternalExternalSelect = (value) => { const getSidebarCommentStyle = computed(() => { const style = {}; - if (isActiveComment.value || isPendingNewComment.value) { + if (isActiveComment.value || isPendingNewComment.value || isEditingAnyComment.value) { style.zIndex = 50; } @@ -639,17 +636,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; @@ -983,24 +991,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; -} - .internal-dropdown { display: inline-block; } -.comment-editing { - padding-bottom: 10px; -} -.comment-editing button { - margin-left: 5px; -} diff --git a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue index 990ac5879a..1a322a987f 100644 --- a/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue +++ b/packages/superdoc/src/components/CommentsLayer/FloatingComments.vue @@ -67,7 +67,8 @@ const props = defineProps({ const superdocStore = useSuperdocStore(); const commentsStore = useCommentsStore(); -const { getFloatingComments, activeComment, editorCommentPositions, pendingComment } = storeToRefs(commentsStore); +const { getFloatingComments, activeComment, editorCommentPositions, pendingComment, editingCommentId } = + storeToRefs(commentsStore); const { activeZoom } = storeToRefs(superdocStore); const floatingCommentsContainer = ref(null); @@ -311,6 +312,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)); + }); +}); + // Scroll to the active comment ONLY when its anchor is off-screen. // getBoundingClientRect() is viewport-relative (accounts for scroll + zoom). watch(activeComment, () => { diff --git a/packages/superdoc/src/stores/comments-store.js b/packages/superdoc/src/stores/comments-store.js index 577a613494..55b7b1ae31 100644 --- a/packages/superdoc/src/stores/comments-store.js +++ b/packages/superdoc/src/stores/comments-store.js @@ -594,11 +594,18 @@ 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; + // 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' }); }; From 9fb331e60ae42e7d16ffed375cd12d84585a3a7a Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Thu, 5 Mar 2026 08:51:38 -0300 Subject: [PATCH 2/2] fix(comments): address review feedback for SD-2035 - Remove changedActiveThread flag from plugin state. The flag persisted through the decoration early-return path and was never consumed since view.focus() is now skipped for sidebar activations. This caused the next real editor click to be silently ignored. The view.focus() skip is the primary fix; the flag was a broken safety net. - Reset isReplying in handleCancel so the reply input closes when the user cancels a reply. Previously relied on removePendingComment clearing activeComment, which no longer happens for non-pending flows. - Add fallback setActiveComment call when setCursorById fails to find the comment position, preventing Vue store and editor state desync. - Remove dead showButtons computed (identical to showInputSection, never referenced in template). - Update test assertions to match removed changedActiveThread field and add setActiveComment mock to CommentDialog test stub. --- .../src/extensions/comment/comments-plugin.js | 12 +++--------- .../src/extensions/comment/comments-plugin.test.js | 1 - .../src/extensions/comment/comments.test.js | 1 - .../components/CommentsLayer/CommentDialog.test.js | 3 ++- .../src/components/CommentsLayer/CommentDialog.vue | 10 +++++----- 5 files changed, 10 insertions(+), 17 deletions(-) diff --git a/packages/super-editor/src/extensions/comment/comments-plugin.js b/packages/super-editor/src/extensions/comment/comments-plugin.js index 2292c31f7b..ed634b3931 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.js @@ -449,7 +449,6 @@ export const CommentsPlugin = Extension.create({ decorations: DecorationSet.empty, allCommentPositions: {}, allCommentIds: [], - changedActiveThread: false, trackedChanges: {}, }; }, @@ -479,7 +478,6 @@ export const CommentsPlugin = Extension.create({ return { ...pluginState, activeThreadId: newActiveThreadId, - changedActiveThread: true, }; } @@ -503,13 +501,9 @@ export const CommentsPlugin = Extension.create({ ); } - // Check for changes in the actively selected comment. - // Skip position-based detection if the previous transaction explicitly set the - // active comment (changedActiveThread flag). This prevents focus-induced DOM - // selection sync transactions from overriding a sidebar-initiated activation. + // Check for changes in the actively selected comment const trChangedActiveComment = meta?.type === 'setActiveComment'; - const suppressPositionDetection = pluginState.changedActiveThread && !trChangedActiveComment; - if ((!tr.docChanged && tr.selectionSet && !suppressPositionDetection) || trChangedActiveComment) { + if ((!tr.docChanged && tr.selectionSet) || trChangedActiveComment) { const { selection } = tr; let currentActiveThread = getActiveCommentId(newEditorState.doc, selection); if (trChangedActiveComment) currentActiveThread = meta.activeThreadId; @@ -528,7 +522,7 @@ export const CommentsPlugin = Extension.create({ } } - return { ...pluginState, changedActiveThread: false }; + 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 f4d17f5698..f72550b35e 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.test.js @@ -609,7 +609,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 e75c482a18..8d2abec182 100644 --- a/packages/super-editor/src/extensions/comment/comments.test.js +++ b/packages/super-editor/src/extensions/comment/comments.test.js @@ -878,7 +878,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/superdoc/src/components/CommentsLayer/CommentDialog.test.js b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js index 7fc639e9ef..0d2afda2aa 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js @@ -175,7 +175,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(), diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue index 82c05360ab..9b7ef43b83 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue @@ -81,10 +81,6 @@ const isPendingNewComment = computed(() => { return pendingComment.value && pendingComment.value.commentId === props.comment.commentId; }); -const showButtons = computed(() => { - return !getConfig.readOnly && isActiveComment.value && !props.comment.resolvedTime && !isEditingAnyComment.value; -}); - const showSeparator = computed(() => (index) => { const visible = visibleComments.value; if (showInputSection.value && index === visible.length - 1) return true; @@ -288,7 +284,10 @@ const setFocus = () => { ? props.comment.commentId : props.comment.importedId || props.comment.commentId; const activeCommentId = !props.comment.resolvedTime ? props.comment.commentId : null; - editor.commands?.setCursorById(cursorId, { activeCommentId }); + const didScroll = editor.commands?.setCursorById(cursorId, { activeCommentId }); + if (!didScroll && activeCommentId) { + editor.commands?.setActiveComment({ commentId: activeCommentId }); + } } }; @@ -452,6 +451,7 @@ const getProcessedDate = (timestamp) => { const handleCancel = (comment) => { editingCommentId.value = null; + isReplying.value = false; cancelComment(proxy.$superdoc); };