Skip to content

Commit f0959bd

Browse files
authored
Merge pull request #2214 from pie-framework/fix/PIE-439
fix: made sure multiple divs are treated as line breaks [PIE-439]
2 parents d19651a + 2b507e0 commit f0959bd

5 files changed

Lines changed: 428 additions & 3 deletions

File tree

packages/editable-html-tip-tap/src/__tests__/EditableHtml.test.jsx

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,4 +349,126 @@ describe('EditableHtml', () => {
349349

350350
jest.useRealTimers();
351351
});
352+
353+
describe('onUpdate callback', () => {
354+
it('calls onChange when transaction.isDone is true', async () => {
355+
const onChange = jest.fn();
356+
const markup = '<p>Initial content</p>';
357+
358+
render(<EditableHtml {...defaultProps} markup={markup} onChange={onChange} />);
359+
360+
await waitFor(() => {
361+
expect(useEditor).toHaveBeenCalled();
362+
});
363+
364+
const editorConfig = useEditor.mock.calls[useEditor.mock.calls.length - 1][0];
365+
const mockEditor = {
366+
getHTML: jest.fn(() => '<p>Updated content</p>'),
367+
};
368+
369+
const mockTransaction = {
370+
isDone: true,
371+
};
372+
373+
editorConfig.onUpdate({ editor: mockEditor, transaction: mockTransaction });
374+
375+
expect(onChange).toHaveBeenCalledWith('<p>Updated content</p>');
376+
});
377+
378+
it('calls onChange when markup differs from editor HTML', async () => {
379+
const onChange = jest.fn();
380+
const markup = '<p>Initial content</p>';
381+
382+
render(<EditableHtml {...defaultProps} markup={markup} onChange={onChange} />);
383+
384+
await waitFor(() => {
385+
expect(useEditor).toHaveBeenCalled();
386+
});
387+
388+
const editorConfig = useEditor.mock.calls[useEditor.mock.calls.length - 1][0];
389+
const mockEditor = {
390+
getHTML: jest.fn(() => '<p>Different content</p>'),
391+
};
392+
393+
const mockTransaction = {
394+
isDone: false,
395+
};
396+
397+
editorConfig.onUpdate({ editor: mockEditor, transaction: mockTransaction });
398+
399+
expect(onChange).toHaveBeenCalledWith('<p>Different content</p>');
400+
});
401+
402+
it('does not call onChange when transaction.isDone is false and markup matches editor HTML', async () => {
403+
const onChange = jest.fn();
404+
const markup = '<p>Same content</p>';
405+
406+
render(<EditableHtml {...defaultProps} markup={markup} onChange={onChange} />);
407+
408+
await waitFor(() => {
409+
expect(useEditor).toHaveBeenCalled();
410+
});
411+
412+
const editorConfig = useEditor.mock.calls[useEditor.mock.calls.length - 1][0];
413+
const mockEditor = {
414+
getHTML: jest.fn(() => '<p>Same content</p>'),
415+
};
416+
417+
const mockTransaction = {
418+
isDone: false,
419+
};
420+
421+
editorConfig.onUpdate({ editor: mockEditor, transaction: mockTransaction });
422+
423+
expect(onChange).not.toHaveBeenCalled();
424+
});
425+
426+
it('calls onChange when transaction.isDone is true even if markup matches', async () => {
427+
const onChange = jest.fn();
428+
const markup = '<p>Same content</p>';
429+
430+
render(<EditableHtml {...defaultProps} markup={markup} onChange={onChange} />);
431+
432+
await waitFor(() => {
433+
expect(useEditor).toHaveBeenCalled();
434+
});
435+
436+
const editorConfig = useEditor.mock.calls[useEditor.mock.calls.length - 1][0];
437+
const mockEditor = {
438+
getHTML: jest.fn(() => '<p>Same content</p>'),
439+
};
440+
441+
const mockTransaction = {
442+
isDone: true,
443+
};
444+
445+
editorConfig.onUpdate({ editor: mockEditor, transaction: mockTransaction });
446+
447+
expect(onChange).toHaveBeenCalledWith('<p>Same content</p>');
448+
});
449+
450+
it('does not call onChange when onChange is not provided', async () => {
451+
const markup = '<p>Content</p>';
452+
453+
render(<EditableHtml {...defaultProps} markup={markup} onChange={undefined} />);
454+
455+
await waitFor(() => {
456+
expect(useEditor).toHaveBeenCalled();
457+
});
458+
459+
const editorConfig = useEditor.mock.calls[useEditor.mock.calls.length - 1][0];
460+
const mockEditor = {
461+
getHTML: jest.fn(() => '<p>Updated content</p>'),
462+
};
463+
464+
const mockTransaction = {
465+
isDone: true,
466+
};
467+
468+
// Should not throw error when onChange is undefined
469+
expect(() => {
470+
editorConfig.onUpdate({ editor: mockEditor, transaction: mockTransaction });
471+
}).not.toThrow();
472+
});
473+
});
352474
});
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
import React from 'react';
2+
import { render, waitFor } from '@testing-library/react';
3+
import { EditableHtml } from '../components/EditableHtml';
4+
5+
describe('Div to Paragraph Conversion', () => {
6+
it('converts consecutive divs to paragraph with br tags', async () => {
7+
const markup = '<div>A</div><div>B</div>';
8+
const { container } = render(<EditableHtml markup={markup} onChange={() => {}} pluginProps={{}} />);
9+
10+
// Wait for the editor to initialize
11+
await waitFor(() => {
12+
const prosemirror = container.querySelector('.ProseMirror');
13+
expect(prosemirror).toBeInTheDocument();
14+
});
15+
16+
// Check that the content was converted to a paragraph
17+
const paragraph = container.querySelector('.ProseMirror p');
18+
expect(paragraph).toBeInTheDocument();
19+
20+
// Check that br tag is present
21+
const br = container.querySelector('.ProseMirror p br');
22+
expect(br).toBeInTheDocument();
23+
24+
// Verify the text content
25+
expect(paragraph.textContent).toBe('AB');
26+
});
27+
28+
it('converts three consecutive divs correctly', async () => {
29+
const markup = '<div>First</div><div>Second</div><div>Third</div>';
30+
const { container } = render(<EditableHtml markup={markup} onChange={() => {}} pluginProps={{}} />);
31+
32+
await waitFor(() => {
33+
const prosemirror = container.querySelector('.ProseMirror');
34+
expect(prosemirror).toBeInTheDocument();
35+
});
36+
37+
const paragraph = container.querySelector('.ProseMirror p');
38+
expect(paragraph).toBeInTheDocument();
39+
40+
// Should have 2 br tags (between 3 items)
41+
const brTags = container.querySelectorAll('.ProseMirror p br');
42+
expect(brTags.length).toBe(2);
43+
44+
expect(paragraph.textContent).toBe('FirstSecondThird');
45+
});
46+
47+
it('does not convert single div', async () => {
48+
const markup = '<div>Single</div>';
49+
const { container } = render(<EditableHtml markup={markup} onChange={() => {}} pluginProps={{}} />);
50+
51+
await waitFor(() => {
52+
const prosemirror = container.querySelector('.ProseMirror');
53+
expect(prosemirror).toBeInTheDocument();
54+
});
55+
56+
// Should remain as a div
57+
const div = container.querySelector('.ProseMirror div');
58+
expect(div).toBeInTheDocument();
59+
expect(div.textContent).toBe('Single');
60+
});
61+
62+
it('does not convert divs with attributes', async () => {
63+
const markup = '<div class="test">A</div><div>B</div>';
64+
const { container } = render(<EditableHtml markup={markup} onChange={() => {}} pluginProps={{}} />);
65+
66+
await waitFor(() => {
67+
const prosemirror = container.querySelector('.ProseMirror');
68+
expect(prosemirror).toBeInTheDocument();
69+
});
70+
71+
// Should remain as divs since one has an attribute
72+
const divs = container.querySelectorAll('.ProseMirror div');
73+
expect(divs.length).toBeGreaterThanOrEqual(2);
74+
});
75+
76+
it('handles divs with inline formatting', async () => {
77+
const markup = '<div><strong>Bold</strong></div><div><em>Italic</em></div>';
78+
const { container } = render(<EditableHtml markup={markup} onChange={() => {}} pluginProps={{}} />);
79+
80+
await waitFor(() => {
81+
const prosemirror = container.querySelector('.ProseMirror');
82+
expect(prosemirror).toBeInTheDocument();
83+
});
84+
85+
// Should be converted to paragraph
86+
const paragraph = container.querySelector('.ProseMirror p');
87+
expect(paragraph).toBeInTheDocument();
88+
89+
// Check that formatting is preserved
90+
const strong = container.querySelector('.ProseMirror p strong');
91+
const em = container.querySelector('.ProseMirror p em');
92+
expect(strong).toBeInTheDocument();
93+
expect(em).toBeInTheDocument();
94+
});
95+
96+
it('does not convert mixed element types', async () => {
97+
const markup = '<div>A</div><p>B</p>';
98+
const { container } = render(<EditableHtml markup={markup} onChange={() => {}} pluginProps={{}} />);
99+
100+
await waitFor(() => {
101+
const prosemirror = container.querySelector('.ProseMirror');
102+
expect(prosemirror).toBeInTheDocument();
103+
});
104+
105+
// Should have both div and paragraph
106+
const div = container.querySelector('.ProseMirror div');
107+
const p = container.querySelector('.ProseMirror p');
108+
expect(div).toBeInTheDocument();
109+
expect(p).toBeInTheDocument();
110+
});
111+
112+
it('does not convert divs with nested block elements', async () => {
113+
const markup = '<div><div>Nested</div></div><div>B</div>';
114+
const { container } = render(<EditableHtml markup={markup} onChange={() => {}} pluginProps={{}} />);
115+
116+
await waitFor(() => {
117+
const prosemirror = container.querySelector('.ProseMirror');
118+
expect(prosemirror).toBeInTheDocument();
119+
});
120+
121+
// Should remain as divs
122+
const divs = container.querySelectorAll('.ProseMirror > div');
123+
expect(divs.length).toBeGreaterThanOrEqual(1);
124+
});
125+
});

packages/editable-html-tip-tap/src/components/EditableHtml.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ export const EditableHtml = (props) => {
288288
editable: !props.disabled,
289289
content: normalizeInitialMarkup(props.markup),
290290
onUpdate: ({ editor, transaction }) => {
291-
if (transaction.isDone) {
291+
if (transaction.isDone || props.markup !== editor.getHTML()) {
292292
props.onChange?.(editor.getHTML());
293293
}
294294
},
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import { normalizeInitialMarkup } from '../helper';
2+
3+
describe('normalizeInitialMarkup', () => {
4+
describe('basic normalization', () => {
5+
it('returns empty div for empty string', () => {
6+
expect(normalizeInitialMarkup('')).toBe('<div></div>');
7+
});
8+
9+
it('returns empty div for null', () => {
10+
expect(normalizeInitialMarkup(null)).toBe('<div></div>');
11+
});
12+
13+
it('returns empty div for undefined', () => {
14+
expect(normalizeInitialMarkup(undefined)).toBe('<div></div>');
15+
});
16+
17+
it('wraps plain text in div', () => {
18+
expect(normalizeInitialMarkup('Hello')).toBe('<div>Hello</div>');
19+
});
20+
21+
it('returns HTML tags as-is when detected as HTML', () => {
22+
// Since '<script>' matches the HTML pattern, it's returned as-is
23+
// To be escaped, it would need to not match the HTML pattern
24+
expect(normalizeInitialMarkup('<script>')).toBe('<script>');
25+
});
26+
27+
it('escapes HTML entities in plain text', () => {
28+
// Plain text without angle brackets gets escaped
29+
expect(normalizeInitialMarkup('Hello & World')).toBe('<div>Hello &amp; World</div>');
30+
});
31+
32+
it('returns single div as-is', () => {
33+
const html = '<div>Hello</div>';
34+
expect(normalizeInitialMarkup(html)).toBe(html);
35+
});
36+
37+
it('returns paragraph as-is', () => {
38+
const html = '<p>Hello</p>';
39+
expect(normalizeInitialMarkup(html)).toBe(html);
40+
});
41+
});
42+
43+
describe('consecutive divs to paragraph conversion', () => {
44+
it('converts two consecutive divs to paragraph with br', () => {
45+
const input = '<div>A</div><div>B</div>';
46+
const expected = '<p>A<br>B</p>';
47+
expect(normalizeInitialMarkup(input)).toBe(expected);
48+
});
49+
50+
it('converts three consecutive divs to paragraph with br tags', () => {
51+
const input = '<div>A</div><div>B</div><div>C</div>';
52+
const expected = '<p>A<br>B<br>C</p>';
53+
expect(normalizeInitialMarkup(input)).toBe(expected);
54+
});
55+
56+
it('handles divs with whitespace', () => {
57+
const input = '<div> A </div><div> B </div>';
58+
const expected = '<p> A <br> B </p>';
59+
expect(normalizeInitialMarkup(input)).toBe(expected);
60+
});
61+
62+
it('handles divs with inline elements', () => {
63+
const input = '<div><strong>A</strong></div><div><em>B</em></div>';
64+
const expected = '<p><strong>A</strong><br><em>B</em></p>';
65+
expect(normalizeInitialMarkup(input)).toBe(expected);
66+
});
67+
68+
it('handles empty divs', () => {
69+
const input = '<div></div><div>B</div>';
70+
const expected = '<p><br>B</p>';
71+
expect(normalizeInitialMarkup(input)).toBe(expected);
72+
});
73+
74+
it('preserves existing br tags within divs', () => {
75+
const input = '<div>A<br>A2</div><div>B</div>';
76+
const expected = '<p>A<br>A2<br>B</p>';
77+
expect(normalizeInitialMarkup(input)).toBe(expected);
78+
});
79+
});
80+
81+
describe('cases that should NOT convert', () => {
82+
it('does not convert single div', () => {
83+
const input = '<div>Hello</div>';
84+
expect(normalizeInitialMarkup(input)).toBe(input);
85+
});
86+
87+
it('does not convert divs with nested block elements', () => {
88+
const input = '<div><div>Nested</div></div><div>B</div>';
89+
expect(normalizeInitialMarkup(input)).toBe(input);
90+
});
91+
92+
it('does not convert divs containing tables', () => {
93+
const input = '<div><table><tr><td>A</td></tr></table></div><div>B</div>';
94+
expect(normalizeInitialMarkup(input)).toBe(input);
95+
});
96+
97+
it('does not convert divs containing lists', () => {
98+
const input = '<div><ul><li>Item</li></ul></div><div>B</div>';
99+
expect(normalizeInitialMarkup(input)).toBe(input);
100+
});
101+
102+
it('does not convert mixed element types', () => {
103+
const input = '<div>A</div><p>B</p>';
104+
expect(normalizeInitialMarkup(input)).toBe(input);
105+
});
106+
107+
it('does not convert paragraphs', () => {
108+
const input = '<p>A</p><p>B</p>';
109+
expect(normalizeInitialMarkup(input)).toBe(input);
110+
});
111+
});
112+
113+
describe('edge cases', () => {
114+
it('handles divs with attributes', () => {
115+
const input = '<div class="test">A</div><div>B</div>';
116+
// Should not convert since we only want simple divs
117+
expect(normalizeInitialMarkup(input)).toBe(input);
118+
});
119+
120+
it('handles complex inline formatting', () => {
121+
const input = '<div><strong><em>A</em></strong></div><div><u>B</u></div>';
122+
const expected = '<p><strong><em>A</em></strong><br><u>B</u></p>';
123+
expect(normalizeInitialMarkup(input)).toBe(expected);
124+
});
125+
});
126+
});

0 commit comments

Comments
 (0)