fix(preview): East Asian line breaking and measured legend packing - #297
Conversation
CJK clauses arrived as single unbreakable words (whitespace-only tokenization), so the wrapper pushed whole clauses to the next line — e.g. a lone bullet glyph left behind on its own line. CJK runs now break between any two characters with simple kinsoku. Chart legends packed items into fixed-width slots, overlapping long CJK series names; horizontal legends now pack by estimated label width (shrinking to fit), vertical ones size the column to the widest label. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Not ready to approve
The horizontal legend “shrink” logic currently scales layout spacing while keeping swatch size fixed, which can reintroduce overlaps, and the new behaviors need regression tests in the existing preview test suite.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR improves the @office-kit/pptx-preview rendering fidelity for CJK-heavy decks by fixing SVG text wrapping behavior (East Asian line breaking) and making chart legend layout more robust for long series names.
Changes:
- Add CJK-aware tokenization for wrapped text, with simple kinsoku-style punctuation/bracket gluing.
- Rework chart legend packing to use estimated per-label widths, shrink horizontal legend fonts when needed, and size right-side legend columns to the widest label.
- Add a patch changeset describing the user-visible preview fixes.
File summaries
| File | Description |
|---|---|
| packages/preview/src/text-layout.ts | Adds East Asian breakable token splitting to prevent unbreakable CJK “words” from causing bad wraps (e.g., lone bullets). |
| packages/preview/src/render-slide.ts | Replaces fixed-slot legend packing with estimated-width packing + font shrink; improves right-side legend sizing for long labels. |
| .changeset/cjk-wrap-and-legend-packing.md | Patch changeset describing the preview wrapping + legend packing fixes. |
Review details
Suppressed comments (2)
packages/preview/src/text-layout.ts:402
specOf(piece)is re-created repeatedly inside the inner tokenization loops, and this path can get much hotter now that CJK runs split into many per-character tokens. Hoist theFontSpeconce perpieceand reuse it for allmWidthcalls to reduce allocations.
for (const word of piece.text.match(/\s+|\S+/g) ?? []) {
const isSpace = /^\s+$/.test(word);
for (const seg of isSpace ? [word] : splitEastAsianBreakables(word)) {
const w = mWidth(seg, specOf(piece));
if (input.wrap && !isSpace && w > avail - bulletLead && [...seg].length > 1) {
for (const ch of seg) {
packages/preview/src/render-slide.ts:3855
- When the legend row is scaled down, the swatch (9×9) keeps its original size but the swatch→label gap and per-item advance are multiplied by
scale. For smallscalethis can make the text overlap the swatch and/or make swatches overlap each other (because their spacing shrinks but their width doesn’t). Scale the font size (and the label widths) but keep the swatch size and inter-item padding in absolute px.
let cursor = f.x + Math.max(0, (f.w - naturalTotal * scale) / 2);
for (let i = 0; i < names.length; i++) {
out.push(
swatch(i, cursor, rowY - 4),
`<text x="${px(cursor + swatchGapPx * scale)}" y="${px(rowY)}" dominant-baseline="middle" ${effAttrs}>${escapeXml(names[i] ?? `Series ${i + 1}`)}</text>`,
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| if (position === 'b' || position === 't') { | ||
| // Horizontal row centered along the chosen edge. Items are packed by the | ||
| // estimated label width — fixed per-item slots make long (especially CJK) | ||
| // series names spill into the neighbouring slot and overlap. When the row | ||
| // is wider than the frame, shrink the text instead of overlapping. |
| // East Asian text carries no spaces, so a whole CJK clause arrives as one | ||
| // `\S+` "word". Treating it as unbreakable pushes the entire clause to the | ||
| // next line (leaving e.g. a lone bullet glyph behind) — PowerPoint instead | ||
| // breaks East Asian runs between any two characters. Split CJK runs into | ||
| // per-character tokens, gluing closing punctuation to its predecessor and |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
•) on its own line. CJK runs now break between any two characters with simple kinsoku (closing punctuation glued to its predecessor, opening brackets to their successor), matching PowerPoint's East Asian line breaking.min(140px, frameW / n)), so long CJK series names (e.g. 「平均品質スコア(点)」) overflowed into the neighbouring item. Horizontal legends ('b'/'t') now pack items by estimated per-label width and shrink the font when the row exceeds the frame; vertical legends ('r'/'tr') size the right column to the widest label instead of a fixed 100px.Test plan
pnpm vitest run— 311 files / 1310 tests pass🤖 Generated with Claude Code