mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-03-24 16:18:39 +08:00
fix(editor): invalid caret in note-edgeless-block on focus (#14229)
### Problem ●In edgeless mode, when starting to edit, `note-block` exhibits two types of invalid caret behavior: (1)**Title Region Misalignment**: Clicking on the title region incorrectly generates the caret in the first line of the note content, rather than in the title itself. (2)**Vanishing Caret at Line End**: When clicking in the empty space beyond the end of a text section, the caret appears momentarily at the line's end but disappears immediately. ●The following video demonstrates these issues: https://github.com/user-attachments/assets/db9c2c50-709f-4d32-912c-0f01841d2024 ### Solution ●**Title Click Interception**: Added a check to determine if the click coordinates fall in the title region. If so, the caret positioning is now handled by a dedicated logic path. Otherwise, it falls back to the existing note-content logic as before. ●**Range Normalization**: When the generated `range.startContainer` is not a `TextNode`, try to find a most appropriate `TextNode` and update the `range` accordingly. ### After ●The video below shows the behavior after this fix. https://github.com/user-attachments/assets/b2f70b64-1fc6-4049-8379-8bcf3a488a05 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Clicking a page block title no longer creates unwanted paragraphs and reliably focuses the title. * Paragraph creation now occurs only when needed and focus is applied only after successful creation. * Click coordinates are clamped to container bounds to prevent misplaced cursors or focus. * **Improvements** * Caret normalization: clicks place the caret at the last meaningful text position for consistent single-cursor behavior. * **Tests** * Added end-to-end coverage for caret placement and focus transitions. * New ratio-based click/double-click test utilities and a helper for double-clicking note bodies. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -454,6 +454,28 @@ export const EdgelessNoteInteraction =
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let isClickOnTitle = false;
|
||||||
|
const titleRect = view
|
||||||
|
.querySelector('edgeless-page-block-title')
|
||||||
|
?.getBoundingClientRect();
|
||||||
|
|
||||||
|
if (titleRect) {
|
||||||
|
const titleBound = new Bound(
|
||||||
|
titleRect.x,
|
||||||
|
titleRect.y,
|
||||||
|
titleRect.width,
|
||||||
|
titleRect.height
|
||||||
|
);
|
||||||
|
if (titleBound.isPointInBound([e.clientX, e.clientY])) {
|
||||||
|
isClickOnTitle = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (isClickOnTitle) {
|
||||||
|
handleNativeRangeAtPoint(e.clientX, e.clientY);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (model.children.length === 0) {
|
if (model.children.length === 0) {
|
||||||
const blockId = std.store.addBlock(
|
const blockId = std.store.addBlock(
|
||||||
'affine:paragraph',
|
'affine:paragraph',
|
||||||
|
|||||||
108
blocksuite/affine/shared/src/__tests__/utils/range.unit.spec.ts
Normal file
108
blocksuite/affine/shared/src/__tests__/utils/range.unit.spec.ts
Normal file
@@ -0,0 +1,108 @@
|
|||||||
|
import {
|
||||||
|
beforeEach,
|
||||||
|
describe,
|
||||||
|
expect,
|
||||||
|
it,
|
||||||
|
type MockInstance,
|
||||||
|
vi,
|
||||||
|
} from 'vitest';
|
||||||
|
|
||||||
|
import * as PointToRangeUtils from '../../utils/dom/point-to-range';
|
||||||
|
import { handleNativeRangeAtPoint } from '../../utils/dom/point-to-range';
|
||||||
|
|
||||||
|
describe('test handleNativeRangeAtPoint', () => {
|
||||||
|
let caretRangeFromPointSpy: MockInstance<
|
||||||
|
(clientX: number, clientY: number) => Range | null
|
||||||
|
>;
|
||||||
|
let resetNativeSelectionSpy: MockInstance<(range: Range | null) => void>;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
caretRangeFromPointSpy = vi.spyOn(
|
||||||
|
PointToRangeUtils.api,
|
||||||
|
'caretRangeFromPoint'
|
||||||
|
);
|
||||||
|
resetNativeSelectionSpy = vi.spyOn(
|
||||||
|
PointToRangeUtils.api,
|
||||||
|
'resetNativeSelection'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does nothing if caretRangeFromPoint returns null', () => {
|
||||||
|
caretRangeFromPointSpy.mockReturnValue(null);
|
||||||
|
|
||||||
|
handleNativeRangeAtPoint(10, 10);
|
||||||
|
expect(resetNativeSelectionSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps range untouched if startContainer is a Text node', () => {
|
||||||
|
const div = document.createElement('div');
|
||||||
|
div.textContent = 'hello';
|
||||||
|
|
||||||
|
const text = div.firstChild!;
|
||||||
|
|
||||||
|
const range = document.createRange();
|
||||||
|
range.setStart(text, 2);
|
||||||
|
range.collapse(true);
|
||||||
|
|
||||||
|
caretRangeFromPointSpy.mockReturnValue(range);
|
||||||
|
|
||||||
|
handleNativeRangeAtPoint(10, 10);
|
||||||
|
|
||||||
|
expect(range.startContainer).toBe(text);
|
||||||
|
expect(range.startOffset).toBe(2);
|
||||||
|
expect(resetNativeSelectionSpy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('moves caret into direct text child when clicking element', () => {
|
||||||
|
const div = document.createElement('div');
|
||||||
|
div.append('hello');
|
||||||
|
|
||||||
|
const range = document.createRange();
|
||||||
|
range.setStart(div, 1);
|
||||||
|
range.collapse(true);
|
||||||
|
|
||||||
|
caretRangeFromPointSpy.mockReturnValue(range);
|
||||||
|
|
||||||
|
handleNativeRangeAtPoint(10, 10);
|
||||||
|
|
||||||
|
expect(range.startContainer.nodeType).toBe(Node.TEXT_NODE);
|
||||||
|
expect(range.startContainer.textContent).toBe('hello');
|
||||||
|
expect(range.startOffset).toBe(5);
|
||||||
|
expect(resetNativeSelectionSpy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('moves caret to last meaningful text inside nested element', () => {
|
||||||
|
const div = document.createElement('div');
|
||||||
|
div.innerHTML = `<span>a</span><span><em>b</em>c</span>`;
|
||||||
|
|
||||||
|
const range = document.createRange();
|
||||||
|
range.setStart(div, 2);
|
||||||
|
range.collapse(true);
|
||||||
|
|
||||||
|
caretRangeFromPointSpy.mockReturnValue(range);
|
||||||
|
|
||||||
|
handleNativeRangeAtPoint(10, 10);
|
||||||
|
|
||||||
|
expect(range.startContainer.nodeType).toBe(Node.TEXT_NODE);
|
||||||
|
expect(range.startContainer.textContent).toBe('c');
|
||||||
|
expect(range.startOffset).toBe(1);
|
||||||
|
expect(resetNativeSelectionSpy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to searching startContainer when offset element has no text', () => {
|
||||||
|
const div = document.createElement('div');
|
||||||
|
div.innerHTML = `<span></span><span>ok</span>`;
|
||||||
|
|
||||||
|
const range = document.createRange();
|
||||||
|
range.setStart(div, 1);
|
||||||
|
range.collapse(true);
|
||||||
|
|
||||||
|
caretRangeFromPointSpy.mockReturnValue(range);
|
||||||
|
|
||||||
|
handleNativeRangeAtPoint(10, 10);
|
||||||
|
|
||||||
|
expect(range.startContainer.textContent).toBe('ok');
|
||||||
|
expect(range.startOffset).toBe(2);
|
||||||
|
expect(resetNativeSelectionSpy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -88,11 +88,73 @@ export function getCurrentNativeRange(selection = window.getSelection()) {
|
|||||||
return selection.getRangeAt(0);
|
return selection.getRangeAt(0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// functions need to be mocked in unit-test
|
||||||
|
export const api = {
|
||||||
|
caretRangeFromPoint,
|
||||||
|
resetNativeSelection,
|
||||||
|
};
|
||||||
|
|
||||||
export function handleNativeRangeAtPoint(x: number, y: number) {
|
export function handleNativeRangeAtPoint(x: number, y: number) {
|
||||||
const range = caretRangeFromPoint(x, y);
|
const range = api.caretRangeFromPoint(x, y);
|
||||||
|
if (range) {
|
||||||
|
normalizeCaretRange(range);
|
||||||
|
}
|
||||||
|
|
||||||
const startContainer = range?.startContainer;
|
const startContainer = range?.startContainer;
|
||||||
// click on rich text
|
// click on rich text
|
||||||
if (startContainer instanceof Node) {
|
if (startContainer instanceof Node) {
|
||||||
resetNativeSelection(range);
|
api.resetNativeSelection(range);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function lastMeaningfulTextNode(node: Node) {
|
||||||
|
const walker = document.createTreeWalker(node, NodeFilter.SHOW_TEXT, {
|
||||||
|
acceptNode(node) {
|
||||||
|
return node.textContent && node.textContent?.trim().length > 0
|
||||||
|
? NodeFilter.FILTER_ACCEPT
|
||||||
|
: NodeFilter.FILTER_REJECT;
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
let last = null;
|
||||||
|
while (walker.nextNode()) {
|
||||||
|
last = walker.currentNode;
|
||||||
|
}
|
||||||
|
return last;
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeCaretRange(range: Range) {
|
||||||
|
let { startContainer, startOffset } = range;
|
||||||
|
if (startContainer.nodeType === Node.TEXT_NODE) return;
|
||||||
|
|
||||||
|
// Try to find text in the element at `startOffset`
|
||||||
|
const offsetEl =
|
||||||
|
startOffset > 0
|
||||||
|
? startContainer.childNodes[startOffset - 1]
|
||||||
|
: startContainer.childNodes[0];
|
||||||
|
if (offsetEl) {
|
||||||
|
if (offsetEl.nodeType === Node.TEXT_NODE) {
|
||||||
|
range.setStart(
|
||||||
|
offsetEl,
|
||||||
|
startOffset > 0 ? (offsetEl.textContent?.length ?? 0) : 0
|
||||||
|
);
|
||||||
|
range.collapse(true);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const text = lastMeaningfulTextNode(offsetEl);
|
||||||
|
if (text) {
|
||||||
|
range.setStart(text, text.textContent?.length ?? 0);
|
||||||
|
range.collapse(true);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fallback, try to find text in startContainer
|
||||||
|
const text = lastMeaningfulTextNode(startContainer);
|
||||||
|
if (text) {
|
||||||
|
range.setStart(text, text.textContent?.length ?? 0);
|
||||||
|
range.collapse(true);
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import { test } from '@affine-test/kit/playwright';
|
import { test } from '@affine-test/kit/playwright';
|
||||||
import {
|
import {
|
||||||
clickEdgelessModeButton,
|
clickEdgelessModeButton,
|
||||||
|
dblclickNoteBody,
|
||||||
locateEditorContainer,
|
locateEditorContainer,
|
||||||
locateToolbar,
|
locateToolbar,
|
||||||
} from '@affine-test/kit/utils/editor';
|
} from '@affine-test/kit/utils/editor';
|
||||||
@@ -43,7 +44,7 @@ test('should close embed editing modal when editor switching to page mode by sho
|
|||||||
|
|
||||||
test('embed card should not overflow the edgeless note', async ({ page }) => {
|
test('embed card should not overflow the edgeless note', async ({ page }) => {
|
||||||
const note = page.locator('affine-edgeless-note');
|
const note = page.locator('affine-edgeless-note');
|
||||||
await note.dblclick();
|
await dblclickNoteBody(page);
|
||||||
await type(page, '/github');
|
await type(page, '/github');
|
||||||
await pressEnter(page);
|
await pressEnter(page);
|
||||||
await page
|
await page
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ import {
|
|||||||
type,
|
type,
|
||||||
waitForEditorLoad,
|
waitForEditorLoad,
|
||||||
} from '@affine-test/kit/utils/page-logic';
|
} from '@affine-test/kit/utils/page-logic';
|
||||||
|
import { clickLocatorByRatio } from '@affine-test/kit/utils/utils';
|
||||||
import type { EdgelessRootBlockComponent } from '@blocksuite/affine/blocks/root';
|
import type { EdgelessRootBlockComponent } from '@blocksuite/affine/blocks/root';
|
||||||
import type { IVec } from '@blocksuite/affine/global/gfx';
|
import type { IVec } from '@blocksuite/affine/global/gfx';
|
||||||
import type { NoteBlockModel } from '@blocksuite/affine/model';
|
import type { NoteBlockModel } from '@blocksuite/affine/model';
|
||||||
@@ -160,6 +161,59 @@ test.describe('edgeless page block', () => {
|
|||||||
await expect(infoButton).toBeHidden();
|
await expect(infoButton).toBeHidden();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('caret on focusing', async ({ page }) => {
|
||||||
|
const note = page.locator('affine-edgeless-note');
|
||||||
|
await note.click(); // focus note
|
||||||
|
|
||||||
|
// click on title's rear
|
||||||
|
const docTitle = note.locator('edgeless-page-block-title');
|
||||||
|
await expect(docTitle).toBeVisible();
|
||||||
|
await clickLocatorByRatio(page, docTitle, { xRatio: 0.9, yRatio: 0.8 });
|
||||||
|
|
||||||
|
const hasCaretInTitle = await page.evaluate(
|
||||||
|
hasCaretIn,
|
||||||
|
'edgeless-page-block-title'
|
||||||
|
);
|
||||||
|
expect(hasCaretInTitle).toBe(true);
|
||||||
|
|
||||||
|
await clickLocatorByRatio(page, note, { xRatio: 1.1, yRatio: 0.1 }); // cancel note focus
|
||||||
|
await note.click(); // focus note again
|
||||||
|
|
||||||
|
// click on firstParagraph's rear
|
||||||
|
const firstParagraph = note.locator('affine-paragraph:first-child');
|
||||||
|
await expect(firstParagraph).toBeVisible();
|
||||||
|
|
||||||
|
await clickLocatorByRatio(page, firstParagraph, {
|
||||||
|
xRatio: 0.9,
|
||||||
|
yRatio: 0.5,
|
||||||
|
});
|
||||||
|
|
||||||
|
const hasCaretInParagraph = await page.evaluate(
|
||||||
|
hasCaretIn,
|
||||||
|
'affine-paragraph'
|
||||||
|
);
|
||||||
|
expect(hasCaretInParagraph).toBe(true);
|
||||||
|
|
||||||
|
function hasCaretIn(elemSelector: string) {
|
||||||
|
const sel = document.getSelection();
|
||||||
|
if (!sel || sel.rangeCount === 0) return false;
|
||||||
|
|
||||||
|
const startContainer = sel.getRangeAt(0).startContainer;
|
||||||
|
const selContainer =
|
||||||
|
startContainer.nodeType === Node.TEXT_NODE
|
||||||
|
? startContainer.parentElement
|
||||||
|
: startContainer;
|
||||||
|
|
||||||
|
if (!selContainer) return false;
|
||||||
|
|
||||||
|
const closestDstElem = (selContainer as HTMLElement)?.closest(
|
||||||
|
elemSelector
|
||||||
|
);
|
||||||
|
if (!closestDstElem) return false;
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
test('page title should be editable', async ({ page }) => {
|
test('page title should be editable', async ({ page }) => {
|
||||||
const note = page.locator('affine-edgeless-note');
|
const note = page.locator('affine-edgeless-note');
|
||||||
const docTitle = note.locator('edgeless-page-block-title');
|
const docTitle = note.locator('edgeless-page-block-title');
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import {
|
|||||||
clickEdgelessModeButton,
|
clickEdgelessModeButton,
|
||||||
clickView,
|
clickView,
|
||||||
createEdgelessNoteBlock,
|
createEdgelessNoteBlock,
|
||||||
|
dblclickNoteBody,
|
||||||
fitViewportToContent,
|
fitViewportToContent,
|
||||||
focusDocTitle,
|
focusDocTitle,
|
||||||
getSelectedXYWH,
|
getSelectedXYWH,
|
||||||
@@ -35,8 +36,7 @@ test.beforeEach(async ({ page }) => {
|
|||||||
test('should not show hidden note in embed view page mode', async ({
|
test('should not show hidden note in embed view page mode', async ({
|
||||||
page,
|
page,
|
||||||
}) => {
|
}) => {
|
||||||
const note = page.locator('affine-edgeless-note');
|
await dblclickNoteBody(page);
|
||||||
await note.dblclick();
|
|
||||||
await page.keyboard.type('visible content');
|
await page.keyboard.type('visible content');
|
||||||
await createEdgelessNoteBlock(page, [100, 100]);
|
await createEdgelessNoteBlock(page, [100, 100]);
|
||||||
await page.keyboard.press('Enter');
|
await page.keyboard.press('Enter');
|
||||||
|
|||||||
@@ -6,6 +6,8 @@ import type { ParagraphBlockComponent } from '@blocksuite/affine-block-paragraph
|
|||||||
import type { BlockComponent } from '@blocksuite/std';
|
import type { BlockComponent } from '@blocksuite/std';
|
||||||
import { expect, type Locator, type Page } from '@playwright/test';
|
import { expect, type Locator, type Page } from '@playwright/test';
|
||||||
|
|
||||||
|
import { dblclickLocatorByRatio } from './utils';
|
||||||
|
|
||||||
const EDGELESS_TOOLBAR_WIDGET = 'edgeless-toolbar-widget';
|
const EDGELESS_TOOLBAR_WIDGET = 'edgeless-toolbar-widget';
|
||||||
export const ZERO_WIDTH_FOR_EMPTY_LINE =
|
export const ZERO_WIDTH_FOR_EMPTY_LINE =
|
||||||
process.env.BROWSER === 'webkit' ? '\u200C' : '\u200B';
|
process.env.BROWSER === 'webkit' ? '\u200C' : '\u200B';
|
||||||
@@ -65,6 +67,11 @@ export function locateEditorContainer(page: Page, editorIndex = 0) {
|
|||||||
return page.locator('[data-affine-editor-container]').nth(editorIndex);
|
return page.locator('[data-affine-editor-container]').nth(editorIndex);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export async function dblclickNoteBody(page: Page) {
|
||||||
|
const note = page.locator('affine-edgeless-note');
|
||||||
|
await dblclickLocatorByRatio(page, note, { yRatio: 0.7 });
|
||||||
|
}
|
||||||
|
|
||||||
export function locateDocTitle(page: Page, editorIndex = 0) {
|
export function locateDocTitle(page: Page, editorIndex = 0) {
|
||||||
return locateEditorContainer(page, editorIndex).locator('doc-title');
|
return locateEditorContainer(page, editorIndex).locator('doc-title');
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import { setTimeout } from 'node:timers/promises';
|
import { setTimeout } from 'node:timers/promises';
|
||||||
|
|
||||||
import type { Locator, Page } from '@playwright/test';
|
import type { Locator, Page } from '@playwright/test';
|
||||||
|
import { expect } from '@playwright/test';
|
||||||
import fs from 'fs-extra';
|
import fs from 'fs-extra';
|
||||||
|
|
||||||
export async function waitForLogMessage(
|
export async function waitForLogMessage(
|
||||||
@@ -70,3 +71,48 @@ export async function isContainedInBoundingBox(
|
|||||||
}
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Click at a specific position relative to a locator's bounding box.
|
||||||
|
* * Ratios are NOT clamped:
|
||||||
|
* - 0 ~ 1 : inside the bounding box
|
||||||
|
* - < 0 : outside (left / top of the box)
|
||||||
|
* - > 1 : outside (right / bottom of the box)
|
||||||
|
*
|
||||||
|
* @param locator The locator to click
|
||||||
|
* @param options Optional click position ratios
|
||||||
|
* @param options.xRatio Horizontal ratio relative to box width (not clamped), default is 0.5 (center)
|
||||||
|
* @param options.yRatio Vertical ratio relative to box height (not clamped), default is 0.5 (center)
|
||||||
|
*/
|
||||||
|
export async function clickLocatorByRatio(
|
||||||
|
page: Page,
|
||||||
|
locator: Locator,
|
||||||
|
{ xRatio = 0.5, yRatio = 0.5 } = {}
|
||||||
|
) {
|
||||||
|
const box = await getLocatorBox(locator);
|
||||||
|
|
||||||
|
await page.mouse.click(
|
||||||
|
box.x + box.width * xRatio,
|
||||||
|
box.y + box.height * yRatio
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function dblclickLocatorByRatio(
|
||||||
|
page: Page,
|
||||||
|
locator: Locator,
|
||||||
|
{ xRatio = 0.5, yRatio = 0.5 } = {}
|
||||||
|
) {
|
||||||
|
const box = await getLocatorBox(locator);
|
||||||
|
|
||||||
|
await page.mouse.dblclick(
|
||||||
|
box.x + box.width * xRatio,
|
||||||
|
box.y + box.height * yRatio
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function getLocatorBox(locator: Locator) {
|
||||||
|
await expect(locator).toBeVisible();
|
||||||
|
const box = await locator.boundingBox();
|
||||||
|
if (!box) throw new Error(`error getting locator's bounding box`);
|
||||||
|
return box;
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user