fix(text-block): preserve line breaks on plain text #11794
Conversation
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
hamza221
requested review from
ChristophWurst,
GretaD and
kesselb
as code owners
|
/backport to stable5.5 |
| const itemElement = writer.createElement('paragraph') | ||
| writer.insertText(item.content, itemElement) | ||
| editor.model.insertContent(itemElement) | ||
| const lines = item.content.split('\n') |
There was a problem hiding this comment.
Can you explain the logic behind this? I have created a test text block with both types of line breaks: a regular one and a paragraph. The database content is this: <p style="margin:0;">Paragraph 1 Line 1<br>Paragraph 1 Line 2</p><p style="margin:0;">Paragraph 2 Line 1</p>.
There was a problem hiding this comment.
The text is created in Ckeditor intially in the text block config so it will look like
<p> line one </p> <p> line 2</p>
then on insert it will get converted to
line one\nline 2
which here will get reconverted to
<p> line one </p> <p> line 2</p>
There was a problem hiding this comment.
Test with this input in the text block:
- a
- Shift+Enter
- b
- Enter
- c
Then you will have two paragraphs. The first one has a line break. Do you still have this format after converting to line breaks and back?
There was a problem hiding this comment.
yes that works, because the intermediate to Plain conversion would have \nfor both paragraphs and <br>would look like this a\nb\nc
There was a problem hiding this comment.
visually it's the same, but it would only create <p> which is okay imo for plain text, because at the end of day the html wont be sent and visually it's exactly the same
There was a problem hiding this comment.
I would expect \n for a <br> and \n\n for a <p>
There was a problem hiding this comment.
That would be a separate bug in https://github.com/nextcloud/mail/blob/fix/text-block-multiline/src/util/text.js#L96
no?
| writer.insertText(item.content, itemElement) | ||
| editor.model.insertContent(itemElement) | ||
| const lines = item.content.split('\n') | ||
| const htmlContent = lines.map(line => `<p>${line}</p>`).join('') |
There was a problem hiding this comment.
I'm skeptical about the use of paragraphs for line breaks. <br> and </p><p> is not the same semantically and also often looks different visually. You will have a regular line height between line breaks and more space between paragraphs.
There was a problem hiding this comment.
that's how CkEditor creates line breaks internally, we can reconfigure it to <br> https://ckeditor.com/docs/ckeditor4/latest/features/enterkey.html
There was a problem hiding this comment.
I couldn't find docs for ckeditor5 but if you inspect composer and press enter you can see it live
fix #11729