fix: Code block PDF export (BLO-987)#2725
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe code block PDF exporter now extracts text by concatenating all styled text items from block content instead of selecting a single item, and adjusts the container styling from dark background with white text to a solid black border. ChangesCode Block PDF Rendering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/xl-pdf-exporter/src/pdf/defaultSchema/blocks.tsx (1)
120-122: ⚡ Quick winUnsafe cast may silently produce
"undefined"in output.
(block.content as StyledText<any>[]).map((item) => item.text)— ifblock.contentever contains a non-StyledTextitem (e.g., aLink, which has no top-level.text), thenitem.textisundefined, and.join("")silently emits the string"undefined"into the exported code block text.The comment acknowledges code blocks should only contain
StyledText, but the cast provides no runtime guard.🛡️ Proposed defensive fix
- const textContent = (block.content as StyledText<any>[]) - .map((item) => item.text) - .join(""); + const textContent = (block.content as StyledText<any>[]) + .filter((item) => item.type === "text") + .map((item) => item.text) + .join("");🤖 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/xl-pdf-exporter/src/pdf/defaultSchema/blocks.tsx` around lines 120 - 122, The current unsafe cast of block.content to StyledText<any>[] may produce "undefined" if non-StyledText items (e.g., Link) appear; update the computation of textContent to guard at runtime by treating block.content as an array of unknown, filter or flatMap items that actually have a string .text property (using a type guard or typeof check) before mapping and joining, and ensure non-text items are skipped or replaced with a safe fallback (empty string) so textContent never contains "undefined" while keeping references to block.content and StyledText intact.
🤖 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/xl-pdf-exporter/src/pdf/defaultSchema/blocks.tsx`:
- Around line 120-122: The current unsafe cast of block.content to
StyledText<any>[] may produce "undefined" if non-StyledText items (e.g., Link)
appear; update the computation of textContent to guard at runtime by treating
block.content as an array of unknown, filter or flatMap items that actually have
a string .text property (using a type guard or typeof check) before mapping and
joining, and ensure non-text items are skipped or replaced with a safe fallback
(empty string) so textContent never contains "undefined" while keeping
references to block.content and StyledText intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f1d7e4d0-8ec5-47b5-a6cf-c00bdc69a5b0
📒 Files selected for processing (1)
packages/xl-pdf-exporter/src/pdf/defaultSchema/blocks.tsx
nperez0111
left a comment
nperez0111
left a comment
There was a problem hiding this comment.
Thanks @matthewlipski
2 participants
Summary
This PR mainly changes the styling of code blocks when exported to PDF, replacing the black background for just a black border. The idea of this is to make it more suitable for print, which is a fairly common use case of exporting PDFs.
Additionally, it fixes an issue referenced in #2324, where only the first content with matching syntax highlighting is exported. The root cause of said issue is uncertain as syntax highlighting is applied via decorations, so I have no idea how it would ever show up in the editor content, but the fix should work all the same.
Closes #2324
Rationale
See above.
Changes
StyledTextinline content get merged into a single string before exporting code block to PDF, instead of only the firstStyledTexttext string being used.Impact
N/A
Testing
N/A
Screenshots/Video
N/A
Checklist
Additional Notes
N/A
Summary by CodeRabbit
Bug Fixes
Style