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
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ function getCommentHighlightThreadIds(target: EventTarget | null): string[] {
.filter(Boolean);
}

function isDirectSingleCommentHighlightHit(target: EventTarget | null): boolean {
return getCommentHighlightThreadIds(target).length === 1;
}

function resolveTrackChangeThreadId(target: EventTarget | null): string | null {
if (!(target instanceof Element)) {
return null;
Expand Down Expand Up @@ -216,6 +220,14 @@ function shouldIgnoreRepeatClickOnActiveComment(
return false;
}

// Direct clicks on commented text should place a caret at the clicked
// position and let the comments plugin infer the active thread from the
// resulting selection. Only preserve the pointerdown short-circuit for
// nearby non-text surfaces, such as split-run gaps.
if (isDirectSingleCommentHighlightHit(target)) {
return false;
}

return resolveCommentThreadIdNearPointer(target, clientX, clientY) === activeThreadId;
}

Expand Down Expand Up @@ -2230,6 +2242,10 @@ export class EditorInputManager {
}

#handleSingleCommentHighlightClick(event: PointerEvent, target: HTMLElement | null, editor: Editor): boolean {
if (isDirectSingleCommentHighlightHit(target)) {
return false;
}

const clickedThreadId = resolveCommentThreadIdNearPointer(target, event.clientX, event.clientY);
if (!clickedThreadId) {
return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest';

import { comments_module_events } from '@superdoc/common';
import { clickToPosition } from '@superdoc/layout-bridge';
import { resolvePointerPositionHit } from '../input/PositionHitResolver.js';
import { TextSelection } from 'prosemirror-state';
Expand Down Expand Up @@ -203,28 +202,26 @@ describe('EditorInputManager - single-thread comment highlight clicks', () => {
return elementsFromPoint;
}

it('treats a click on the already-active single-thread highlight as a no-op', () => {
it('lets direct clicks on the active single-thread highlight fall through to generic caret placement', () => {
const highlight = document.createElement('span');
highlight.className = 'superdoc-comment-highlight';
highlight.setAttribute('data-comment-ids', 'comment-1');
viewportHost.appendChild(highlight);

dispatchPointerDown(highlight);

expect(mockEditor.emit).toHaveBeenCalledWith('commentsUpdate', {
type: comments_module_events.SELECTED,
activeCommentId: 'comment-1',
});
expect(resolvePointerPositionHit).not.toHaveBeenCalled();
expect(TextSelection.create as unknown as Mock).not.toHaveBeenCalled();
expect(mockEditor.state.tr.setSelection).not.toHaveBeenCalled();
expect(mockEditor.view.dispatch).not.toHaveBeenCalled();
expect(mockEditor.view.focus).not.toHaveBeenCalled();
expect(editorDom.focus).not.toHaveBeenCalled();
expect(viewportHost.setPointerCapture).not.toHaveBeenCalled();
expect(mockEditor.emit).not.toHaveBeenCalledWith(
'commentsUpdate',
expect.objectContaining({ activeCommentId: 'comment-1' }),
);
expect(mockEditor.commands.setCursorById).not.toHaveBeenCalled();
expect(resolvePointerPositionHit).toHaveBeenCalled();
expect(TextSelection.create as unknown as Mock).toHaveBeenCalled();
expect(mockEditor.state.tr.setSelection).toHaveBeenCalled();
expect(viewportHost.setPointerCapture).toHaveBeenCalled();
});

it('activates an inactive single-thread highlight without falling back to generic selection handling', () => {
it('lets direct clicks on an inactive single-thread highlight fall through to generic caret placement', () => {
mockEditor.state.comments$.activeThreadId = 'comment-2';

const highlight = document.createElement('span');
Expand All @@ -234,16 +231,11 @@ describe('EditorInputManager - single-thread comment highlight clicks', () => {

dispatchPointerDown(highlight);

expect(mockEditor.commands.setCursorById).toHaveBeenCalledWith('comment-1', {
activeCommentId: 'comment-1',
});
expect(resolvePointerPositionHit).not.toHaveBeenCalled();
expect(TextSelection.create as unknown as Mock).not.toHaveBeenCalled();
expect(mockEditor.state.tr.setSelection).not.toHaveBeenCalled();
expect(mockEditor.view.dispatch).not.toHaveBeenCalled();
expect(mockEditor.view.focus).not.toHaveBeenCalled();
expect(editorDom.focus).not.toHaveBeenCalled();
expect(viewportHost.setPointerCapture).not.toHaveBeenCalled();
expect(mockEditor.commands.setCursorById).not.toHaveBeenCalled();
expect(resolvePointerPositionHit).toHaveBeenCalled();
expect(TextSelection.create as unknown as Mock).toHaveBeenCalled();
expect(mockEditor.state.tr.setSelection).toHaveBeenCalled();
expect(viewportHost.setPointerCapture).toHaveBeenCalled();
});

it('activates a tracked-change decoration when it owns the clicked visual surface', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,73 @@ describe('CommentDialog.vue', () => {
expect(wrapper.emitted()).not.toHaveProperty('dialog-exit');
});

it('does not deselect when e.target is wrong but elementFromPoint finds a comment highlight', async () => {
const { wrapper, baseComment } = await mountDialog();
commentsStoreStub.activeComment.value = baseComment.commentId;

// Simulate pointer capture redirecting e.target to the viewport host
const viewportHost = document.createElement('div');
const commentHighlight = document.createElement('span');
commentHighlight.className = 'superdoc-comment-highlight';
document.body.appendChild(commentHighlight);

const originalElementFromPoint = document.elementFromPoint;
document.elementFromPoint = vi.fn(() => commentHighlight);

const handler = wrapper.element.__clickOutside;
handler({ target: viewportHost, clientX: 50, clientY: 50 });

expect(commentsStoreStub.setActiveComment).not.toHaveBeenCalled();
expect(wrapper.emitted()).not.toHaveProperty('dialog-exit');

document.elementFromPoint = originalElementFromPoint;
document.body.removeChild(commentHighlight);
});

it('does not deselect when elementFromPoint finds a tracked-change element', async () => {
const { wrapper, baseComment } = await mountDialog();
commentsStoreStub.activeComment.value = baseComment.commentId;

const viewportHost = document.createElement('div');
const trackedInsert = document.createElement('span');
trackedInsert.className = 'track-insert';
document.body.appendChild(trackedInsert);

const originalElementFromPoint = document.elementFromPoint;
document.elementFromPoint = vi.fn(() => trackedInsert);

const handler = wrapper.element.__clickOutside;
handler({ target: viewportHost, clientX: 50, clientY: 50 });

expect(commentsStoreStub.setActiveComment).not.toHaveBeenCalled();
expect(wrapper.emitted()).not.toHaveProperty('dialog-exit');

document.elementFromPoint = originalElementFromPoint;
document.body.removeChild(trackedInsert);
});

it('deselects when elementFromPoint returns a non-ignored element', async () => {
const { wrapper, baseComment } = await mountDialog();
commentsStoreStub.activeComment.value = baseComment.commentId;

const viewportHost = document.createElement('div');
const plainDiv = document.createElement('div');
plainDiv.className = 'some-normal-content';
document.body.appendChild(plainDiv);

const originalElementFromPoint = document.elementFromPoint;
document.elementFromPoint = vi.fn(() => plainDiv);

const handler = wrapper.element.__clickOutside;
handler({ target: viewportHost, clientX: 50, clientY: 50 });

expect(commentsStoreStub.setActiveComment).toHaveBeenCalledWith(expect.any(Object), null);
expect(wrapper.emitted('dialog-exit')).toHaveLength(1);

document.elementFromPoint = originalElementFromPoint;
document.body.removeChild(plainDiv);
});

it('sorts tracked change parent first, then child comments by creation time', async () => {
// Simulate a tracked change with two comments on it
// The comments were created after the tracked change but should appear below it
Expand Down
32 changes: 18 additions & 14 deletions packages/superdoc/src/components/CommentsLayer/CommentDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -438,20 +438,24 @@ const setFocus = () => {

const handleClickOutside = (e) => {
const targetElement = e.target instanceof Element ? e.target : e.target?.parentElement;
const clickedIgnoredTarget = targetElement?.closest?.(
[
'.comments-dropdown__option-label',
'.superdoc-comment-highlight',
'.sd-editor-comment-highlight',
'.sd-editor-tracked-change-highlight',
'.track-insert',
'.track-insert-dec',
'.track-delete',
'.track-delete-dec',
'.track-format',
'.track-format-dec',
].join(','),
);
// Also check what's under the actual click coordinates. Pointer capture
// (used by the presentation editor) can redirect e.target away from the
// originally clicked element, causing the selector check to miss it.
const elementAtPoint = document.elementFromPoint(e.clientX, e.clientY);
const ignoredSelectors = [
'.comments-dropdown__option-label',
'.superdoc-comment-highlight',
'.sd-editor-comment-highlight',
'.sd-editor-tracked-change-highlight',
'.track-insert',
'.track-insert-dec',
'.track-delete',
'.track-delete-dec',
'.track-format',
'.track-format-dec',
].join(',');
const clickedIgnoredTarget =
targetElement?.closest?.(ignoredSelectors) || elementAtPoint?.closest?.(ignoredSelectors);

if (clickedIgnoredTarget || isCommentHighlighted.value) return;

Expand Down
31 changes: 15 additions & 16 deletions tests/behavior/helpers/editor-interactions.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
import type { Page } from '@playwright/test';

/**
* Right-clicks at the screen location corresponding to a document position in the SuperDoc editor.
*
* This helper queries the editor's coordinates for the given logical document position, calculates a suitable
* (x, y) point within the bounding rectangle, and dispatches a mouse right-click at that spot.
*
* Throws if coordinates cannot be resolved for the given position.
*
* @param {Page} page - The Playwright test page instance.
* @param {number} pos - The logical document position (character offset) at which to right-click.
* @returns {Promise<void>} Resolves when the click has been dispatched.
*/
export async function rightClickAtDocPos(page: Page, pos: number): Promise<void> {
async function getDocPosCoords(
page: Page,
pos: number,
): Promise<{ left: number; right: number; top: number; bottom: number }> {
const coords = await page.evaluate((targetPos) => {
const editor = (window as any).editor;
const rect = editor?.coordsAtPos?.(targetPos);
Expand All @@ -29,7 +20,15 @@ export async function rightClickAtDocPos(page: Page, pos: number): Promise<void>
throw new Error(`Could not resolve coordinates for document position ${pos}`);
}

const x = Math.min(Math.max(coords.left + 1, coords.left), Math.max(coords.right - 1, coords.left + 1));
const y = (coords.top + coords.bottom) / 2;
await page.mouse.click(x, y, { button: 'right' });
return coords;
Comment thread
caio-pizzol marked this conversation as resolved.
}

export async function clickAtDocPos(page: Page, pos: number): Promise<void> {
const coords = await getDocPosCoords(page, pos);
await page.mouse.click(coords.left + 1, (coords.top + coords.bottom) / 2);
}

export async function rightClickAtDocPos(page: Page, pos: number): Promise<void> {
const coords = await getDocPosCoords(page, pos);
await page.mouse.click(coords.left + 1, (coords.top + coords.bottom) / 2, { button: 'right' });
}
102 changes: 102 additions & 0 deletions tests/behavior/tests/comments/sd-2442-commented-text-caret.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { test, expect } from '../../fixtures/superdoc.js';
import { addCommentViaUI } from '../../helpers/comments.js';
import { clickAtDocPos } from '../../helpers/editor-interactions.js';

test.use({ config: { toolbar: 'full', comments: 'on' } });

test('SD-2442: clicking inside commented text places a caret and allows typing', async ({ superdoc }) => {
await superdoc.type('alpha beta gamma');
await superdoc.waitForStable();

await addCommentViaUI(superdoc, {
textToSelect: 'beta gamma',
commentText: 'outer comment',
});

await superdoc.assertCommentHighlightExists({ text: 'beta gamma' });

const betaStart = await superdoc.findTextPos('beta');
const insertionPos = betaStart + 2;

await superdoc.clickOnLine(0, 5);
await superdoc.waitForStable();
await expect((await superdoc.getSelection()).from).not.toBe(insertionPos);

await clickAtDocPos(superdoc.page, insertionPos);
await superdoc.waitForStable();

await superdoc.assertSelection(insertionPos);

await superdoc.page.keyboard.type('X');
await superdoc.waitForStable();

await expect.poll(() => superdoc.getTextContent()).toContain('alpha beXta gamma');
});

test('SD-2442: clicking inside commented text activates the comment bubble', async ({ superdoc }) => {
await superdoc.type('hello world');
await superdoc.waitForStable();

await addCommentViaUI(superdoc, {
textToSelect: 'world',
commentText: 'bubble test',
});

await superdoc.assertCommentHighlightExists({ text: 'world' });

// Click outside the comment to deselect it first
await superdoc.clickOnLine(0, 0);
await superdoc.waitForStable();
await expect(superdoc.page.locator('.comments-dialog.is-active')).toHaveCount(0, { timeout: 3000 });

// Click inside the commented text
const worldPos = await superdoc.findTextPos('world');
await clickAtDocPos(superdoc.page, worldPos + 2);
await superdoc.waitForStable();

// The comment bubble should be active and stay active
await expect(superdoc.page.locator('.comments-dialog.is-active')).toBeVisible({ timeout: 5000 });
await expect(superdoc.page.locator('.comments-dialog.is-active')).toContainText('bubble test');
});

test('SD-2442: double-clicking inside commented text selects a word', async ({ superdoc }) => {
await superdoc.type('select this word');
await superdoc.waitForStable();

await addCommentViaUI(superdoc, {
textToSelect: 'this word',
commentText: 'dblclick test',
});

await superdoc.assertCommentHighlightExists({ text: 'this word' });

// Click outside first
await superdoc.clickOnLine(0, 0);
await superdoc.waitForStable();

// Double-click on "word" inside the comment highlight
const wordPos = await superdoc.findTextPos('word');
const coords = await superdoc.page.evaluate((pos) => {
const editor = (window as any).editor;
const rect = editor?.coordsAtPos?.(pos);
if (!rect) return null;
return { left: Number(rect.left), right: Number(rect.right), top: Number(rect.top), bottom: Number(rect.bottom) };
}, wordPos);

if (coords) {
const x = coords.left + 5;
const y = (coords.top + coords.bottom) / 2;
await superdoc.page.mouse.dblclick(x, y);
await superdoc.waitForStable();

const sel = await superdoc.getSelection();
const selectedText = await superdoc.page.evaluate(
({ from, to }) => {
const editor = (window as any).editor;
return editor.state.doc.textBetween(from, to);
},
{ from: sel.from, to: sel.to },
);
expect(selectedText).toBe('word');
}
});
Loading