feat: nested blocks POC#2697
Open
nperez0111 wants to merge 6 commits intomainfrom
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
nperez0111
force-pushed
the
feat/nested-blocks
branch
from
aa70fc1 to
ab86f5b
Compare
📝 WalkthroughWalkthroughThis PR introduces container blocks, a new block type that can host nested child blocks. It adds a complete example demonstrating container block creation, updates core schema and conversion logic to handle container configuration, modifies React rendering to support container-specific handling, and migrates the existing Column block to the new container block infrastructure. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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 |
@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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react/src/schema/ReactBlockSpec.tsx (1)
239-277: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftReact container blocks now lose
domAttributes.blockContent.Once
isContaineris true, these paths return the block component directly and never applythis.blockContentDOMAttributes. In the vanilla container path that data is still available via the render call context, but React renderers are plain FCs, so consumers have no way to preserve editor-level classes/data-* attrs on container blocks.Please thread
blockContentDOMAttributesinto the React container renderer contract, or provide a small container-root helper that applies them before returning the outerNodeViewWrapper. Right now any styling/test hooks configured throughdomAttributes.blockContentdisappear specifically on container blocks.Also applies to: 323-359, 367-399
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/schema/ReactBlockSpec.tsx` around lines 239 - 277, When isContainer is true the code returns the BlockContent directly from the renderToDOMSpec callback and therefore never applies this.blockContentDOMAttributes; update the React container rendering path to accept and apply blockContentDOMAttributes before returning content. Concretely, modify the renderToDOMSpec callback surrounding isContainer/BlockContent so that the container path either (a) wraps the returned BlockContent in a lightweight DOM wrapper that merges this.blockContentDOMAttributes onto the container root, or (b) passes those attributes into BlockContent via a new prop (e.g. blockContentDOMAttributes) and ensure BlockContent consumers apply them to their root; adjust the renderToDOMSpec usage and any BlockContent consumer expectations accordingly (also apply the same change in the other similar blocks around the referenced ranges).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/06-custom-schema/08-container-block/index.html`:
- Line 1: Add the HTML5 doctype declaration before the opening <html> tag in the
file (the top of examples/06-custom-schema/08-container-block/index.html) so the
document starts with <!doctype html> to prevent quirks-mode rendering; simply
insert the doctype line immediately above the existing <html lang="en"> tag.
In `@examples/06-custom-schema/08-container-block/src/Callout.tsx`:
- Around line 62-68: The icon-only toggle button rendered in Callout.tsx (button
with className "callout-icon-button", onClick={cycleFlavor}, title={`Click to
cycle flavor (current: ${flavor.title})`}) lacks an explicit accessible name;
add one by supplying an aria-label (or aria-labelledby) that conveys its purpose
and current state (e.g., include flavor.title) so screen readers get a
consistent name across AT instead of relying solely on title attributes.
In `@examples/06-custom-schema/08-container-block/vite.config.ts`:
- Around line 17-29: The alias paths and the fs.existsSync check are using
two-level up paths ("../../packages/...") which are one directory too shallow
from this file; update the existence check and each path.resolve for the alias
entries (the block that sets "@blocknote/core" and "@blocknote/react") to go up
three levels (e.g., "../../../packages/core/src" and
"../../../packages/react/src") so the fs.existsSync correctly detects the repo
packages and the aliasing works.
In `@packages/core/src/api/nodeConversions/blockToNode.ts`:
- Around line 373-387: The defaultBlocks expansion in blockToNode can recurse
indefinitely for cyclic container defaults; modify blockToNode (or the call path
that maps defaultBlocks) to detect and prevent cycles by tracking a visited/set
or depth for block types (e.g., pass a visitedTypes Set or maxDepth through the
blockToNode call) and skip expanding a default block if its type is already in
the visited set (or depth exceeded); reference blockToNode, getBlockSchema,
containerConfig, defaultBlocks, and effectiveChildren when adding the cycle
guard so seeded defaults do not re-enter types already on the current expansion
stack.
In `@packages/core/src/editor/managers/ExtensionManager/extensions.ts`:
- Around line 70-76: The code assumes every block spec has
spec.implementation.node and reads node.config.group directly; guard that access
by first checking spec.implementation.node exists and has a config (e.g., use an
existence check or optional chaining) before reading config.group so specs
without a node don't throw during setup; update the filter that references
editor.schema.blockSpecs and spec.implementation.node/config.group to skip
entries where node or node.config is missing while still matching the "bnBlock"
group.
In `@packages/xl-multi-column/src/blocks/Columns/index.ts`:
- Around line 51-69: The update handler in the Columns block (the update:
(newNode: ...) function) currently only sets data-id when newNode.attrs.id is
truthy, leaving a stale data-id on the DOM if id is removed; change the logic so
that when newNode.attrs.id is present you set dom.setAttribute("data-id", id)
and when it's absent you call dom.removeAttribute("data-id") — mirror the
existing data-width handling around COLUMN_WIDTH_DEFAULT to ensure the DOM's
data-id always reflects the node's attrs.id.
---
Outside diff comments:
In `@packages/react/src/schema/ReactBlockSpec.tsx`:
- Around line 239-277: When isContainer is true the code returns the
BlockContent directly from the renderToDOMSpec callback and therefore never
applies this.blockContentDOMAttributes; update the React container rendering
path to accept and apply blockContentDOMAttributes before returning content.
Concretely, modify the renderToDOMSpec callback surrounding
isContainer/BlockContent so that the container path either (a) wraps the
returned BlockContent in a lightweight DOM wrapper that merges
this.blockContentDOMAttributes onto the container root, or (b) passes those
attributes into BlockContent via a new prop (e.g. blockContentDOMAttributes) and
ensure BlockContent consumers apply them to their root; adjust the
renderToDOMSpec usage and any BlockContent consumer expectations accordingly
(also apply the same change in the other similar blocks around the referenced
ranges).
🪄 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: 2e4e27fc-0786-4e9f-ac0d-c1095acb02e5
⛔ Files ignored due to path filters (5)
packages/react/src/schema/__snapshots__/ReactBlockSpec.container.test.tsx.snapis excluded by!**/*.snap,!**/__snapshots__/**packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/external.htmlis excluded by!**/__snapshots__/**packages/xl-multi-column/src/test/conversions/__snapshots__/multi-column/undefined/internal.htmlis excluded by!**/__snapshots__/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/src/unit/core/schema/__snapshots__/blocks.jsonis excluded by!**/__snapshots__/**
📒 Files selected for processing (25)
examples/06-custom-schema/08-container-block/.bnexample.jsonexamples/06-custom-schema/08-container-block/README.mdexamples/06-custom-schema/08-container-block/index.htmlexamples/06-custom-schema/08-container-block/main.tsxexamples/06-custom-schema/08-container-block/package.jsonexamples/06-custom-schema/08-container-block/src/App.tsxexamples/06-custom-schema/08-container-block/src/Callout.tsxexamples/06-custom-schema/08-container-block/src/styles.cssexamples/06-custom-schema/08-container-block/tsconfig.jsonexamples/06-custom-schema/08-container-block/vite.config.tspackages/core/src/api/nodeConversions/blockToNode.tspackages/core/src/editor/managers/ExtensionManager/extensions.tspackages/core/src/extensions/TrailingNode/TrailingNode.tspackages/core/src/extensions/tiptap-extensions/UniqueID/UniqueID.tspackages/core/src/schema/blocks/createSpec.tspackages/core/src/schema/blocks/internal.tspackages/core/src/schema/blocks/types.tspackages/react/src/index.tspackages/react/src/schema/ReactBlockSpec.container.test.tsxpackages/react/src/schema/ReactBlockSpec.tsxpackages/xl-multi-column/src/blocks/Columns/index.tspackages/xl-multi-column/src/extensions/ColumnResize/ColumnResizeExtension.tspackages/xl-multi-column/src/pm-nodes/Column.tsplayground/src/examples.gen.tsxtests/nextjs-test-app/package.json
💤 Files with no reviewable changes (1)
- packages/xl-multi-column/src/pm-nodes/Column.ts
| @@ -0,0 +1,14 @@ | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
Add HTML5 doctype at the top.
Line [1] starts with <html> directly. Add <!doctype html> before it to avoid quirks-mode rendering differences.
Suggested fix
+<!doctype html>
<html lang="en">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <html lang="en"> | |
| <!doctype html> | |
| <html lang="en"> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/06-custom-schema/08-container-block/index.html` at line 1, Add the
HTML5 doctype declaration before the opening <html> tag in the file (the top of
examples/06-custom-schema/08-container-block/index.html) so the document starts
with <!doctype html> to prevent quirks-mode rendering; simply insert the doctype
line immediately above the existing <html lang="en"> tag.
Comment on lines
+62
to
+68
| <button | ||
| className={"callout-icon-button"} | ||
| type={"button"} | ||
| contentEditable={false} | ||
| onClick={cycleFlavor} | ||
| title={`Click to cycle flavor (current: ${flavor.title})`} | ||
| > |
There was a problem hiding this comment.
Add an explicit accessible name to the flavor toggle.
This is an icon-only button, so relying on title alone leaves its accessible name inconsistent across assistive tech.
Suggested change
<button
className={"callout-icon-button"}
type={"button"}
contentEditable={false}
onClick={cycleFlavor}
+ aria-label={`Cycle callout flavor (current: ${flavor.title})`}
title={`Click to cycle flavor (current: ${flavor.title})`}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| className={"callout-icon-button"} | |
| type={"button"} | |
| contentEditable={false} | |
| onClick={cycleFlavor} | |
| title={`Click to cycle flavor (current: ${flavor.title})`} | |
| > | |
| <button | |
| className={"callout-icon-button"} | |
| type={"button"} | |
| contentEditable={false} | |
| onClick={cycleFlavor} | |
| aria-label={`Cycle callout flavor (current: ${flavor.title})`} | |
| title={`Click to cycle flavor (current: ${flavor.title})`} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/06-custom-schema/08-container-block/src/Callout.tsx` around lines 62
- 68, The icon-only toggle button rendered in Callout.tsx (button with className
"callout-icon-button", onClick={cycleFlavor}, title={`Click to cycle flavor
(current: ${flavor.title})`}) lacks an explicit accessible name; add one by
supplying an aria-label (or aria-labelledby) that conveys its purpose and
current state (e.g., include flavor.title) so screen readers get a consistent
name across AT instead of relying solely on title attributes.
Comment on lines
+17
to
+29
| !fs.existsSync(path.resolve(__dirname, "../../packages/core/src")) | ||
| ? {} | ||
| : ({ | ||
| // Comment out the lines below to load a built version of blocknote | ||
| // or, keep as is to load live from sources with live reload working | ||
| "@blocknote/core": path.resolve( | ||
| __dirname, | ||
| "../../packages/core/src/" | ||
| ), | ||
| "@blocknote/react": path.resolve( | ||
| __dirname, | ||
| "../../packages/react/src/" | ||
| ), |
There was a problem hiding this comment.
Local alias paths look one directory too shallow.
From this folder, Line [17] / Line [24] / Line [28] should go up three levels to reach repo-root packages/*. With the current paths, the existence check likely returns false and aliasing is skipped.
Suggested fix
- !fs.existsSync(path.resolve(__dirname, "../../packages/core/src"))
+ !fs.existsSync(path.resolve(__dirname, "../../../packages/core/src"))
? {}
: ({
@@
- "../../packages/core/src/"
+ "../../../packages/core/src/"
),
"@blocknote/react": path.resolve(
__dirname,
- "../../packages/react/src/"
+ "../../../packages/react/src/"
),
} as any),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| !fs.existsSync(path.resolve(__dirname, "../../packages/core/src")) | |
| ? {} | |
| : ({ | |
| // Comment out the lines below to load a built version of blocknote | |
| // or, keep as is to load live from sources with live reload working | |
| "@blocknote/core": path.resolve( | |
| __dirname, | |
| "../../packages/core/src/" | |
| ), | |
| "@blocknote/react": path.resolve( | |
| __dirname, | |
| "../../packages/react/src/" | |
| ), | |
| !fs.existsSync(path.resolve(__dirname, "../../../packages/core/src")) | |
| ? {} | |
| : ({ | |
| // Comment out the lines below to load a built version of blocknote | |
| // or, keep as is to load live from sources with live reload working | |
| "@blocknote/core": path.resolve( | |
| __dirname, | |
| "../../../packages/core/src/" | |
| ), | |
| "@blocknote/react": path.resolve( | |
| __dirname, | |
| "../../../packages/react/src/" | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/06-custom-schema/08-container-block/vite.config.ts` around lines 17
- 29, The alias paths and the fs.existsSync check are using two-level up paths
("../../packages/...") which are one directory too shallow from this file;
update the existence check and each path.resolve for the alias entries (the
block that sets "@blocknote/core" and "@blocknote/react") to go up three levels
(e.g., "../../../packages/core/src" and "../../../packages/react/src") so the
fs.existsSync correctly detects the repo packages and the aliasing works.
Comment on lines
+373
to
+387
| // Seed `defaultBlocks` for container blocks when no children would | ||
| // otherwise be present — covers both `block.children === undefined` and | ||
| // `block.children === []` (e.g. converting a leaf block whose | ||
| // `nodeToBlock` produced empty children into a container). | ||
| if (children.length === 0) { | ||
| // `container` is normalized to `ContainerConfig | undefined` at spec | ||
| // registration time (see addNodeAndExtensionsToSpec). | ||
| const containerConfig = getBlockSchema(schema)[block.type] | ||
| ?.container as ContainerConfig | undefined; | ||
| const defaultBlocks = containerConfig?.defaultBlocks; | ||
| if (defaultBlocks && defaultBlocks.length > 0) { | ||
| effectiveChildren = defaultBlocks.map((type) => | ||
| blockToNode({ type } as PartialBlock<any, any, any>, schema, styleSchema), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Guard against recursive defaultBlocks expansion loops.
On Line 384, seeded defaults recurse through blockToNode without cycle protection. A container config like defaultBlocks: ["callout"] (or an indirect cycle) will recurse indefinitely and crash on insert/update.
Suggested fix
-export function blockToNode(
+export function blockToNode(
block: PartialBlock<any, any, any>,
schema: Schema,
styleSchema: StyleSchema = getStyleSchema(schema),
+ defaultSeedPath: string[] = [],
) {
@@
- if (defaultBlocks && defaultBlocks.length > 0) {
- effectiveChildren = defaultBlocks.map((type) =>
- blockToNode({ type } as PartialBlock<any, any, any>, schema, styleSchema),
- );
+ if (defaultBlocks && defaultBlocks.length > 0) {
+ const currentType = block.type;
+ if (currentType && defaultSeedPath.includes(currentType)) {
+ throw new Error(
+ `container defaultBlocks cycle detected: ${[...defaultSeedPath, currentType].join(" -> ")}`,
+ );
+ }
+ const nextPath = currentType
+ ? [...defaultSeedPath, currentType]
+ : defaultSeedPath;
+ effectiveChildren = defaultBlocks.map((type) =>
+ blockToNode(
+ { type } as PartialBlock<any, any, any>,
+ schema,
+ styleSchema,
+ nextPath,
+ ),
+ );
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/api/nodeConversions/blockToNode.ts` around lines 373 - 387,
The defaultBlocks expansion in blockToNode can recurse indefinitely for cyclic
container defaults; modify blockToNode (or the call path that maps
defaultBlocks) to detect and prevent cycles by tracking a visited/set or depth
for block types (e.g., pass a visitedTypes Set or maxDepth through the
blockToNode call) and skip expanding a default block if its type is already in
the visited set (or depth exceeded); reference blockToNode, getBlockSchema,
containerConfig, defaultBlocks, and effectiveChildren when adding the cycle
guard so seeded defaults do not re-enter types already on the current expansion
stack.
Comment on lines
+70
to
+76
| ...Object.entries(editor.schema.blockSpecs) | ||
| .filter(([, spec]) => { | ||
| const group = (spec.implementation.node as Node).config.group; | ||
| return ( | ||
| typeof group === "string" && | ||
| group.split(/\s+/).includes("bnBlock") | ||
| ); |
There was a problem hiding this comment.
Guard access to spec.implementation.node before reading config.group.
Line [72] assumes every block spec has a node implementation. For specs without node, this can throw during editor setup and break initialization.
Suggested fix
...Object.entries(editor.schema.blockSpecs)
.filter(([, spec]) => {
- const group = (spec.implementation.node as Node).config.group;
+ if (!("node" in spec.implementation)) {
+ return false;
+ }
+ const group = (spec.implementation.node as Node).config.group;
return (
typeof group === "string" &&
group.split(/\s+/).includes("bnBlock")
);
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ...Object.entries(editor.schema.blockSpecs) | |
| .filter(([, spec]) => { | |
| const group = (spec.implementation.node as Node).config.group; | |
| return ( | |
| typeof group === "string" && | |
| group.split(/\s+/).includes("bnBlock") | |
| ); | |
| ...Object.entries(editor.schema.blockSpecs) | |
| .filter(([, spec]) => { | |
| if (!("node" in spec.implementation)) { | |
| return false; | |
| } | |
| const group = (spec.implementation.node as Node).config.group; | |
| return ( | |
| typeof group === "string" && | |
| group.split(/\s+/).includes("bnBlock") | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/editor/managers/ExtensionManager/extensions.ts` around
lines 70 - 76, The code assumes every block spec has spec.implementation.node
and reads node.config.group directly; guard that access by first checking
spec.implementation.node exists and has a config (e.g., use an existence check
or optional chaining) before reading config.group so specs without a node don't
throw during setup; update the filter that references editor.schema.blockSpecs
and spec.implementation.node/config.group to skip entries where node or
node.config is missing while still matching the "bnBlock" group.
Comment on lines
+51
to
+69
| update: (newNode: { | ||
| type: { name: string }; | ||
| attrs: { id?: string; width?: number }; | ||
| }) => { | ||
| if (newNode.type.name !== "column") { | ||
| return false; | ||
| } | ||
| const newWidth = newNode.attrs.width ?? COLUMN_WIDTH_DEFAULT; | ||
| dom.style.flexGrow = String(newWidth); | ||
| if (newWidth !== COLUMN_WIDTH_DEFAULT) { | ||
| dom.setAttribute("data-width", String(newWidth)); | ||
| } else { | ||
| dom.removeAttribute("data-width"); | ||
| } | ||
| if (newNode.attrs.id) { | ||
| dom.setAttribute("data-id", newNode.attrs.id); | ||
| } | ||
| return true; | ||
| }, |
There was a problem hiding this comment.
Handle data-id removal to avoid stale DOM identity.
At Line 65, data-id is only updated when truthy; if id becomes absent, the old attribute remains. That can desync DOM↔node identity and break lookup paths that rely on data-id (e.g., column resize node resolution).
Proposed fix
if (newNode.attrs.id) {
dom.setAttribute("data-id", newNode.attrs.id);
+ } else {
+ dom.removeAttribute("data-id");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| update: (newNode: { | |
| type: { name: string }; | |
| attrs: { id?: string; width?: number }; | |
| }) => { | |
| if (newNode.type.name !== "column") { | |
| return false; | |
| } | |
| const newWidth = newNode.attrs.width ?? COLUMN_WIDTH_DEFAULT; | |
| dom.style.flexGrow = String(newWidth); | |
| if (newWidth !== COLUMN_WIDTH_DEFAULT) { | |
| dom.setAttribute("data-width", String(newWidth)); | |
| } else { | |
| dom.removeAttribute("data-width"); | |
| } | |
| if (newNode.attrs.id) { | |
| dom.setAttribute("data-id", newNode.attrs.id); | |
| } | |
| return true; | |
| }, | |
| update: (newNode: { | |
| type: { name: string }; | |
| attrs: { id?: string; width?: number }; | |
| }) => { | |
| if (newNode.type.name !== "column") { | |
| return false; | |
| } | |
| const newWidth = newNode.attrs.width ?? COLUMN_WIDTH_DEFAULT; | |
| dom.style.flexGrow = String(newWidth); | |
| if (newWidth !== COLUMN_WIDTH_DEFAULT) { | |
| dom.setAttribute("data-width", String(newWidth)); | |
| } else { | |
| dom.removeAttribute("data-width"); | |
| } | |
| if (newNode.attrs.id) { | |
| dom.setAttribute("data-id", newNode.attrs.id); | |
| } else { | |
| dom.removeAttribute("data-id"); | |
| } | |
| return true; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/xl-multi-column/src/blocks/Columns/index.ts` around lines 51 - 69,
The update handler in the Columns block (the update: (newNode: ...) function)
currently only sets data-id when newNode.attrs.id is truthy, leaving a stale
data-id on the DOM if id is removed; change the logic so that when
newNode.attrs.id is present you set dom.setAttribute("data-id", id) and when
it's absent you call dom.removeAttribute("data-id") — mirror the existing
data-width handling around COLUMN_WIDTH_DEFAULT to ensure the DOM's data-id
always reflects the node's attrs.id.
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
1 participant
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
This implements an approach we can take nested blocks, by mimicking what we already do for columns and generalizing that API to make it externally consumable.
I gave it the name
containerto sort of give a different name than a "nested block", what we care about is actually the parent block, not the children (which are inter-changeable).So, I felt like "container" better described the parent element than "nested block wrapper" or so. But, of course, always open to names.
This is what the JSON for a "callout", which allows multiple children would look like in code:
And, here is what the equivalent BlockNote JSON:
Rationale
Changes
containerconfig for block creation, which modifies the content type and rendering behavior for containerscontainernodes (we should apply this anyway, strictly more correct)Future
I would likely want to look into 2 more things if we decide to pursure this route:
contentmust be"none"for a container to work properly, would like to consider allowing content for containers tooTesting
Screenshots/Video
Summary by CodeRabbit
New Features
Documentation