Skip to content

Commit 026328f

Browse files
committed
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.
1 parent 07fecd8 commit 026328f

5 files changed

Lines changed: 101 additions & 59 deletions

File tree

packages/super-editor/src/extensions/comment/comments-plugin.js

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -404,12 +404,23 @@ export const CommentsPlugin = Extension.create({
404404
return true;
405405
},
406406
setCursorById:
407-
(id) =>
407+
(id, options) =>
408408
({ state, editor }) => {
409409
const { from } = findRangeById(state.doc, id) || {};
410410
if (from != null) {
411-
state.tr.setSelection(TextSelection.create(state.doc, from));
412-
if (editor.view && typeof editor.view.focus === 'function') {
411+
const tr = state.tr;
412+
tr.setSelection(TextSelection.create(state.doc, from));
413+
if (options?.activeCommentId) {
414+
tr.setMeta(CommentsPluginKey, {
415+
type: 'setActiveComment',
416+
activeThreadId: options.activeCommentId,
417+
forceUpdate: true,
418+
});
419+
}
420+
// Skip view.focus() when activating from the sidebar (activeCommentId set).
421+
// Focusing the hidden PM view can trigger a DOM selection sync transaction
422+
// that overwrites the activeThreadId via position-based detection.
423+
if (!options?.activeCommentId && editor.view && typeof editor.view.focus === 'function') {
413424
editor.view.focus();
414425
}
415426
return true;
@@ -492,9 +503,13 @@ export const CommentsPlugin = Extension.create({
492503
);
493504
}
494505

495-
// Check for changes in the actively selected comment
506+
// Check for changes in the actively selected comment.
507+
// Skip position-based detection if the previous transaction explicitly set the
508+
// active comment (changedActiveThread flag). This prevents focus-induced DOM
509+
// selection sync transactions from overriding a sidebar-initiated activation.
496510
const trChangedActiveComment = meta?.type === 'setActiveComment';
497-
if ((!tr.docChanged && tr.selectionSet) || trChangedActiveComment) {
511+
const suppressPositionDetection = pluginState.changedActiveThread && !trChangedActiveComment;
512+
if ((!tr.docChanged && tr.selectionSet && !suppressPositionDetection) || trChangedActiveComment) {
498513
const { selection } = tr;
499514
let currentActiveThread = getActiveCommentId(newEditorState.doc, selection);
500515
if (trChangedActiveComment) currentActiveThread = meta.activeThreadId;
@@ -513,7 +528,7 @@ export const CommentsPlugin = Extension.create({
513528
}
514529
}
515530

516-
return pluginState;
531+
return { ...pluginState, changedActiveThread: false };
517532
},
518533
},
519534

packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,10 @@ describe('CommentDialog.vue', () => {
227227
const { wrapper, baseComment, superdocStub } = await mountDialog();
228228

229229
await nextTick();
230-
expect(baseComment.setActive).toHaveBeenCalledWith(superdocStub);
231-
expect(superdocStub.activeEditor.commands.setCursorById).toHaveBeenCalledWith(baseComment.commentId);
230+
// setFocus combines cursor move and active comment into a single PM transaction
231+
expect(superdocStub.activeEditor.commands.setCursorById).toHaveBeenCalledWith(baseComment.commentId, {
232+
activeCommentId: baseComment.commentId,
233+
});
232234
expect(commentsStoreStub.activeComment.value).toBe(baseComment.commentId);
233235

234236
// Click the reply pill to expand the editor
@@ -476,7 +478,8 @@ describe('CommentDialog.vue', () => {
476478
const headers = wrapper.findAllComponents(CommentHeaderStub);
477479
headers[1].vm.$emit('overflow-select', 'edit');
478480
expect(commentsStoreStub.editingCommentId.value).toBe(childComment.commentId);
479-
expect(commentsStoreStub.setActiveComment).toHaveBeenCalledWith(superdocStub, childComment.commentId);
481+
// Edit activates the root thread (props.comment), not the individual child being edited
482+
expect(commentsStoreStub.setActiveComment).toHaveBeenCalledWith(superdocStub, baseComment.commentId);
480483

481484
commentsStoreStub.currentCommentText.value = '<p>Updated</p>';
482485
await nextTick();

packages/superdoc/src/components/CommentsLayer/CommentDialog.vue

Lines changed: 39 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,7 @@ const isPendingNewComment = computed(() => {
8282
});
8383
8484
const showButtons = computed(() => {
85-
return (
86-
!getConfig.readOnly &&
87-
isActiveComment.value &&
88-
!props.comment.resolvedTime &&
89-
editingCommentId.value !== props.comment.commentId
90-
);
85+
return !getConfig.readOnly && isActiveComment.value && !props.comment.resolvedTime && !isEditingAnyComment.value;
9186
});
9287
9388
const showSeparator = computed(() => (index) => {
@@ -97,12 +92,7 @@ const showSeparator = computed(() => (index) => {
9792
});
9893
9994
const showInputSection = computed(() => {
100-
return (
101-
!getConfig.readOnly &&
102-
isActiveComment.value &&
103-
!props.comment.resolvedTime &&
104-
editingCommentId.value !== props.comment.commentId
105-
);
95+
return !getConfig.readOnly && isActiveComment.value && !props.comment.resolvedTime && !isEditingAnyComment.value;
10696
});
10797
10898
// Reply pill → expanded editor toggle
@@ -268,6 +258,11 @@ const isInternalDropdownDisabled = computed(() => {
268258
269259
const isEditingThisComment = computed(() => (comment) => editingCommentId.value === comment.commentId);
270260
261+
const isEditingAnyComment = computed(() => {
262+
if (!editingCommentId.value) return false;
263+
return comments.value.some((c) => c.commentId === editingCommentId.value);
264+
});
265+
271266
const shouldShowInternalExternal = computed(() => {
272267
if (!proxy.$superdoc.config.isInternal) return false;
273268
return !suppressInternalExternal.value && !props.comment.trackedChange;
@@ -280,20 +275,20 @@ const hasTextContent = computed(() => {
280275
const setFocus = () => {
281276
const editor = proxy.$superdoc.activeEditor;
282277
283-
// Only set as active if not resolved (resolved comments can't be edited)
278+
// Update Vue store immediately for responsive UI
284279
if (!props.comment.resolvedTime) {
285280
activeComment.value = props.comment.commentId;
286-
props.comment.setActive(proxy.$superdoc);
287281
}
288282
289-
// Always allow scrolling to the comment location, even for resolved comments
283+
// Move cursor to the comment location and set active comment in a single PM
284+
// transaction. This prevents a race where position-based comment detection in the
285+
// plugin clears the activeThreadId before the setActiveComment meta is processed.
290286
if (editor) {
291-
// For resolved comments, use commentId since prepareCommentsForImport rewrites
292-
// commentRangeStart/End nodes' w:id to the internal commentId (not importedId)
293287
const cursorId = props.comment.resolvedTime
294288
? props.comment.commentId
295289
: props.comment.importedId || props.comment.commentId;
296-
editor.commands?.setCursorById(cursorId);
290+
const activeCommentId = !props.comment.resolvedTime ? props.comment.commentId : null;
291+
editor.commands?.setCursorById(cursorId, { activeCommentId });
297292
}
298293
};
299294
@@ -349,6 +344,8 @@ const handleAddComment = () => {
349344
350345
const comment = commentsStore.getPendingComment(options);
351346
addComment({ superdoc: proxy.$superdoc, comment });
347+
isReplying.value = false;
348+
nextTick(() => emit('resize'));
352349
};
353350
354351
const handleReject = () => {
@@ -411,7 +408,7 @@ const handleOverflowSelect = (value, comment) => {
411408
switch (value) {
412409
case 'edit':
413410
currentCommentText.value = comment?.commentText?.value ?? comment?.commentText ?? '';
414-
activeComment.value = comment.commentId;
411+
activeComment.value = props.comment.commentId;
415412
editingCommentId.value = comment.commentId;
416413
commentsStore.setActiveComment(proxy.$superdoc, activeComment.value);
417414
nextTick(() => {
@@ -441,7 +438,7 @@ const handleInternalExternalSelect = (value) => {
441438
const getSidebarCommentStyle = computed(() => {
442439
const style = {};
443440
444-
if (isActiveComment.value || isPendingNewComment.value) {
441+
if (isActiveComment.value || isPendingNewComment.value || isEditingAnyComment.value) {
445442
style.zIndex = 50;
446443
}
447444
@@ -639,17 +636,26 @@ watch(editingCommentId, (commentId) => {
639636
editorCommentPositions[comment.importedId !== undefined ? comment.importedId : comment.commentId]?.bounds
640637
}}
641638
</div>
642-
<div v-else class="comment-editing">
643-
<CommentInput
644-
:ref="setEditCommentInputRef(comment.commentId)"
645-
:users="usersFiltered"
646-
:config="getConfig"
647-
:include-header="false"
648-
:comment="comment"
649-
/>
650-
<div class="comment-footer">
651-
<button class="sd-button" @click.stop.prevent="handleCancel(comment)">Cancel</button>
652-
<button class="sd-button primary" @click.stop.prevent="handleCommentUpdate(comment)">Update</button>
639+
<div v-else class="reply-expanded">
640+
<div class="reply-input-wrapper">
641+
<CommentInput
642+
:ref="setEditCommentInputRef(comment.commentId)"
643+
:users="usersFiltered"
644+
:config="getConfig"
645+
:include-header="false"
646+
:comment="comment"
647+
/>
648+
</div>
649+
<div class="reply-actions">
650+
<button class="sd-button reply-btn-cancel" @click.stop.prevent="handleCancel(comment)">Cancel</button>
651+
<button
652+
class="sd-button primary reply-btn-primary"
653+
@click.stop.prevent="handleCommentUpdate(comment)"
654+
:disabled="!hasTextContent"
655+
:class="{ 'is-disabled': !hasTextContent }"
656+
>
657+
Update
658+
</button>
653659
</div>
654660
</div>
655661
<div
@@ -734,6 +740,8 @@ watch(editingCommentId, (commentId) => {
734740
max-width: var(--sd-comment-max-width, 300px);
735741
min-width: var(--sd-comment-min-width, 200px);
736742
width: 100%;
743+
overflow-wrap: break-word;
744+
word-break: break-word;
737745
}
738746
.comments-dialog:not(.is-active) {
739747
cursor: pointer;
@@ -983,24 +991,7 @@ watch(editingCommentId, (commentId) => {
983991
margin-bottom: 10px;
984992
}
985993
986-
.comment-footer {
987-
margin: 5px 0 5px;
988-
display: flex;
989-
justify-content: flex-end;
990-
width: 100%;
991-
}
992-
.comment-footer .sd-button {
993-
font-size: 12px;
994-
margin-left: 5px;
995-
}
996-
997994
.internal-dropdown {
998995
display: inline-block;
999996
}
1000-
.comment-editing {
1001-
padding-bottom: 10px;
1002-
}
1003-
.comment-editing button {
1004-
margin-left: 5px;
1005-
}
1006997
</style>

packages/superdoc/src/components/CommentsLayer/FloatingComments.vue

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ const props = defineProps({
6767
const superdocStore = useSuperdocStore();
6868
const commentsStore = useCommentsStore();
6969
70-
const { getFloatingComments, activeComment, editorCommentPositions, pendingComment } = storeToRefs(commentsStore);
70+
const { getFloatingComments, activeComment, editorCommentPositions, pendingComment, editingCommentId } =
71+
storeToRefs(commentsStore);
7172
const { activeZoom } = storeToRefs(superdocStore);
7273
7374
const floatingCommentsContainer = ref(null);
@@ -311,6 +312,31 @@ watch(activeCommentKey, (newKey, oldKey) => {
311312
});
312313
});
313314
315+
// Re-measure when editing state changes. Entering/exiting edit mode changes
316+
// the dialog height (CommentInput + action buttons vs static text).
317+
// We remeasure all visible dialogs because the editing comment's parent dialog
318+
// might not be the activeComment (e.g., dropdown interaction deactivated it).
319+
watch(editingCommentId, () => {
320+
// Cancel stale timers from previous edit state change
321+
remeasureTimers.forEach(clearTimeout);
322+
remeasureTimers = [];
323+
324+
const remeasure = () => {
325+
for (const pos of allPositions.value) {
326+
const el = placeholderRefs.value[pos.id];
327+
if (!el) continue;
328+
const dialog = el.querySelector('.comments-dialog');
329+
if (!dialog) continue;
330+
storeHeight(pos.id, dialog.getBoundingClientRect().height);
331+
}
332+
};
333+
334+
nextTick(() => {
335+
remeasureTimers.push(setTimeout(remeasure, 50));
336+
remeasureTimers.push(setTimeout(remeasure, 350));
337+
});
338+
});
339+
314340
// Scroll to the active comment ONLY when its anchor is off-screen.
315341
// getBoundingClientRect() is viewport-relative (accounts for scroll + zoom).
316342
watch(activeComment, () => {

packages/superdoc/src/stores/comments-store.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,11 +594,18 @@ export const useCommentsStore = defineStore('comments', () => {
594594
* @returns {void}
595595
*/
596596
const removePendingComment = (superdoc) => {
597+
const hadPending = !!pendingComment.value;
597598
currentCommentText.value = '';
598599
pendingComment.value = null;
599-
activeComment.value = null;
600600
superdocStore.selectionPosition = null;
601601

602+
// Only clear active comment when removing an actual pending comment.
603+
// Replies and edits also call this to reset currentCommentText, but
604+
// clearing activeComment would deactivate the thread (SD-2035).
605+
if (hadPending) {
606+
activeComment.value = null;
607+
}
608+
602609
superdoc.activeEditor?.commands.removeComment({ commentId: 'pending' });
603610
};
604611

0 commit comments

Comments
 (0)