fix(markdown): skip placeholder text for empty files (#434)#2719
fix(markdown): skip placeholder text for empty files (#434)#2719nperez0111 merged 1 commit intomainfrom
Conversation
Empty image/video/audio/file blocks (no url set) previously leaked their "Add image"/"Add video"/etc. UI text into HTML and markdown exports. Each block's toExternalHTML now emits the bare element (<img>, <video>, <audio>, <embed>) with no src, so the placeholder text is gone from exports while the block still round-trips back to an empty block on re-import via the existing tag-name parsers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR changes media blocks (Image, Video, Audio, File) and the markdown exporter to handle missing source URLs by returning empty elements instead of placeholder text, and adds embed element support to the markdown serialization pipeline. ChangesMedia Block Fallbacks & Markdown Embed Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 |
@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: |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/core/src/blocks/Image/block.ts (1)
143-147: ⚡ Quick winAdd
alt=""to the empty placeholder<img>for a11y/HTML conformance.A bare
<img>with neithersrcnoraltis flagged by HTML validators and is treated by assistive technologies as an image of unknown content. Setting an emptyaltmarks it as decorative and is the standard way to express "no meaningful image":♻️ Proposed fix
if (!block.props.url) { + const placeholder = document.createElement("img"); + placeholder.alt = ""; return { - dom: document.createElement("img"), + dom: placeholder, }; }🤖 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/blocks/Image/block.ts` around lines 143 - 147, The placeholder image returned when block.props.url is falsy currently creates an <img> without alt text; update the fallback in the Image block (the branch checking block.props.url in block.ts) to return an <img> element with alt="" (i.e., set the element's alt attribute to an empty string) so the placeholder is marked decorative and conforms to a11y/HTML validation.packages/core/src/blocks/File/block.ts (1)
76-81: 💤 Low valueBare
<embed>works for round-trip but may render a small visual artifact.
<embed>withoutsrcis well-suited for the round-trip path (fileParsealready handlesEMBED), so functionally this is fine. Note that some browsers render a small empty plugin region for<embed>lacking a meaningfulsrc, unlike<img>/<video>/<audio>which collapse cleanly. If any downstream HTML consumers are strict about validation or visible artifacts, you could optionally add a hidden marker (e.g.,style="display:none"or adata-blocknote-emptyattribute) to make intent explicit. Not a blocker — current behavior already meets the issue#434objective of removing the placeholder text.🤖 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/blocks/File/block.ts` around lines 76 - 81, The toExternalHTML function currently returns an <embed> element with no src when block.props.url is missing, which can render a small empty plugin region in some browsers; update toExternalHTML (in the File block) so that when !block.props.url you still create the embed DOM but add a hidden marker (e.g., set style.display="none" or set a data attribute like data-blocknote-empty="true") to avoid visible artifacts and make the intent explicit for downstream consumers.packages/core/src/api/exporters/markdown/htmlToMarkdown.ts (1)
520-531: 💤 Low valueOptional: deduplicate
serializeAudioandserializeEmbed.The two functions are now byte-identical (empty-src →
"\n\n", otherwisectx.indent + \\n\n``). You could collapse them into a shared helper to make future changes (e.g., adding alt-text-style handling) consistent across both block types.♻️ Possible refactor
-function serializeAudio(el: HTMLElement, ctx: SerializeContext): string { - const src = el.getAttribute("src") || ""; - if (!src) {return "\n\n";} - // Audio has no visible representation in markdown; output as link with empty text - return ctx.indent + `[](${src})\n\n`; -} - -function serializeEmbed(el: HTMLElement, ctx: SerializeContext): string { - const src = el.getAttribute("src") || ""; - if (!src) {return "\n\n";} - return ctx.indent + `[](${src})\n\n`; -} +function serializeSrcAsBlockLink( + el: HTMLElement, + ctx: SerializeContext, +): string { + const src = el.getAttribute("src") || ""; + if (!src) {return "\n\n";} + // No visible markdown representation; emit empty-text link. + return ctx.indent + `[](${src})\n\n`; +}🤖 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/exporters/markdown/htmlToMarkdown.ts` around lines 520 - 531, serializeAudio and serializeEmbed are identical; extract the shared behavior into a helper (e.g., serializeSrcAsLink or formatMediaAsLink) that takes an HTMLElement and SerializeContext, returns "\n\n" when src is empty else ctx.indent + `[](${src})\n\n`, then update serializeAudio and serializeEmbed to call that helper so future changes (like alt-text handling) are applied in one place; reference functions serializeAudio, serializeEmbed, and the new helper name in your change.
🤖 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.
Nitpick comments:
In `@packages/core/src/api/exporters/markdown/htmlToMarkdown.ts`:
- Around line 520-531: serializeAudio and serializeEmbed are identical; extract
the shared behavior into a helper (e.g., serializeSrcAsLink or
formatMediaAsLink) that takes an HTMLElement and SerializeContext, returns
"\n\n" when src is empty else ctx.indent + `[](${src})\n\n`, then update
serializeAudio and serializeEmbed to call that helper so future changes (like
alt-text handling) are applied in one place; reference functions serializeAudio,
serializeEmbed, and the new helper name in your change.
In `@packages/core/src/blocks/File/block.ts`:
- Around line 76-81: The toExternalHTML function currently returns an <embed>
element with no src when block.props.url is missing, which can render a small
empty plugin region in some browsers; update toExternalHTML (in the File block)
so that when !block.props.url you still create the embed DOM but add a hidden
marker (e.g., set style.display="none" or set a data attribute like
data-blocknote-empty="true") to avoid visible artifacts and make the intent
explicit for downstream consumers.
In `@packages/core/src/blocks/Image/block.ts`:
- Around line 143-147: The placeholder image returned when block.props.url is
falsy currently creates an <img> without alt text; update the fallback in the
Image block (the branch checking block.props.url in block.ts) to return an <img>
element with alt="" (i.e., set the element's alt attribute to an empty string)
so the placeholder is marked decorative and conforms to a11y/HTML validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe6d04d7-e1e5-4ee7-9a44-6e17b1d4d348
⛔ Files ignored due to path filters (9)
tests/src/unit/core/clipboard/copy/__snapshots__/text/html/basicBlocks.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/html/audio/button.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/html/file/button.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/html/image/button.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/markdown/audio/button.mdis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/markdown/file/button.mdis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/markdown/image/button.mdis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/html/reactFile/button.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/html/reactImage/button.htmlis excluded by!**/__snapshots__/**
📒 Files selected for processing (5)
packages/core/src/api/exporters/markdown/htmlToMarkdown.tspackages/core/src/blocks/Audio/block.tspackages/core/src/blocks/File/block.tspackages/core/src/blocks/Image/block.tspackages/core/src/blocks/Video/block.ts
Summary
Empty image/video/audio/file blocks (no url set) no longer leak their "Add image"/"Add video"/etc. UI text into HTML and markdown exports — fixes #434.
Rationale
When a user inserts a media block but hasn't selected a file yet, the block renders an "Add image" placeholder UI in the editor. Previously that placeholder text was being passed through to the external HTML / markdown exporters as a literal
<p>Add image</p>paragraph, which is a confusing artifact in any exported document.Changes
toExternalHTML(Image, Video, Audio, File) now emits the bare element (<img>,<video>,<audio>,<embed>) with nosrcattribute whenprops.urlis empty, instead of a paragraph containing the UI placeholder text.htmlToMarkdownno longer emits the stale "Add image" / "Add video" fallback strings; empty media now emits\n\nto preserve block-level spacing consistent with other empty-content serializers (paragraph, heading), and a newserializeEmbedcase handles file placeholders.Impact
The editor UI is unaffected — the "Add image" / "Add file" buttons are rendered by
render(viacreateAddFileButton), nottoExternalHTML, so users still see the upload UI as before. Round-trip works because the existing tag-name parsers (imageParse,videoParse,audioParse,fileParse) already handle emptysrcby returningurl: undefined, so an empty block exported to HTML re-imports as the same empty block. Markdown round-trip cannot preserve empty placeholder blocks (markdown has no syntax for them), but the placeholder text no longer appears.Testing
All 842 existing format-conversion tests pass; the
image/button,video/button,audio/button, andfile/buttonsnapshots were updated and verify the new<img />/<video></video>/<audio></audio>/<embed />output.Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes