fix(core): preserve list item type when pasting into empty list items#2722
Merged
Conversation
…tent blocks Fixes #2330 Pasting plain text or a paragraph into an empty bullet/numbered/check list item replaced the list item with a paragraph because BlockNote's serializer wraps content in `blockGroup > blockContainer > paragraph`, producing a closed slice that ProseMirror inserts as a new block rather than splicing inline. `transformPasted` now retypes the leading paragraph in such a slice to match the empty target block, so the list item keeps its type and any trailing blocks become siblings. Also fixes bare `<li>a</li><li>b</li>` HTML parsing: the BulletListItem parse rule requires a `<ul>`/`<ol>` parent, so orphan `<li>`s used to fall back to paragraphs. `nestedListsToBlockNoteStructure` now wraps consecutive orphan `<li>` siblings in a fresh `<ul>` before parsing.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe pull request fixes a paste handling issue where empty list items were incorrectly replaced with paragraphs when pasting content. The fix comprises orphan ChangesEmpty List Item Paste Preservation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/src/api/parsers/html/util/nestedLists.test.ts (1)
146-169: ⚡ Quick winAdd a regression test for non-whitespace separators between orphan
<li>nodes.Current cases miss
<li>a</li>text<li>b</li>, which should not be merged into one wrapped list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/api/parsers/html/util/nestedLists.test.ts` around lines 146 - 169, Add a regression test in nestedLists.test.ts that ensures non-whitespace separators prevent orphan <li> nodes from being merged: create an it(...) (e.g., "Does not merge <li> nodes separated by non-whitespace text") that calls testHTML with the HTML string `<li>a</li>text<li>b</li>` and asserts the output does NOT wrap both <li> elements into one <ul>; use the existing test pattern (the testHTML helper) and mirror surrounding test style so the new case integrates with other cases like "Wraps consecutive bare <li> elements in a <ul>" and "Wraps bare <li>s mixed with other top-level content".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/api/parsers/html/util/nestedLists.ts`:
- Around line 34-38: The grouping loop uses nextElementSibling which skips text
nodes and therefore merges <li> across meaningful text; change the iteration to
use nextSibling (start from orphan.nextSibling) and in the while loop stop if
you encounter a text node with non-whitespace content (nodeType ===
Node.TEXT_NODE && node.textContent.trim() !== '') or any node that is not an LI
element; only treat nodes as continued list items when the node is an Element
with tagName "LI" and orphanSet.has(node as HTMLElement). Update variables
referenced (next, orphan, orphanSet, group, handled) accordingly so grouping
only proceeds across whitespace text nodes but not across meaningful text.
In `@packages/core/src/editor/transformPasted.ts`:
- Around line 198-242: The helper retypeLeadingParagraphForEmptyTarget is called
unconditionally from transformPasted using
getBlockInfoFromSelection(view.state), which is unsafe for drop operations whose
insertion target may differ from the selection; update transformPasted to only
call retypeLeadingParagraphForEmptyTarget for paste flows (check transaction
metadata similar to shouldApplyFix) or add a target-equivalence check before
invoking retypeLeadingParagraphForEmptyTarget (compare drop insertion point
target vs selection-derived target), and add a regression test covering drops
into empty list-item targets to ensure correctness.
---
Nitpick comments:
In `@packages/core/src/api/parsers/html/util/nestedLists.test.ts`:
- Around line 146-169: Add a regression test in nestedLists.test.ts that ensures
non-whitespace separators prevent orphan <li> nodes from being merged: create an
it(...) (e.g., "Does not merge <li> nodes separated by non-whitespace text")
that calls testHTML with the HTML string `<li>a</li>text<li>b</li>` and asserts
the output does NOT wrap both <li> elements into one <ul>; use the existing test
pattern (the testHTML helper) and mirror surrounding test style so the new case
integrates with other cases like "Wraps consecutive bare <li> elements in a
<ul>" and "Wraps bare <li>s mixed with other top-level content".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 421fd044-3488-4c9a-9d23-9a5f085b1b7e
⛔ Files ignored due to path filters (1)
packages/core/src/api/parsers/html/util/__snapshots__/nestedLists.test.ts.snapis excluded by!**/*.snap,!**/__snapshots__/**
📒 Files selected for processing (4)
packages/core/src/api/parsers/html/util/nestedLists.test.tspackages/core/src/api/parsers/html/util/nestedLists.tspackages/core/src/editor/transformPasted.test.tspackages/core/src/editor/transformPasted.ts
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
- nestedLists: walk siblings via nextSibling so meaningful (non-whitespace) text between bare <li>s prevents them from being merged into one <ul>. Whitespace text nodes still bridge consecutive orphans. - transformPasted: bail out of retypeLeadingParagraphForEmptyTarget during drop events (view.dragging is set), since the slice is inserted at the drop point rather than the current selection.
Collaborator
|
My main question regarding this is why not set the
Could you comment on this? |
Contributor
Author
Good question. Your hunch is right that we kind of diverge from prosemirror semantics (because our HTML structure is different) so a Now that I mention it though, maybe I can just have the bullet list item parse match if it is an li without a parent? I'll give that a try |
…tent blocks Match orphan `<li>` (no `<ul>`/`<ol>` ancestor) as a `bulletListItem` in the parse rule, so pasting bare `<li>a</li><li>b</li>` HTML produces two list items instead of falling through to paragraphs. Replaces the previous `wrapOrphanListItems` HTML preprocessing step.
Contributor
Author
|
Yep, that worked better I think! |
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
2 participants
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.
Summary
Fixes #2330. Pasting plain text (or a paragraph) into an empty bullet/numbered/check list item used to replace the list item with a paragraph; it now keeps its type and absorbs the pasted inline content. Also fixes bare
<li>a</li><li>b</li>HTML pastes that previously produced a list item followed by a paragraph instead of two list items.Rationale
BlockNote's
editor.pasteHTMLround-trips through the BlockNote HTML serializer, producing a closed slice (blockGroup > blockContainer > paragraph) that ProseMirror inserts as a new block — walking up the document and replacing the surrounding container. That's surprising when the user is just pasting text into an empty list item. Separately, theBulletListItemparse rule requires a<ul>/<ol>parent, so orphan<li>HTML fell back to paragraph parsing.Changes
packages/core/src/editor/transformPasted.ts—retypeLeadingParagraphForEmptyTargetretypes the slice's leading paragraph to match an empty, non-paragraph, inline-content target block (list items, headings, custom inline-content blocks). Subsequent blocks are kept as-is and become siblings. Non-paragraph leading blocks (heading, list item) keep the existing replace behavior.packages/core/src/api/parsers/html/util/nestedLists.ts— newwrapOrphanListItemsstep wraps consecutive<li>siblings with no<ul>/<ol>ancestor in a fresh<ul>before list-lifting runs.Impact
Scoped: only fires when the target is an empty inline-content block whose type isn't
paragraphand the slice's leading block is aparagraph. Pasting into non-empty blocks, into paragraphs, or pasting non-paragraph leading blocks (heading, list item, table) is unchanged. The orphan-<li>wrap only affects HTML where<li>has no list ancestor.Testing
packages/core/src/editor/transformPasted.test.ts(15 tests) drives the actual paste path viaeditor.pasteHTMLfor: paragraph into empty bullet/numbered/check list items, paragraphs with marks, multi-paragraph paste, heading paste, heading + paragraph paste, list item paste, nested list paste, bare<li>paste, two list items into empty/non-empty list items, and regression cases for non-empty list items and paragraphs.packages/core/src/api/parsers/html/util/nestedLists.test.tscover orphan<li>wrapping and confirm<li>s already inside<ul>are left alone.Checklist
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests