Skip to content

Commit

Permalink
Fix caret jumping in certain cases with cursor blot
Browse files Browse the repository at this point in the history
  • Loading branch information
luin committed Jun 21, 2024
1 parent 5d752e3 commit ff381d0
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 27 deletions.
55 changes: 34 additions & 21 deletions packages/quill/src/core/selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,18 +116,7 @@ class Selection {
});
this.emitter.on(Emitter.events.COMPOSITION_END, () => {
this.composing = false;
if (this.cursor.parent) {
const range = this.cursor.restore();
if (!range) return;
setTimeout(() => {
this.setNativeRange(
range.startNode,
range.startOffset,
range.endNode,
range.endOffset,
);
}, 1);
}
this.restoreCursor();
});
}

Expand Down Expand Up @@ -449,15 +438,7 @@ class Selection {
nativeRange.native.collapsed &&
nativeRange.start.node !== this.cursor.textNode
) {
const range = this.cursor.restore();
if (range) {
this.setNativeRange(
range.startNode,
range.startOffset,
range.endNode,
range.endOffset,
);
}
this.restoreCursor();
}
const args = [
Emitter.events.SELECTION_CHANGE,
Expand All @@ -471,6 +452,38 @@ class Selection {
}
}
}

private restoreCursor() {
if (!this.cursor.parent) return;

const restoredRange = this.cursor.restore();
if (restoredRange) {
this.setNativeRange(
restoredRange.startNode,
restoredRange.startOffset,
restoredRange.endNode,
restoredRange.endOffset,
);
}
this.emitter.once(Emitter.events.SCROLL_OPTIMIZE, () => {
// Re-apply the selection after the optimize phase.
// This is needed in two cases:
// 1. When `restoredRange` is null: this can happen when the caret
// is moved out of the cursor blot by the user.
// 2. When `restoredRange` is not null but either point of it does not
// exist in the DOM tree anymore: this can happen when the cursor blot
// is empty (e.g. when user cancels a composition session) so that the
// wrapping empty blot referred by `restoredRange` is removed in the
// optimize phase.
if (
!restoredRange ||
!this.root.contains(restoredRange.startNode) ||
(restoredRange.endNode && !this.root.contains(restoredRange.endNode))
) {
this.setRange(this.lastRange);
}
});
}
}

function contains(parent: Node, descendant: Node) {
Expand Down
137 changes: 137 additions & 0 deletions packages/quill/test/e2e/cursor.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import { expect } from '@playwright/test';
import { test } from './fixtures/index.js';
import { SHORTKEY } from './utils/index.js';

test.describe('cursor', () => {
test.beforeEach(async ({ editorPage }) => {
await editorPage.open();
});

test.describe('type', () => {
test('normal typing with one format', async ({ page, editorPage }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.type('abc');
expect(await editorPage.getContents()).toEqual([
{ insert: '12' },
{ insert: 'abc', attributes: { bold: true } },
{ insert: '34\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 5, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('normal typing with two formats', async ({ page, editorPage }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.press(`${SHORTKEY}+i`);
await page.keyboard.type('abc');
expect(await editorPage.getContents()).toEqual([
{ insert: '12' },
{ insert: 'abc', attributes: { bold: true, italic: true } },
{ insert: '34\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 5, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('normal typing with one format omitting', async ({
page,
editorPage,
}) => {
await editorPage.setContents([
{ insert: '1234', attributes: { bold: true, italic: true } },
{ insert: '\n' },
]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.type('abc');
expect(await editorPage.getContents()).toEqual([
{ insert: '12', attributes: { bold: true, italic: true } },
{ insert: 'abc', attributes: { italic: true } },
{ insert: '34', attributes: { bold: true, italic: true } },
{ insert: '\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 5, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});
});

test('paste', async ({ clipboard, editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await clipboard.writeText('abc');
await page.keyboard.press(`${SHORTKEY}+b`);
await clipboard.paste();
expect(await editorPage.getContents()).toEqual([
{ insert: '12' },
{ insert: 'abc', attributes: { bold: true } },
{ insert: '34\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 5, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test.describe('IME', () => {
test('confirm composition', async ({ composition, editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
const ime = await composition.start();
await ime.update('w');
await ime.update('o');
await ime.commit('我');
expect(await editorPage.getContents()).toEqual([
{ insert: '12' },
{ insert: '我', attributes: { bold: true } },
{ insert: '34\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 3, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('cancel composition', async ({ composition, editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
const ime = await composition.start();
await ime.update('w');
await ime.update('o');
await ime.cancel();
expect(await editorPage.getContents()).toEqual([{ insert: '1234\n' }]);
expect(await editorPage.getSelection()).toEqual({ index: 2, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});
});

test.describe('caret movements', () => {
test('right arrow key', async ({ editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.press('ArrowRight');
expect(await editorPage.getSelection()).toEqual({ index: 3, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('left arrow key', async ({ editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.press('ArrowLeft');
expect(await editorPage.getSelection()).toEqual({ index: 1, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('jumping', async ({ editorPage, page }) => {
await editorPage.setContents([{ insert: '12345\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await editorPage.moveCursorAfterText('34');
expect(await editorPage.getSelection()).toEqual({ index: 4, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});
});
});
23 changes: 17 additions & 6 deletions packages/quill/test/e2e/fixtures/Composition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
abstract class CompositionSession {
abstract update(key: string): Promise<void>;
abstract commit(committedText: string): Promise<void>;
abstract cancel(): Promise<void>;

protected composingData = '';

Expand Down Expand Up @@ -35,12 +36,7 @@ class ChromiumCompositionSession extends CompositionSession {
async update(key: string) {
await this.withKeyboardEvents(key, async () => {
this.composingData += key;

await this.session.send('Input.imeSetComposition', {
selectionStart: this.composingData.length,
selectionEnd: this.composingData.length,
text: this.composingData,
});
this.updateComposition();
});
}

Expand All @@ -51,6 +47,21 @@ class ChromiumCompositionSession extends CompositionSession {
});
});
}

async cancel() {
await this.withKeyboardEvents('Escape', async () => {
this.composingData = '';
await this.updateComposition();
});
}

private async updateComposition() {
await this.session.send('Input.imeSetComposition', {
selectionStart: this.composingData.length,
selectionEnd: this.composingData.length,
text: this.composingData,
});
}
}

class Composition {
Expand Down
39 changes: 39 additions & 0 deletions packages/quill/test/e2e/fixtures/Locker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { unlink, writeFile } from 'fs/promises';
import { unlinkSync } from 'fs';
import { tmpdir } from 'os';
import { join } from 'path';
import { globSync } from 'glob';

const sleep = (ms: number) =>
new Promise((resolve) => {
setTimeout(resolve, ms);
});

const PREFIX = 'playwright_locker_';

class Locker {
public static clearAll() {
globSync(join(tmpdir(), `${PREFIX}*.txt`)).forEach(unlinkSync);
}

constructor(private key: string) {}

private get filePath() {
return join(tmpdir(), `${PREFIX}${this.key}.txt`);
}

async lock() {
try {
await writeFile(this.filePath, '', { flag: 'wx' });
} catch {
await sleep(50);
await this.lock();
}
}

async release() {
await unlink(this.filePath);
}
}

export default Locker;
4 changes: 4 additions & 0 deletions packages/quill/test/e2e/pageobjects/EditorPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ export default class EditorPage {
return this.page.locator('.ql-editor');
}

get cursorBlot() {
return this.root.locator('.ql-cursor');
}

async open() {
await this.page.goto('/');
await this.page.waitForSelector('.ql-editor', { timeout: 10000 });
Expand Down

0 comments on commit ff381d0

Please sign in to comment.