Skip to content

Preserve line breaks in question rich text (fixes #372) - #373

Merged
Famousmaster206 merged 1 commit into
AP-Students:mainfrom
saa938:fix/issue-372-explanation-line-breaks
Aug 18, 2026
Merged

Preserve line breaks in question rich text (fixes #372)#373
Famousmaster206 merged 1 commit into
AP-Students:mainfrom
saa938:fix/issue-372-explanation-line-breaks

Conversation

@saa938

@saa938 saa938 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes #372 — line breaks typed into the Explanation box were removed after saving.

This turned out to be two independent bugs, both introduced by the rich-text question editor in #362. Either one alone produces the reported symptom, so both need fixing.

1. EditorJS deleted the line breaks on save

QuestionsAddCard declared no sanitize config. A block tool without one inherits the merged tag list of the enabled inline tools, and EditorJS applies it to every string in the saved block data. That list contains no <br> and no <div>, so editor.save() silently rewrote every question and explanation:

before save:  First paragraph.<div><br></div><div>Second paragraph.</div>
after save:   First paragraph.Second paragraph.

This is the "text entered on separate lines is combined into one paragraph" half of the report. It only affects the article creator — the admin test page (/admin/subject/.../test/[id]) writes straight to Firestore and never goes through EditorJS, which is why the bug looked inconsistent.

Fixed by declaring a sanitize config. true means "leave this value as-is" in EditorJS's sanitizer. That is safe because question rich text is already sanitized by sanitizeQuestionRichText (DOMPurify with an explicit allow-list, no attributes) — once when the editor writes it, and again in RenderContent at render time. The stricter, purpose-built sanitizer stays in charge; we just stop EditorJS from applying a different vocabulary on top of it.

2. The editor collapsed newlines in existing questions

The Explanation box is now a contentEditable, which is white-space: normal. Questions written before it became rich text store their breaks as plain newline characters — 910 of 1048 explanations currently in production. HTML collapses each \n to a single space, so opening any older question showed its paragraphs already merged, before saving anything.

This is the "a space is inserted where the line break previously existed" half of the report, and it explains why re-adding the breaks and re-saving never helped: the author was editing a view that had already lost them. The stored data and the learner-facing view were both correct the whole time — only the editor was wrong.

Fixed by adding whitespace-pre-wrap to the editor, matching the whitespace-pre-wrap the learner renderer (RenderContent) already uses. The two views now agree, which is also the last acceptance criterion on the issue.

Related

One thing a reviewer should know

Cause 1 means EditorJS is no longer stripping attributes from question data, so the data-empty markers EditorJS stamps onto elements now persist into stored content. They are inertrenderNode switches on tag name and ignores attributes entirely, and DOMPurify still strips every non-data-* attribute at render — but they are noise.

I tried removing them with ALLOW_DATA_ATTR: false and reverted it: EditorJS re-stamps data-empty continuously, so the sanitized HTML then differs from the live DOM on every keystroke, causing emitChange to rewrite innerHTML and reset the caret. Typing came out reversed (.hpargarap dnoceS). A proper fix needs normalized comparison in emitChange and the value effect, which is out of scope here and worth its own issue.

Pull request type

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Two files, 29 insertions, 2 deletions. No API, data-format, or schema changes, and no migration needed — existing content in both the old plain-text form and the new rich-text form renders correctly.

Demo

The bug is about exact saved values, so the clearest demo is the string that reaches storage. Typing First paragraph. / Enter / Enter / Second paragraph. into an Explanation box, then saving:

Before

editor content:  "First paragraph.<div><br></div><div>Second paragraph.</div>"
after save:      "First paragraph.Second paragraph."          <- breaks gone
learner sees:    "First paragraph.Second paragraph."

After

editor content:  "First paragraph.<div><br></div><div>Second paragraph.</div>"
after save:      "First paragraph.<div><br></div><div>Second paragraph.</div>"
learner sees:    "First paragraph.\n\nSecond paragraph."

Opening an existing question whose explanation is stored as "First paragraph.\n\nSecond paragraph.":

before:  Explanation box shows  "First paragraph. Second paragraph."   <- one line, space inserted
after:   Explanation box shows  "First paragraph.
                                 
                                 Second paragraph."

How Has This Been Tested? How can the reviewer test it?

How it was tested. I drove the real components in a headless browser through the actual EditorJS save path (not a stand-in), using temporary harness pages that mounted QuestionsAddCard in a real EditorJS instance plus RenderContent for the learner view. The harness pages were deleted before committing; only the two source files are in this PR.

The acceptance checks, taken from the issue's criteria, score 6/6 with this change and 1/6 without it. On unfixed code the failure output is exactly the reported symptom — typing four lines and saving yields "Alpha.Beta.Gamma.Delta.". Checks covered:

  • a single Enter produces a break that survives the save
  • Enter twice produces a blank line that survives the save
  • three consecutive breaks all survive
  • the saved value still contains break markup rather than concatenated text
  • the learner view's line structure matches the editor's exactly
  • lines stay in order (guards the caret regression described above)

I also confirmed against production data that explanations really are stored in two different shapes (910 of 1048 with plain \n), which is what motivated fix 2.

npm run lint reports no new warnings, and npm run build compiles successfully.

How to test it yourself.

The regression that shipped (cause 1):

  1. Open any chapter in the article creator and add or open a Questions block.
  2. In Explanation, type First paragraph., press Enter twice, type Second paragraph.
  3. Save the article, then reload the page and reopen the question.
  4. On main the two paragraphs come back merged into one; with this branch the blank line is still there.

The existing-content half (cause 2):

  1. Open any question authored before Text Features #362 — most of them, e.g. anything under AP Biology Unit 2 — whose explanation has multiple paragraphs.
  2. Look at the Explanation box before touching anything.
  3. On main the paragraphs are run together with a space; with this branch they are on separate lines, matching what the learner already sees.

Worth confirming no regressions: bold / italic / underline / highlight still apply and toggle, pasting still works, LaTeX ($@...$) and ``` code blocks still render, and the admin test page editor (/admin/subject/.../test/[id]) behaves as before — it was never affected by cause 1.

Checklist

  • I have performed a self-review of my own code

Line breaks typed into the Explanation box disappeared. Two separate
causes, both from the rich-text question editor:

EditorJS stripped them on save. QuestionsAddCard declared no `sanitize`
config, so EditorJS sanitized the saved block data with the merged tag
list of the enabled inline tools, which contains no `<br>` or `<div>`.
Saving an article rewrote

  First.<div><br></div><div>Second.</div>

as `First.Second.`. Declaring `sanitize` opts the block out; question
rich text is already sanitized by `sanitizeQuestionRichText` with an
explicit allow-list, on write and again at render.

The editor collapsed legacy newlines. The box is a `contentEditable`,
so `white-space: normal` applied, but questions written before it became
rich text store their breaks as newline characters. HTML collapsed each
one to a space, so opening an older explanation showed its paragraphs
already merged even though the stored data and the learner view were
both fine. `whitespace-pre-wrap` matches the learner renderer.
Copilot AI lite review requested due to automatic review settings August 17, 2026 19:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@Famousmaster206 Famousmaster206 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Famousmaster206
Famousmaster206 merged commit 8b6549f into AP-Students:main Aug 18, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Line Breaks Are Removed From Explanation Box After Saving

3 participants