fix: set width attribute on image and video elements in editor render#2740
Open
nperez0111 wants to merge 3 commits into
Open
fix: set width attribute on image and video elements in editor render#2740nperez0111 wants to merge 3 commits into
nperez0111 wants to merge 3 commits into
Conversation
When the editor is rendered with editable: false, the <img> tag did not include a width attribute even though the block's previewWidth prop was set — the width was only applied as inline style on the wrapper. This prevented the browser from reserving space before the image loaded (CLS) and made the rendered DOM lossy for downstream consumers reading innerHTML. Apply previewWidth directly to the <img> element in the editor render path, mirroring what imageToExternalHTML and the core Video block already do. CSS width: 100% on .bn-visual-media still controls layout so resize behavior is unaffected. Fixes #2726
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR applies ChangesMedia preview width propagation
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Apply the same fix as the image block to video: - React VideoPreview now sets width on the <video> element so read-only rendering includes it. - React VideoToExternalHTML now serializes previewWidth (it was previously dropped entirely on export). - Core video render path guards `video.width = previewWidth` against undefined (assigning undefined to a numeric DOM property coerces to 0). Audio and File blocks don't have a previewWidth prop, so no change is needed there. Refs #2726
nperez0111
changed the title
- Image snapshots now include the new width attribute on the <img> element when previewWidth is set. - Video snapshots no longer include width="0" since the core video render now guards against an undefined previewWidth. Refs #2726
@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: 1
🤖 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/react/src/blocks/Video/block.tsx`:
- Line 30: The React implementation passes props.block.props.previewWidth
directly into the width attribute causing width="0" to be emitted; update
VideoPreview and VideoToExternalHTML so they only set the width attribute when
props.block.props.previewWidth is truthy (i.e., use a conditional guard like the
core block does) — locate the uses of props.block.props.previewWidth in the
VideoPreview and VideoToExternalHTML components and change them to only assign
width when that value is truthy.
🪄 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: 418a9712-4fa6-4ba6-a89f-067ec85746cb
⛔ Files ignored due to path filters (10)
tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/basic.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/nested.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/noCaption.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/image/noName.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/video.htmlis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/blocknoteHTML/video/withCaption.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactImage/basic.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactImage/nested.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactImage/noCaption.htmlis excluded by!**/__snapshots__/**tests/src/unit/react/formatConversion/export/__snapshots__/blocknoteHTML/reactImage/noName.htmlis excluded by!**/__snapshots__/**
📒 Files selected for processing (2)
packages/core/src/blocks/Video/block.tspackages/react/src/blocks/Video/block.tsx
| : resolved.downloadUrl | ||
| } | ||
| controls={true} | ||
| width={props.block.props.previewWidth} |
There was a problem hiding this comment.
Inconsistent handling of zero width compared to core implementation.
The React implementation passes previewWidth directly to the width attribute, which means React will render width="0" when previewWidth is 0. In contrast, the core implementation (lines 95-97 in packages/core/src/blocks/Video/block.ts) uses a truthiness check that prevents setting width when previewWidth is any falsy value, including 0.
While previewWidth=0 is unlikely in practice, the implementations should handle edge cases consistently.
🔧 Align with core's conditional guard pattern
For VideoPreview (line 30):
<video
className={"bn-visual-media"}
src={
resolved.loadingState === "loading"
? props.block.props.url
: resolved.downloadUrl
}
controls={true}
- width={props.block.props.previewWidth}
+ width={props.block.props.previewWidth || undefined}
contentEditable={false}
draggable={false}
/>For VideoToExternalHTML (line 48):
const video = props.block.props.showPreview ? (
- <video src={props.block.props.url} width={props.block.props.previewWidth} />
+ <video src={props.block.props.url} width={props.block.props.previewWidth || undefined} />
) : (This ensures React won't render the width attribute when previewWidth is falsy (including 0), matching the core behavior.
Also applies to: 48-48
🤖 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/react/src/blocks/Video/block.tsx` at line 30, The React
implementation passes props.block.props.previewWidth directly into the width
attribute causing width="0" to be emitted; update VideoPreview and
VideoToExternalHTML so they only set the width attribute when
props.block.props.previewWidth is truthy (i.e., use a conditional guard like the
core block does) — locate the uses of props.block.props.previewWidth in the
VideoPreview and VideoToExternalHTML components and change them to only assign
width when that value is truthy.
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
When the editor is rendered with
editable: false, the<img>and<video>tags did not include awidthattribute even though the block'spreviewWidthprop was set.Fixes #2726
Rationale
The width was only applied as an inline style on the wrapper
<div>, not on the media element itself. This prevented the browser from reserving space before the media loaded (CLS), and made the rendered DOM lossy for downstream consumers readinginnerHTMLfrom a read-only editor.While auditing, I found the same issue in the video block — plus the React
VideoToExternalHTMLwas droppingpreviewWidthon export entirely. Audio and File blocks don't have a width concept, so they're unaffected.Changes
packages/react/src/blocks/Image/block.tsx: setwidthonImagePreview's<img>.packages/core/src/blocks/Image/block.ts: setimage.width(when defined) inimageRender.packages/react/src/blocks/Video/block.tsx: setwidthonVideoPreview's<video>and also onVideoToExternalHTML(was missing).packages/core/src/blocks/Video/block.ts: guard the existingvideo.width = previewWidthagainst undefined (previously coerced to0).ServerBlockNoteEditorsnapshot to reflect the newwidth="256"attribute.Impact
CSS
width: 100%on.bn-visual-mediastill controls layout, so resize behavior in editable mode is unaffected — the HTML attribute is purely additive and advertises the intrinsic width to read-only consumers.Testing
pnpm --filter @blocknote/server-util testpasses (snapshot updated).pnpm --filter @blocknote/react testpasses.pnpm --filter @blocknote/core testpasses (an unrelated numbered-list performance test is flaky on this machine but is not touched by this change).Summary by CodeRabbit