Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions packages/super-editor/src/extensions/comment/comments-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -443,7 +453,6 @@ export const CommentsPlugin = Extension.create({
decorations: DecorationSet.empty,
allCommentPositions: {},
allCommentIds: [],
changedActiveThread: false,
trackedChanges: {},
};
},
Expand Down Expand Up @@ -473,7 +482,6 @@ export const CommentsPlugin = Extension.create({
return {
...pluginState,
activeThreadId: newActiveThreadId,
changedActiveThread: true,
};
}

Expand Down Expand Up @@ -525,7 +533,7 @@ export const CommentsPlugin = Extension.create({
}
}

return pluginState;
return { ...pluginState };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover from removing changedActiveThread — nothing checks if this object changed by reference, so the copy isn't needed.

Suggested change
return { ...pluginState };
return pluginState;

},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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 = '<p>Updated</p>';
await nextTick();
Expand Down
95 changes: 44 additions & 51 deletions packages/superdoc/src/components/CommentsLayer/CommentDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -89,28 +89,14 @@ 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;
return visible.length > 1 && index !== visible.length - 1;
});

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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -377,6 +370,8 @@ const handleAddComment = () => {

const comment = commentsStore.getPendingComment(options);
addComment({ superdoc: proxy.$superdoc, comment });
isReplying.value = false;
nextTick(() => emit('resize'));
};

const handleReject = () => {
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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;
}

Expand All @@ -483,6 +478,7 @@ const getProcessedDate = (timestamp) => {

const handleCancel = (comment) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment isn't used in the function body anymore. safe to drop?

Suggested change
const handleCancel = (comment) => {
const handleCancel = () => {

editingCommentId.value = null;
isReplying.value = false;
cancelComment(proxy.$superdoc);
};

Expand Down Expand Up @@ -667,17 +663,26 @@ watch(editingCommentId, (commentId) => {
editorCommentPositions[comment.importedId !== undefined ? comment.importedId : comment.commentId]?.bounds
}}
</div>
<div v-else class="comment-editing">
<CommentInput
:ref="setEditCommentInputRef(comment.commentId)"
:users="usersFiltered"
:config="getConfig"
:include-header="false"
:comment="comment"
/>
<div class="comment-footer">
<button class="sd-button" @click.stop.prevent="handleCancel(comment)">Cancel</button>
<button class="sd-button primary" @click.stop.prevent="handleCommentUpdate(comment)">Update</button>
<div v-else class="reply-expanded">
<div class="reply-input-wrapper">
<CommentInput
:ref="setEditCommentInputRef(comment.commentId)"
:users="usersFiltered"
:config="getConfig"
:include-header="false"
:comment="comment"
/>
</div>
<div class="reply-actions">
<button class="sd-button reply-btn-cancel" @click.stop.prevent="handleCancel(comment)">Cancel</button>
<button
class="sd-button primary reply-btn-primary"
@click.stop.prevent="handleCommentUpdate(comment)"
:disabled="!hasTextContent"
:class="{ 'is-disabled': !hasTextContent }"
>
Update
</button>
</div>
</div>
<div
Expand Down Expand Up @@ -762,6 +767,8 @@ watch(editingCommentId, (commentId) => {
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;
Expand Down Expand Up @@ -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;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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, () => {
Expand Down
11 changes: 9 additions & 2 deletions packages/superdoc/src/stores/comments-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Comment thread
tupizz marked this conversation as resolved.

superdoc?.activeEditor?.commands?.removeComment({ commentId: 'pending' });
};

/**
Expand Down
Loading
Loading