fix(book-pdf): prevent missing CJK text in generated PDFs#1080
fix(book-pdf): prevent missing CJK text in generated PDFs#1080
Conversation
|
@Mowmowj is attempting to deploy a commit to the fkadev Team on Vercel. A member of the Team first needs to authorize it. |
- Update Chinese (zh) font stacks for sans-serif and monospace fonts. - Prefer 'Hiragino Sans GB' and 'Noto Sans CJK SC' for better rendering. - Switch promptFont to use sans-serif font for CJK locales.
📝 WalkthroughWalkthroughAdds locale-aware font stack selection to the PDF generation script and applies those stacks via CSS variables. Multiple heading and prompt elements now use locale-specific Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
📝 Coding Plan
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)
scripts/generate-book-pdf.ts (1)
2208-2211: Consider deriving CJK locales from LOCALE_FONT_STACKS to avoid sync issues.The
isCjkcheck duplicates knowledge of which locales are CJK (already defined as keys inLOCALE_FONT_STACKS). If a new CJK locale is added toLOCALE_FONT_STACKSbut not here, the font selection logic would silently use incorrect heading/prompt fonts.♻️ Optional: Derive isCjk from LOCALE_FONT_STACKS
+const CJK_LOCALES = Object.keys(LOCALE_FONT_STACKS); + function generateHtmlDocument(chapters: ProcessedChapter[], locale: string, messages: Record<string, unknown> = {}): string { // ... - const isCjk = ['zh', 'ja', 'ko'].includes(locale); + const isCjk = CJK_LOCALES.includes(locale);Alternatively, keep the explicit array but add a comment linking the two locations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-book-pdf.ts` around lines 2208 - 2211, The CJK determination duplicates locale knowledge; change the isCjk logic in generate-book-pdf.ts to derive from LOCALE_FONT_STACKS instead of hardcoding ['zh','ja','ko']: use LOCALE_FONT_STACKS (via its keys or a helper) to decide CJK based on the current locale, so getFontStacks(locale) and the headingFont/promptFont assignments (headingFont, promptFont, locale, getFontStacks) stay in sync; alternatively, if you prefer to keep the explicit list, add a clear comment referencing LOCALE_FONT_STACKS to avoid future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/generate-book-pdf.ts`:
- Around line 2208-2211: The CJK determination duplicates locale knowledge;
change the isCjk logic in generate-book-pdf.ts to derive from LOCALE_FONT_STACKS
instead of hardcoding ['zh','ja','ko']: use LOCALE_FONT_STACKS (via its keys or
a helper) to decide CJK based on the current locale, so getFontStacks(locale)
and the headingFont/promptFont assignments (headingFont, promptFont, locale,
getFontStacks) stay in sync; alternatively, if you prefer to keep the explicit
list, add a clear comment referencing LOCALE_FONT_STACKS to avoid future drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9108e5f6-edbd-4c08-b98a-f02c0d524c7f
📒 Files selected for processing (1)
scripts/generate-book-pdf.ts
1 participant
Description
This MR fixes missing CJK(China, Japan, Korea) text in generated book PDFs.
The issue was that some Chinese text rendered correctly in HTML but disappeared in exported PDFs, especially in:
.prompt-codeThe root cause was that the generated print HTML relied on default Latin-oriented font stacks, and
.prompt-codedepended on a monospace stack that is often missing CJK coverage on local systems.This change adds locale-aware font stacks for
zh,ja, andko, routes heading styles through a dedicated heading font, and makes.prompt-codeuse the locale sans stack for CJK locales instead of the monospace stack.The fix is intentionally narrow:
Further Suggestion
This PR proposal has good compatibility but does not use the mono fonts.
If still want to use mono font for the prompt/example blocks styled with
.prompt-code, could install the relevent CJK fonts on the local device.Testing
public/book-pdf/book-zh-print.htmlin Chrome and exported pages14-15提示词工程的发展历程is visibleType of Change
Please don't edit
prompts.csvdirectly!Instead, visit prompts.chat and:
This ensures proper attribution, formatting, and keeps the repository in sync. You'll also appear on the Contributors page!
Additional Notes
Summary by CodeRabbit