Repository navigation
Conversation
… for block plumbing Replaces the BlockInfo union's isBlockContainer/childContainer/blockContent shape with block/content/children, and annotates it with the facts callers kept re-deriving by hand: contentStart/contentEnd, childrenStart/ childrenEnd, contentKind (read off the spec config stored on the node), and isContentEmpty. The +1/-1 position arithmetic around content edges, tables and child ranges moves into blockEdgePos/blockEdgeSelection/ tableContentCaretPos and the ChildrenInfo fields. The six producers collapse to four named by the input you already have: getBlockInfoFromNode, getBlockInfoAt, getBlockInfoNearPos, getBlockInfoFromSelection. Block navigation (parent/prev/next/last- descendant) joins them here instead of living beside the merge command. All block manipulation (insert/move/nest/replace/split/update, selections, clipboard, serialization, conversions, keyboard shortcuts) is rewired onto the new vocabulary. insertBlocks gains "first-child"/"last-child" placements resolved through getInsertionPos, shared with the move commands so "can this block go here?" has one schema-driven answer; hand-written nodes are checked against their declared content kind when the schema is built (checkNodeMatchesConfig).
Resolve block shape directly, validate wrapper structure at the BlockInfo boundary, and convert content from the established content kind. Move insertion resolution into BlockInfo and use node bounds for last-descendant navigation, updating callers and regression coverage.
Three follow-ups on the BlockInfo refactor, all behaviour-preserving. `tableContentCaretPos` is no longer exported: both callers checked `contentKind === "table"` themselves and then called it, duplicating the branch `blockEdgePos` already makes. They now ask `blockEdgePos` for the edge, so the table offset lives in one place instead of three. The keyboard shortcuts computed a block's content edges by hand in twelve places (`content.beforePos + 1` / `content.afterPos - 1`) rather than reading `contentStart` / `contentEnd`, which the refactor added for exactly that. Replacing them left `content` unused in five destructures. `getInsertionPos`'s lazy-blockGroup branch says why only a regular block reaches it, so `wrapIn: blockGroup` reads as implied rather than assumed, and why the `hasContent` check is there to narrow the union.
|
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 configuration
📒 Files selected for processing (46)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughBlock metadata now represents both content-bearing blocks and child containers. Block insertion supports child placements with schema validation. Editing commands, keyboard handlers, selection helpers, and integrations use the updated metadata and position APIs. ChangesBlock operations
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant Caller
participant BlockManager
participant InsertBlocks
participant Schema
Caller->>BlockManager: insertBlocks with placement
BlockManager->>InsertBlocks: forward blocks and placement
InsertBlocks->>Schema: resolve valid insertion position
Schema-->>InsertBlocks: position and optional wrapper
InsertBlocks-->>Caller: inserted blocks
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This update refactors how keyboard shortcuts calculate caret positions inside blocks and adds tests that pin down the current behavior. No concrete user-facing defect was found, so the change looks ready to merge. One caveat: the block-info helpers were renamed and reshaped. If outside code relies on the old helper names, consider noting this in the release notes. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)✅ Passed checks (4 passed)Full details: Docstring CoverageExplanation Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 70 functions across 45 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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. A rabbit checks each block in line Comment |
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/diagram-block
@blocknote/mantine
@blocknote/math-block
@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
@blocknote/xl-typst-exporter
commit: |
|
Three follow-ups on top of #3051, all behaviour-preserving. Based on that branch, so the diff is just these changes.
1.
tableContentCaretPosno longer escapes the moduleBoth callers in
KeyboardShortcutsExtensioncheckedcontentKind === "table"themselves and then called the table helper — re-making the decisionblockEdgePosalready makes:All three arms are
blockEdgePos, whose non-table path returns exactlycontentStart/contentEnd. Each site is now one call plus thenullcase — content with no caret, i.e. an image — which is the only thing the helper can't decide for you. The+4table offset is back to living in one place instead of three, and the function is module-private again.2. The keyboard shortcuts use
contentStart/contentEndBlockInfogained those fields in #3051 precisely so callers stop writing the arithmetic, but this file still did it by hand in twelve places (content.beforePos + 1,content.afterPos - 1) — every one a "is the caret at the start/end of this block's content?" check.Replacing them turned out to make
contentunused in five of the destructures, so those shrank too:3. A comment on
getInsertionPos's lazy-blockGroupbranchReading
if (!info.children)it isn't obvious why hardcodingwrapIn: blockGroupis safe, or whyhasContentis tested when it can't be false. Both follow from theBlockInfounion — the container arm makeschildrenrequired, so only a regular block reaches that branch, and thehasContenttest is there to narrow the union socontentcan be read. The comment says so.Verification
Behaviour-preserving, checked against both
mainand #3051 with two differential harnesses — identical scenarios run on each branch and the outputs diffed:Plus core 796, multi-column 82, tests 908, type-aware lint clean.
Net −56 / +39 across the two files.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes