Skip to content

Commit dd60d91

Browse files
harbournickharbournick
committed
fix: insertContentAt fails if new line characters (\n) inserted
1 parent 53523cf commit dd60d91

File tree

2 files changed

+262
-85
lines changed

2 files changed

+262
-85
lines changed

‎packages/super-editor/src/core/commands/insertContentAt.js‎

Lines changed: 112 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,108 +1,135 @@
11
import { createNodeFromContent } from '../helpers/createNodeFromContent';
22
import { selectionToInsertionEnd } from '../helpers/selectionToInsertionEnd';
33

4+
/**
5+
* @typedef {import("prosemirror-model").Node} ProseMirrorNode
6+
* @typedef {import("prosemirror-model").Fragment} ProseMirrorFragment
7+
*/
8+
9+
/**
10+
* Checks if the given node or fragment is a ProseMirror Fragment.
11+
* @param {ProseMirrorNode|ProseMirrorFragment} nodeOrFragment
12+
* @returns {boolean}
13+
*/
414
const isFragment = (nodeOrFragment) => {
515
return !('type' in nodeOrFragment);
616
};
717

8-
//prettier-ignore
9-
export const insertContentAt = (position, value, options) => ({ tr, dispatch, editor }) => {
10-
if (dispatch) {
11-
options = {
12-
parseOptions: {},
13-
updateSelection: true,
14-
applyInputRules: false,
15-
applyPasteRules: false,
16-
...options,
17-
};
18-
19-
let content;
20-
21-
try {
22-
content = createNodeFromContent(value, editor.schema, {
23-
parseOptions: {
24-
preserveWhitespace: 'full',
25-
...options.parseOptions,
26-
},
27-
errorOnInvalidContent: options.errorOnInvalidContent ?? editor.options.enableContentCheck,
28-
});
29-
} catch (e) {
30-
editor.emit('contentError', {
31-
editor,
32-
error: e,
33-
disableCollaboration: () => {
34-
console.error('[super-editor error]: Unable to disable collaboration at this point in time');
35-
},
36-
});
37-
return false;
38-
}
18+
/**
19+
* Inserts content at the specified position.
20+
* @param {import("prosemirror-model").ResolvedPos} position
21+
* @param {string|Array<string|ProseMirrorNode>} value
22+
* @param {Object} options
23+
* @returns
24+
*/
25+
export const insertContentAt =
26+
(position, value, options) =>
27+
({ tr, dispatch, editor }) => {
28+
if (dispatch) {
29+
options = {
30+
parseOptions: {},
31+
updateSelection: true,
32+
applyInputRules: false,
33+
applyPasteRules: false,
34+
...options,
35+
};
36+
37+
let content;
38+
39+
try {
40+
content = createNodeFromContent(value, editor.schema, {
41+
parseOptions: {
42+
preserveWhitespace: 'full',
43+
...options.parseOptions,
44+
},
45+
errorOnInvalidContent: options.errorOnInvalidContent ?? editor.options.enableContentCheck,
46+
});
47+
} catch (e) {
48+
editor.emit('contentError', {
49+
editor,
50+
error: e,
51+
disableCollaboration: () => {
52+
console.error('[super-editor error]: Unable to disable collaboration at this point in time');
53+
},
54+
});
55+
return false;
56+
}
3957

40-
let { from, to } =
41-
typeof position === 'number' ? { from: position, to: position } : { from: position.from, to: position.to };
58+
let { from, to } =
59+
typeof position === 'number' ? { from: position, to: position } : { from: position.from, to: position.to };
4260

43-
let isOnlyTextContent = true;
44-
let isOnlyBlockContent = true;
45-
const nodes = isFragment(content) ? content : [content];
61+
// If the original input is plainly textual, prefer insertText regardless of how parsing represents it.
62+
const forceTextInsert =
63+
typeof value === 'string' ||
64+
(Array.isArray(value) && value.every((v) => typeof v === 'string' || (v && typeof v.text === 'string'))) ||
65+
(value && typeof value === 'object' && typeof value.text === 'string');
4666

47-
nodes.forEach((node) => {
48-
// check if added node is valid
49-
node.check();
67+
let isOnlyTextContent = forceTextInsert; // start true for plain text inputs
68+
let isOnlyBlockContent = true;
69+
const nodes = isFragment(content) ? content : [content];
5070

51-
isOnlyTextContent = isOnlyTextContent ? node.isText && node.marks.length === 0 : false;
71+
nodes.forEach((node) => {
72+
// check if added node is valid
73+
node.check();
5274

53-
isOnlyBlockContent = isOnlyBlockContent ? node.isBlock : false;
54-
});
75+
// only refine text heuristic if we are NOT forcing text insertion based on the original value
76+
if (!forceTextInsert) {
77+
isOnlyTextContent = isOnlyTextContent ? node.isText && node.marks.length === 0 : false;
78+
}
5579

56-
// check if we can replace the wrapping node by
57-
// the newly inserted content
58-
// example:
59-
// replace an empty paragraph by an inserted image
60-
// instead of inserting the image below the paragraph
61-
if (from === to && isOnlyBlockContent) {
62-
const { parent } = tr.doc.resolve(from);
63-
const isEmptyTextBlock = parent.isTextblock && !parent.type.spec.code && !parent.childCount;
80+
isOnlyBlockContent = isOnlyBlockContent ? node.isBlock : false;
81+
});
6482

65-
if (isEmptyTextBlock) {
66-
from -= 1;
67-
to += 1;
83+
// check if we can replace the wrapping node by
84+
// the newly inserted content
85+
// example:
86+
// replace an empty paragraph by an inserted image
87+
// instead of inserting the image below the paragraph
88+
if (from === to && isOnlyBlockContent) {
89+
const { parent } = tr.doc.resolve(from);
90+
const isEmptyTextBlock = parent.isTextblock && !parent.type.spec.code && !parent.childCount;
91+
92+
if (isEmptyTextBlock) {
93+
from -= 1;
94+
to += 1;
95+
}
6896
}
69-
}
7097

71-
let newContent;
72-
73-
// if there is only plain text we have to use `insertText`
74-
// because this will keep the current marks
75-
if (isOnlyTextContent) {
76-
// if value is string, we can use it directly
77-
// otherwise if it is an array, we have to join it
78-
if (Array.isArray(value)) {
79-
newContent = value.map((v) => v.text || '').join('');
80-
} else if (typeof value === 'object' && !!value && !!value.text) {
81-
newContent = value.text;
98+
let newContent;
99+
100+
// if there is only plain text we have to use `insertText`
101+
// because this will keep the current marks
102+
if (isOnlyTextContent) {
103+
// if value is string, we can use it directly
104+
// otherwise if it is an array, we have to join it
105+
if (Array.isArray(value)) {
106+
newContent = value.map((v) => (typeof v === 'string' ? v : (v && v.text) || '')).join('');
107+
} else if (typeof value === 'object' && !!value && !!value.text) {
108+
newContent = value.text;
109+
} else {
110+
newContent = value;
111+
}
112+
113+
tr.insertText(newContent, from, to);
82114
} else {
83-
newContent = value;
84-
}
85-
86-
tr.insertText(newContent, from, to);
87-
} else {
88-
newContent = content;
115+
newContent = content;
89116

90-
tr.replaceWith(from, to, newContent);
91-
}
117+
tr.replaceWith(from, to, newContent);
118+
}
92119

93-
// set cursor at end of inserted content
94-
if (options.updateSelection) {
95-
selectionToInsertionEnd(tr, tr.steps.length - 1, -1);
96-
}
120+
// set cursor at end of inserted content
121+
if (options.updateSelection) {
122+
selectionToInsertionEnd(tr, tr.steps.length - 1, -1);
123+
}
97124

98-
if (options.applyInputRules) {
99-
tr.setMeta('applyInputRules', { from, text: newContent });
100-
}
125+
if (options.applyInputRules) {
126+
tr.setMeta('applyInputRules', { from, text: newContent });
127+
}
101128

102-
if (options.applyPasteRules) {
103-
tr.setMeta('applyPasteRules', { from, text: newContent });
129+
if (options.applyPasteRules) {
130+
tr.setMeta('applyPasteRules', { from, text: newContent });
131+
}
104132
}
105-
}
106133

107-
return true;
108-
};
134+
return true;
135+
};

‎packages/super-editor/src/core/commands/insertContentAt.test.js‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
2+
3+
vi.mock('../helpers/createNodeFromContent', () => ({
4+
createNodeFromContent: vi.fn(),
5+
}));
6+
7+
vi.mock('../helpers/selectionToInsertionEnd', () => ({
8+
selectionToInsertionEnd: vi.fn(),
9+
}));
10+
11+
import { createNodeFromContent } from '../helpers/createNodeFromContent';
12+
import { selectionToInsertionEnd } from '../helpers/selectionToInsertionEnd';
13+
import { insertContentAt } from './insertContentAt';
14+
15+
const makeTr = (overrides = {}) => ({
16+
insertText: vi.fn(),
17+
replaceWith: vi.fn(),
18+
setMeta: vi.fn(),
19+
steps: [1],
20+
doc: {
21+
resolve: vi.fn().mockReturnValue({
22+
parent: {
23+
isTextblock: true,
24+
type: { spec: {} },
25+
childCount: 0,
26+
},
27+
}),
28+
},
29+
...overrides,
30+
});
31+
32+
const makeEditor = (overrides = {}) => ({
33+
schema: {},
34+
options: { enableContentCheck: true },
35+
emit: vi.fn(),
36+
...overrides,
37+
});
38+
39+
describe('insertContentAt', () => {
40+
beforeEach(() => {
41+
vi.clearAllMocks();
42+
});
43+
44+
afterEach(() => {
45+
vi.restoreAllMocks();
46+
});
47+
48+
it('inserts plain text via tr.insertText when given a simple string', () => {
49+
const value = 'Hello world';
50+
// Return a proper Node (has `type`) so isFragment(...) === false
51+
createNodeFromContent.mockImplementation(() => ({
52+
type: { name: 'text' },
53+
isText: true,
54+
isBlock: false,
55+
marks: [],
56+
check: vi.fn(),
57+
}));
58+
59+
const tr = makeTr();
60+
const editor = makeEditor();
61+
62+
const cmd = insertContentAt(5, value, { updateSelection: true });
63+
const result = cmd({ tr, dispatch: true, editor });
64+
65+
expect(result).toBe(true);
66+
expect(createNodeFromContent).toHaveBeenCalled();
67+
expect(tr.insertText).toHaveBeenCalledWith('Hello world', 5, 5);
68+
expect(tr.replaceWith).not.toHaveBeenCalled();
69+
expect(selectionToInsertionEnd).toHaveBeenCalledWith(tr, tr.steps.length - 1, -1);
70+
});
71+
72+
it('applies input rules meta when applyInputRules=true (text case)', () => {
73+
const value = 'abc';
74+
createNodeFromContent.mockImplementation(() => ({
75+
type: { name: 'text' },
76+
isText: true,
77+
isBlock: false,
78+
marks: [],
79+
check: vi.fn(),
80+
}));
81+
82+
const tr = makeTr();
83+
const editor = makeEditor();
84+
85+
const cmd = insertContentAt({ from: 2, to: 4 }, value, {
86+
updateSelection: false,
87+
applyInputRules: true,
88+
});
89+
const result = cmd({ tr, dispatch: true, editor });
90+
91+
expect(result).toBe(true);
92+
expect(tr.insertText).toHaveBeenCalledWith('abc', 2, 4);
93+
expect(tr.setMeta).toHaveBeenCalledWith('applyInputRules', { from: 2, text: 'abc' });
94+
});
95+
96+
it('replaces an empty paragraph when only block content is inserted', () => {
97+
const blockNode = {
98+
type: { name: 'paragraph' }, // still a Node
99+
isText: false,
100+
isBlock: true,
101+
marks: [],
102+
check: vi.fn(),
103+
};
104+
createNodeFromContent.mockImplementation(() => blockNode);
105+
106+
const tr = makeTr({
107+
doc: {
108+
resolve: vi.fn().mockReturnValue({
109+
parent: {
110+
isTextblock: true,
111+
type: { spec: {} },
112+
childCount: 0,
113+
},
114+
}),
115+
},
116+
});
117+
118+
const editor = makeEditor();
119+
const cmd = insertContentAt(10, { type: 'paragraph' }, { updateSelection: true });
120+
const result = cmd({ tr, dispatch: true, editor });
121+
122+
expect(result).toBe(true);
123+
expect(tr.replaceWith).toHaveBeenCalledWith(9, 11, blockNode);
124+
});
125+
126+
// https://github.com/Harbour-Enterprises/SuperDoc/issues/842
127+
it('when value has newlines, still inserts text using tr.insertText', () => {
128+
const value = 'Line 1\nLine 2';
129+
130+
// Simulate a Fragment (array, no `type` on container) so isFragment(...) === true
131+
const fragment = [
132+
{ isText: true, isBlock: false, marks: [], check: vi.fn() }, // "Line 1"
133+
{ isText: false, isBlock: false, marks: [], check: vi.fn() }, // <hardBreak>
134+
{ isText: true, isBlock: false, marks: [], check: vi.fn() }, // "Line 2"
135+
];
136+
createNodeFromContent.mockImplementation(() => fragment);
137+
138+
const tr = makeTr();
139+
const editor = makeEditor();
140+
141+
const cmd = insertContentAt(3, value, { updateSelection: true });
142+
const result = cmd({ tr, dispatch: true, editor });
143+
144+
expect(result).toBe(true);
145+
146+
// Desired behavior (will currently fail): insertText used for raw strings with '\n'
147+
expect(tr.insertText).toHaveBeenCalledWith('Line 1\nLine 2', 3, 3);
148+
expect(tr.replaceWith).not.toHaveBeenCalled();
149+
});
150+
});

0 commit comments

Comments
 (0)