refactor(ContentPreview): change from children to render prop pattern (WEBAPP-49784)#4457
Merged
mergify[bot] merged 2 commits intobox:masterfrom Feb 27, 2026
Merged
Conversation
Refactors the custom preview API from children pattern to render prop
pattern for better ergonomics and clearer developer intent.
Changes:
- Replace children?: React.Node with renderCustomPreview
- Update CustomPreviewWrapper to call render function
- Wrap rendered content in fragment to satisfy ErrorBoundary
- Update all 119 tests to use render prop syntax
- Update JSDoc comments and examples
Before (children pattern):
<ContentPreview fileId="123" token={token}>
<MarkdownEditor />
</ContentPreview>
After (render prop pattern):
<ContentPreview
fileId="123"
token={token}
renderCustomPreview={(props) => <MarkdownEditor {...props} />}
/>
Benefits:
- More explicit intent - function signature shows available props
- Better TypeScript/Flow type inference
- More idiomatic React pattern
- Easier conditional rendering and prop transformation
Testing:
- All 119 ContentPreview tests passing
- Flow: 0 errors
- ESLint: clean
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
WalkthroughThis PR replaces the Changes
Sequence Diagram(s)sequenceDiagram
participant User as Client
participant CP as ContentPreview
participant CWrapper as CustomPreviewWrapper
participant BoxPreview as Box.Preview
participant ErrorB as ErrorBoundary
rect rgba(200,200,255,0.5)
User->>CP: mount with renderCustomPreview
CP->>CP: detect renderCustomPreview -> skip Box.Preview init
CP->>CWrapper: render with renderCustomPreview + childProps
CWrapper->>ErrorB: wrap rendered node
ErrorB->>User: render custom content
end
rect rgba(200,255,200,0.5)
User->>CP: mount without renderCustomPreview
CP->>BoxPreview: initialize/load preview
BoxPreview->>CP: preview ready
CP->>User: render default preview
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/elements/content-preview/ContentPreview.js (1)
1275-1283:⚠️ Potential issue | 🔴 CriticalBug:
childrenprop reference not updated torenderCustomPreview.The
onKeyDownmethod still referenceschildrenfrom props (line 1276), but this prop was removed and replaced withrenderCustomPreview. This means hotkey handling will never be skipped for custom preview content, causing potential conflicts with custom component keyboard shortcuts.🐛 Proposed fix
onKeyDown = (event: SyntheticKeyboardEvent<HTMLElement>) => { - const { useHotkeys, children }: Props = this.props; + const { useHotkeys, renderCustomPreview }: Props = this.props; // Skip ContentPreview hotkeys when custom content children are provided to prevent conflicts. // Custom components must implement their own keyboard shortcuts (arrow navigation, etc) // as ContentPreview's default handlers only work with Box.Preview viewer. - if (!useHotkeys || children) { + if (!useHotkeys || renderCustomPreview) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-preview/ContentPreview.js` around lines 1275 - 1283, The onKeyDown handler still checks the removed children prop, so hotkey skipping never occurs for custom previews; update the conditional in onKeyDown (method onKeyDown in ContentPreview.js) to check this.props.renderCustomPreview instead of this.props.children (and update the destructuring where useHotkeys and children are read to useHotkeys and renderCustomPreview) so that when renderCustomPreview is provided the handler returns early and hotkeys are skipped.src/elements/content-preview/__tests__/ContentPreview.test.js (1)
1753-1767:⚠️ Potential issue | 🟡 MinorTest will fail due to bug in ContentPreview.js.
This test expects
onKeyDownto return early whenrenderCustomPreviewis provided, but the production code at line 1276 still checkschildreninstead ofrenderCustomPreview. This test will fail becauseevent.preventDefaultwill be called (the hotkey logic won't skip).This test correctly describes the expected behavior - once the bug in
ContentPreview.jsis fixed (changingchildrentorenderCustomPreviewinonKeyDown), this test should pass.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-preview/__tests__/ContentPreview.test.js` around lines 1753 - 1767, The onKeyDown handler in ContentPreview.js incorrectly checks the presence of children instead of renderCustomPreview, causing hotkey handling to run when a custom preview is provided; update the conditional in the onKeyDown method (ContentPreview.prototype.onKeyDown / function onKeyDown) to check renderCustomPreview (the prop) and return early when it's present so event.preventDefault and event.stopPropagation are not called for custom previews.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/elements/content-preview/__tests__/ContentPreview.test.js`:
- Around line 1753-1767: The onKeyDown handler in ContentPreview.js incorrectly
checks the presence of children instead of renderCustomPreview, causing hotkey
handling to run when a custom preview is provided; update the conditional in the
onKeyDown method (ContentPreview.prototype.onKeyDown / function onKeyDown) to
check renderCustomPreview (the prop) and return early when it's present so
event.preventDefault and event.stopPropagation are not called for custom
previews.
In `@src/elements/content-preview/ContentPreview.js`:
- Around line 1275-1283: The onKeyDown handler still checks the removed children
prop, so hotkey skipping never occurs for custom previews; update the
conditional in onKeyDown (method onKeyDown in ContentPreview.js) to check
this.props.renderCustomPreview instead of this.props.children (and update the
destructuring where useHotkeys and children are read to useHotkeys and
renderCustomPreview) so that when renderCustomPreview is provided the handler
returns early and hotkeys are skipped.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/elements/content-preview/ContentPreview.jssrc/elements/content-preview/CustomPreviewWrapper.jssrc/elements/content-preview/__tests__/ContentPreview.test.js
The onKeyDown handler was still checking the removed children prop, which meant hotkey skipping never occurred for custom previews. Updated the conditional to check renderCustomPreview instead, ensuring custom previews can implement their own keyboard shortcuts without conflicts. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/elements/content-preview/ContentPreview.js`:
- Line 172: The Props change removed support for the old children pattern but
the code and comment are inconsistent—update the ContentPreview component to
either (A) restore a backward-compatibility warning by detecting a passed
children prop (in the ContentPreview function/component) and emitting a dev-only
console.warn or process.env.NODE_ENV check explaining children are deprecated
and suggesting renderCustomPreview, or (B) if you intend this to be a breaking
change, remove the stale comment referencing "Even when children are provided"
and amend the PR description to explicitly state the breaking change; reference
the renderCustomPreview prop and the ContentPreview component when making the
change so the warning or documentation accurately targets consumers of the
previous <ContentPreview><CustomComponent/></ContentPreview> pattern.
shahzadaziz
approved these changes
Feb 27, 2026
Collaborator
shahzadaziz
left a comment
shahzadaziz
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the follow up.
Nefaris
approved these changes
Feb 27, 2026
Contributor
Merge Queue StatusRule:
This pull request spent 6 seconds in the queue, with no time running CI. Required conditions to merge
|
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
3 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
Changes
children?: React.NodewithrenderCustomPreview?: (props: ContentPreviewChildProps) => React.NodeReact.cloneElement()<>) to satisfy ErrorBoundary'sReact.ElementrequirementAPI Comparison
Before (children pattern):