Collapse double-height empty paragraphs in story editor (#1521)#1554
Open
maebeale wants to merge 1 commit into
Open
Collapse double-height empty paragraphs in story editor (#1521)#1554maebeale wants to merge 1 commit into
maebeale wants to merge 1 commit into
Conversation
Legacy/imported stories use empty `<p><br></p>` paragraphs as spacers. ProseMirror parses the lone <br> as a hard break and then appends its own trailing break for editing, so each spacer renders as two blank lines. This doubled the gap between paragraphs in the editor, making published stories hard to read while editing. Hide the redundant content break in the editor so spacers render as a single blank line. The <br> stays in the DOM, so saved content and the published view are unchanged. The selector is scoped to a content break immediately followed by the trailing break, leaving intentional in-paragraph line breaks untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
maebeale
commented
Jun 5, 2026
| * view are unchanged. Scoped to a content <br> that is immediately followed by | ||
| * the trailing break, so intentional in-paragraph line breaks are untouched. | ||
| */ | ||
| custom-rhino-editor .trix-content p > br:first-child:not(.ProseMirror-trailingBreak):has(+ br.ProseMirror-trailingBreak:last-child) { |
Collaborator
Author
There was a problem hiding this comment.
The selector only matches when the content <br> is the first element child and is immediately followed by the ProseMirror-trailingBreak as the last child — i.e. a paragraph whose only content is the redundant break pair.
Two cases are intentionally left alone:
Line one<br>line two— the break isn't followed by a trailing break, so it stays two lines.<p><br><br></p>(multi-line spacer) — the first break is followed by another content break, not the trailing break.
display: none (not removal) keeps the <br> in the serialized HTML, so saved content and the published view are unchanged.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a readability regression in the Rhino/ProseMirror-based story editor where legacy empty spacer paragraphs (<p><br></p>) appear as double-height blank lines during editing due to an extra ProseMirror-trailingBreak <br>. The change keeps stored HTML and the published story rendering unchanged by addressing the issue purely in editor-scoped CSS, and adds a system spec to prevent regressions.
Changes:
- Add an editor-scoped CSS rule to hide the redundant
<br>in the specific “empty spacer + trailing break” pattern. - Add a system spec that asserts empty spacer paragraphs render at roughly single-line height in the editor.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
app/frontend/javascript/rhino/custom-editor.css |
Adds a narrowly-scoped selector to collapse the double-height spacer behavior in edit mode without mutating stored content. |
spec/system/stories_spec.rb |
Adds a system-level regression test that measures rendered paragraph heights in the editor to validate the spacing behavior. |
| visit edit_story_path(story) | ||
|
|
||
| expect(page).to have_css(".trix-content p", minimum: 3, wait: 10) | ||
| expect(page).to have_css(".trix-content p", text: "First paragraph.") |
Comment on lines
+168
to
+172
| expect(heights["text"]).to be > 0 | ||
| expect(heights["empty"]).to be > 0 | ||
| # A single blank line ≈ one text line. Before the fix the spacer was two | ||
| # lines tall (a content <br> plus ProseMirror's trailing break). | ||
| expect(heights["empty"]).to be <= heights["text"] * 1.5 |
Comment on lines
+145
to
+149
| story = create( | ||
| :story, | ||
| created_by: user, | ||
| rhino_body: "<p>First paragraph.</p><p><br></p><p>Second paragraph.</p>" | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1521
What is the goal of this PR and why is this important?
How did you approach the change?
<p><br></p>paragraphs. ProseMirror (rhino-editor) parses the lone<br>as a hard break and then appends its ownProseMirror-trailingBreak<br>for editing. The two<br>s render as two blank lines (~56px instead of ~28px), doubling every inter-paragraph gap.custom-editor.cssthat hides the redundant content<br>when it is immediately followed by the trailing break, so spacers collapse to a single blank line.<br>is only hidden, not removed — it stays in the DOM and in the serialized HTML. Verified via a save round-trip that empty spacers and intentional in-paragraph breaks are both preserved byte-for-byte, so stored content and the published view are unchanged. Rewriting<p><br></p>to an empty<p>would have shrunk spacing in the published view on the next save, so it was avoided.br:first-childdirectly followed bybr.ProseMirror-trailingBreak:last-child), so intentional in-paragraph line breaks (Line one<br>line two) and multi-line spacers are left untouched.UI Testing Checklist
<p><br></p>round-trips)Anything else to add?
spec/system/stories_spec.rb— "renders empty spacer paragraphs as a single blank line in the editor". It uses a relative height assertion (empty ≤ 1.5× a single text line) so it is robust to font/line-height differences across environments. Confirmed failing before the fix (empty = 56px vs 42px threshold) and passing after.Before (doubled gaps) and after (single blank line) screenshots from a local published story are available — happy to attach.