Skip to content

Keep Shift+Enter line breaks in list items across save and reload - #1217

Open
wintan1418 wants to merge 2 commits into
basecamp:mainfrom
wintan1418:fix-list-item-trailing-line-breaks
Open

Keep Shift+Enter line breaks in list items across save and reload#1217
wintan1418 wants to merge 2 commits into
basecamp:mainfrom
wintan1418:fix-list-item-trailing-line-breaks

Conversation

@wintan1418

Copy link
Copy Markdown

Fixes #1004

Line breaks added with Shift+Enter at the end of a list item were silently dropped when a saved document was re-opened in the editor. The editor exports the break as <li>First<br></li>, but Lexical's LineBreakNode.importDOM refuses to convert a <br> that is the last child of a block element — it assumes contenteditable filler — so every save/reload cycle collapsed the spacing between list items.

The fix

  • html.import override for br (importListItemTrailingLineBreak in lexical_helper.js, registered in the editor config next to the existing html.export overrides): a trailing <br> inside a <li> that follows actual content converts to a real LineBreakNode. A <br> that is the item's only child stays subject to Lexical's default filler rules, so empty items keep their canonical <li></li> form.
  • Paste keeps its existing canonicalization: pasted browser HTML really does pad copied list items with a filler <br>, and test/browser/tests/paste/clean_invalid_pasted_list_html.test.js codifies stripping it. PastedContentFormatter now removes li > br:last-child before import, so paste behavior is unchanged — only the load/re-edit path preserves the break.

Tests

test/browser/tests/formatting/list_item_line_breaks.test.js:

  • a Shift+Enter break typed at the end of a list item survives a full value round-trip (save → setValue → compare), reproducing the exact steps from the issue
  • importing <li>First<br></li> and <li>First<br><br></li> keeps the breaks
  • a filler <br> in an empty list item is still dropped

The first three fail on main and pass with this change. Full Playwright suite passes on Chromium (622, including all existing paste-cleanup tests) and the new tests on Firefox; WebKit couldn't launch locally (missing system libraries), so relying on CI for that project. yarn lint and yarn test (126) are clean.

Lexical drops a <br> that ends a block element on import, treating it as
contenteditable filler. Line breaks Lexxy exports at the end of a list
item are real content the user added with Shift+Enter, so re-opening a
saved document silently collapsed the spacing between list items.

Register an html.import override for <br> that keeps a trailing line
break in a list item when it follows actual content. A <br> that is the
item's only child remains subject to the default filler rules.

Pasted content is the one place where a trailing <br> in a list item
really is filler — browsers pad copied list items with one — so
PastedContentFormatter now strips those before import, preserving the
established paste canonicalization behavior.

Fixes basecamp#1004
Copilot AI review requested due to automatic review settings July 30, 2026 18:40

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

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

Fixes loss of Shift+Enter line breaks at the end of list items across save → reload by adding a targeted HTML import override for trailing <br> in <li> while keeping paste canonicalization consistent with existing behavior.

Changes:

  • Add a custom HTML import conversion for <br> that preserves a trailing line break in list items when it follows real content.
  • Strip trailing list-item filler <br> elements during paste formatting so paste output remains canonicalized as before.
  • Add Playwright coverage for list-item trailing line break round-trips and import edge cases.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/browser/tests/formatting/list_item_line_breaks.test.js Adds regression tests covering save/reload round-trips and import cases for trailing <br> in <li>.
src/helpers/lexical_helper.js Introduces the <br> import converter logic for trailing list-item line breaks.
src/elements/editor.js Registers the new <br> import override in the editor’s Lexical config.
src/editor/contents/pasted_content_formatter.js Removes trailing li > br:last-child on paste to preserve existing paste canonicalization behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +109 to +113
function isTrailingLineBreakAfterListItemContent(domNode) {
const parent = domNode.parentElement
return parent !== null && parent.tagName === "LI" &&
domNode.nextSibling === null && domNode.previousSibling !== null
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Right — that check was too loose. It now walks the preceding siblings and only counts a non-whitespace text node or a non-br element as content, so a trailing <br> preceded only by pretty-printing whitespace (or only by other <br>s) stays subject to the default filler rules. Added a regression test for the whitespace case, which failed before the change. Fixed in bd2d24c.

A previous sibling of any kind was treated as content, so a trailing
<br> preceded only by whitespace text (pretty-printed HTML) or another
<br> imported as a real line break in an otherwise empty item. Walk the
preceding siblings and require a non-whitespace text node or a non-<br>
element before keeping the break.
Copilot AI review requested due to automatic review settings July 30, 2026 19:07

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 4 out of 4 changed files in this pull request and generated no new comments.

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.

Shift+Enter line breaks in list items are lost on save/reload

2 participants