Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
41 changes: 28 additions & 13 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,6 +20,30 @@ export async function rightClickAtDocPos(page: Page, pos: number): Promise<void>
throw new Error(`Could not resolve coordinates for document position ${pos}`);
}

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);
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);
}

/**
* 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> {
const coords = await getDocPosCoords(page, 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' });
Expand Down
34 changes: 34 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,34 @@
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');
});
Loading