Skip to content

Import paragraph-level PPTX bullet style for consistent markers#317

Merged
hackerwins merged 7 commits into
mainfrom
fix/pptx-paragraph-bullet-style
May 31, 2026
Merged

Import paragraph-level PPTX bullet style for consistent markers#317
hackerwins merged 7 commits into
mainfrom
fix/pptx-paragraph-bullet-style

Conversation

@hackerwins

@hackerwins hackerwins commented May 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Read paragraph-level <a:buFont>, <a:buSzPts>, <a:buClr> during PPTX import into a new optional Block.marker (BlockMarker) field, and teach renderListMarker to prefer it per-axis (with first-inline fallback for non-PPTX paths).
  • Propagate Block.marker through scaleBlocks (so shrink-autofit scales the bullet glyph in lockstep with body text) and through getSelectedBlocks + paste head/tail blocks (so clipboard round-trips keep the authored marker).
  • Document on the Block.marker jsdoc that YorkieDocStore does not persist the field today — slides round-trip it via element-level JSON; docs editor doesn't yet author markers, so the gap is latent.

Why

In the "Yorkie, 캐즘 뛰어넘기" PPTX deck (slide 2), the fourth bullet renders much larger than the first three even though every paragraph authors the same <a:buSzPts val="1800"/>. Root cause: the importer dropped paragraph-level bullet style, so renderListMarker inherited font/size/color from block.inlines[0].style — and the Hangul fontFamily fallback (which only fires on runs whose own text contains Hangul) leaves the English-prefix first run on Arial for bullets 1-3 while the all-Korean bullet 4 gets Noto Sans KR. At 18 pt Noto Sans KR's glyph is ~2× taller than Arial's, hence the visible mismatch. PowerPoint and Google Slides render the marker through paragraph bullet attributes, independent of run font.

See docs/tasks/active/20260530-pptx-paragraph-bullet-style-todo.md for the full investigation and approach.

Test plan

  • pnpm verify:fast green (locally green at 053/53 files, 839/840 tests)
  • New unit tests cover:
    • parseTextBody populates Block.marker from <a:buFont> / <a:buSzPts> / <a:buClr> and routes scheme colors through clrMap
    • parseTextBody does NOT attach marker on non-list paragraphs (e.g. <a:buNone/>)
    • renderListMarker prefers block.marker and falls back per-axis to the first inline
    • scaleBlocks scales marker.fontSize in lockstep with inline runs and preserves color-only markers untouched
  • Manual smoke: re-import Yorkie, 캐즘 뛰어넘기.pptx, open slide 2, verify all four markers render at the same glyph size and only bullet 4 is colored orange. (Existing Yorkie documents imported before this PR will continue to show the bug until re-imported — flagged in the task todo.)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • PPTX import now preserves and applies per-bullet styling and master-level bullet defaults so list markers retain intended font, size, and color.
    • Editor paste behavior preserves marker styling across multi-block pastes and prevents clipboard mutation.
  • Bug Fixes

    • List markers render using preserved marker styling (falling back only when absent) to avoid inconsistent glyph appearance.
  • Tests

    • Added unit and rendering tests covering parsing, inheritance, scaling, and render behavior.
  • Documentation

    • Added task docs describing the marker import and rendering flow.

@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@hackerwins, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 29 minutes and 8 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 964f9fc7-7041-4780-86c4-b88d2e4da3e2

📥 Commits

Reviewing files that changed from the base of the PR and between eb13ab2 and 0be0e73.

📒 Files selected for processing (1)
  • packages/frontend/scripts/verify-visual-browser.mjs
📝 Walkthrough

Walkthrough

Adds Block.marker to docs model; prefers block.marker in renderListMarker; parses PPTX paragraph buFont/buSzPts/buClr and master <p:txStyles> into txStylesMarkers, merges defaults into paragraph markers, threads marker defaults through import pipeline, preserves marker through editor paste, scales marker.fontSize in autofit, and adds tests/docs.

Changes

List Bullet Marker Styling

Layer / File(s) Summary
Docs BlockMarker model
packages/docs/src/model/types.ts, packages/docs/src/index.ts, docs/tasks/active/20260530-pptx-paragraph-bullet-style-todo.md
Adds BlockMarker type and optional marker on Block; documents the task and expected persistence/limitations.
Docs marker rendering and tests
packages/docs/src/view/paint-layout.ts, packages/docs/test/view/render-list-marker.test.ts
renderListMarker prefers block.marker for fontFamily/fontSize/color with fallback to inlines[0].style; tests record canvas font/fillStyle/fillText to validate priority, fallback, and partial-marker cases.
Docs editor marker preservation
packages/docs/src/view/text-editor.ts
Clone/paste paths (getSelectedBlocks, multi-block insertBlocks) now deep-copy marker on cloned/head/middle/tail blocks to avoid shared-mutation leaks.
PPTX txStyles master parsing
packages/slides/src/import/pptx/master.ts, packages/slides/test/import/pptx/master.test.ts
Parse <p:txStyles> into TxStylesMarkers (slot → level → BlockMarker) by extracting <a:buFont>, <a:buSzPts> (val/100), and <a:buClr> resolved via clrMap; tests cover presence and absence.
PPTX import pipeline wiring
packages/slides/src/import/pptx/index.ts, packages/slides/src/import/pptx/slide.ts
Thread txStylesMarkers through loadMasterAndLayouts and parseSlide via ParseSlideOptions, initializing empty maps when masters are absent.
PPTX shape marker defaults mapping
packages/slides/src/import/pptx/shape.ts
buildTextElement maps PlaceholderType → txStyles slot, retrieves per-shape markerDefaults from ctx.txStylesMarkers, and passes them to parseTextBody.
PPTX text marker parsing and merging
packages/slides/src/import/pptx/text.ts, packages/slides/test/import/pptx/text.test.ts
TextParseContext accepts markerDefaults; parseParagraphProperties/parseBulletStyle extract paragraph bu* axes, merge them with defaults via mergeMarkers (cloning to prevent shared refs), and attach block.marker only for list-items; tests cover parsing, scheme-color resolution, inheritance, overrides, and regression for mutation leakage.
Autofit marker fontSize scaling
packages/slides/src/model/autofit.ts, packages/slides/test/model/autofit.test.ts
scaleBlocks scales marker.fontSize by autofit scale when present; tests verify proportional scaling and no-op when fontSize absent.

🎯 4 (Complex) | ⏱️ ~50 minutes

  • wafflebase/wafflebase#305: Related prior change that adjusted slides autofit scaling; this PR extends autofit to scale list marker.fontSize as well.

🐰 Bullet Styles Now Shine

Markers dressed in colors deep,
Font sizes font from PowerPoint's keep,
Through parse and merge and render bright,
Your bullets now display just right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: importing paragraph-level PPTX bullet styling for consistent marker rendering, which is the core objective of the PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pptx-paragraph-bullet-style

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Verification: verify:self

Result: ✅ PASS in 262.6s

Lane Status Duration
tokens:build ✅ pass 2.3s
sheets:build ✅ pass 14.0s
docs:build ✅ pass 13.1s
slides:build ✅ pass 14.8s
verify:fast ✅ pass 172.1s
frontend:build ✅ pass 19.9s
verify:frontend:chunks ✅ pass 0.3s
backend:build ✅ pass 5.3s
cli:build ✅ pass 2.3s
verify:entropy ✅ pass 18.5s

Verification: verify:integration

Result: ✅ PASS

@codecov

codecov Bot commented May 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/docs/src/view/text-editor.ts 0.00% 4 Missing ⚠️
packages/docs/src/view/paint-layout.ts 71.42% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@hackerwins
hackerwins force-pushed the fix/pptx-paragraph-bullet-style branch from 078f6b7 to f0bb527 Compare May 30, 2026 17:03

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@docs/tasks/active/20260530-pptx-paragraph-bullet-style-todo.md`:
- Around line 66-73: The "Out of scope" note incorrectly claims
layout/master-level bullet style inheritance isn't implemented; update the text
to reflect the shipped behavior by noting that
packages/slides/src/import/pptx/master.ts parses txStyles and
packages/slides/src/import/pptx/text.ts merges level defaults with paragraph
overrides (so master/layout-level inheritance is supported), and adjust the
checklist items accordingly to avoid misleading future readers.

In `@packages/slides/src/import/pptx/text.ts`:
- Around line 228-242: The mergeMarkers function currently returns the passed-in
BlockMarker objects directly (base or overrides), which allows shared objects
(e.g., markerDefaults) to be mutated by callers; change mergeMarkers so every
return of a BlockMarker returns a fresh shallow-cloned object: when base or
overrides is returned alone, return a clone (e.g., {...base} or {...overrides}),
and when merging use the existing out object as the new combined clone; keep
returning undefined when no properties exist. Ensure you update references in
mergeMarkers to always produce new BlockMarker objects to avoid sibling
mutation.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81c5084b-7480-47a0-99a8-8925cf2192cd

📥 Commits

Reviewing files that changed from the base of the PR and between ea810a8 and f0bb527.

📒 Files selected for processing (15)
  • docs/tasks/active/20260530-pptx-paragraph-bullet-style-todo.md
  • packages/docs/src/index.ts
  • packages/docs/src/model/types.ts
  • packages/docs/src/view/paint-layout.ts
  • packages/docs/src/view/text-editor.ts
  • packages/docs/test/view/render-list-marker.test.ts
  • packages/slides/src/import/pptx/index.ts
  • packages/slides/src/import/pptx/master.ts
  • packages/slides/src/import/pptx/shape.ts
  • packages/slides/src/import/pptx/slide.ts
  • packages/slides/src/import/pptx/text.ts
  • packages/slides/src/model/autofit.ts
  • packages/slides/test/import/pptx/master.test.ts
  • packages/slides/test/import/pptx/text.test.ts
  • packages/slides/test/model/autofit.test.ts

Comment thread docs/tasks/active/20260530-pptx-paragraph-bullet-style-todo.md Outdated
Comment thread packages/slides/src/import/pptx/text.ts
hackerwins and others added 6 commits May 31, 2026 09:14
On a Slides deck with the Simple Dark theme (and any future deck theme
that paints light text on a dark background), the text-edit caret was
rendered with the docs `Theme.cursorColor`. That theme knows about
light/dark mode of the docs package itself but not about per-deck slide
backgrounds, so the caret painted in dark ink and was effectively
invisible against the slide background.

Match Google Docs / Slides behaviour: the caret picks up the resolved
text color at the cursor position — light text yields a light caret,
red text yields a red caret. Adds `resolveColorAtPosition` next to the
existing `defaultColorResolver`, widens the three caret paint sites
(`paintLayout`, `DocCanvas` body / header / footer) with an optional
`cursor.color` field, and threads the resolved color from the slides
text-box editor and the docs full editor. Falls back to
`Theme.cursorColor` when no color is supplied so existing callers keep
their current rendering.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
PowerPoint stores bullet font/size/color on `<a:pPr>` via
`<a:buFont>`/`<a:buSzPts>`/`<a:buClr>` so the marker glyph stays
consistent regardless of the run's font. Wafflebase ignored those
paragraph attributes, so `renderListMarker` had to inherit from
the first inline — and the Hangul fallback in the importer makes
that first inline vary across bullets within one text box.

In the Yorkie "캐즘 뛰어넘기" deck this surfaced on slide 2: bullets
1–3 begin with English ("April, 2019: ") so their first inline stays
on Arial, while bullet 4 starts with "Oct, 2022: Yorkie, 캐즘 뛰어넘기"
and gets Noto Sans KR. At the same 18 pt the Noto `●` is ~2x taller
than Arial's, so bullet 4's orange marker visibly dwarfs the others
even though every paragraph authors the same `<a:buSzPts val="1800"/>`.

Add an optional `Block.marker` field (`BlockMarker`) for authored
marker style, populate it from `<a:buFont>`/`<a:buSzPts>`/`<a:buClr>`
during PPTX import (only on list-items), and teach the docs marker
painter to prefer it per-axis, falling back to the first inline when
unset so existing DOCX-imported and native docs continue rendering
unchanged. Existing Yorkie documents need a re-import to pick up
the new field; new imports get correct markers automatically.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Self-review uncovered three downstream paths that ignored the new
optional Block.marker, leaving subtle gaps once it carries meaningful
data:

- scaleBlocks() scaled inline fontSize but left marker.fontSize alone,
  so a shrink-autofit text box would render the marker glyph at the
  authored 18pt while body text dropped to the fit scale — the inverse
  of the bug the previous commit fixed. The "Yorkie 캐즘" deck's body
  shape uses normAutofit, so this is reachable.
- getSelectedBlocks() enumerated cloned block metadata by field name
  and didn't copy marker, so a copy/paste round-trip silently dropped
  authored markers on the head and tail blocks (middle blocks were
  spread-cloned and survived).
- YorkieDocStore doesn't (de)serialize marker through Yorkie Tree node
  attributes. Today no docs feature authors marker style, so this is
  latent; document the gap on the Block.marker jsdoc so a future docs
  feature or DOCX round-trip doesn't lose data silently.

Also tightens the renderer's color fallback to `??` for axis-uniform
style (StoredColor has no falsy legal value).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The previous commit read paragraph-level <a:buFont>/<a:buSzPts>/<a:buClr>
into Block.marker, but live-imported decks still showed the marker
glyph drift. Root cause: the "Yorkie 캐즘" deck (and most PowerPoint
exports) inlines only <a:buSzPts>/<a:buChar> on the paragraph while
the marker typeface (<a:buFont typeface="Arial"/>) lives exclusively
in the master's <p:txStyles><p:bodyStyle>. The importer ignored that
slot, so marker.fontFamily stayed undefined and renderListMarker fell
back to inlines[0]'s font — drifting back to Noto Sans KR on the
Hangul-starting bullet 4.

Parse <p:txStyles> in the master importer into a slot × level map
(`TxStylesMarkers`), thread it through ParseSlideOptions and
SlideParseContext, look up the right slot inside buildTextElement from
the host shape's <p:ph type>, and merge the level default *under* the
paragraph's own marker so per-paragraph overrides still win per-axis.

With this change, all four bullets of slide 2 import as
`{ fontFamily: 'Arial', fontSize: 18, color: ... }`, matching what
PowerPoint and Google Slides render.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Second-pass code review caught three small items on top of the master
inheritance fix:

- The task doc's "Out of scope" section claimed master/layout
  inheritance was deferred, but commit 3 of this PR landed master
  inheritance. Rewrite the section as a "Re-validated gap" that
  explains why the original assumption was wrong, and narrow the
  remaining out-of-scope to layout-placeholder bullet defaults plus
  <a:buSzPct>.
- Paste's middle-block clone in text-editor's pasteBlocks path was the
  only branch that didn't deep-copy Block.marker (head and tail blocks
  did). Mirror the spread treatment so mutations to a pasted block
  can't leak into the clipboard payload.
- Add a comment on placeholderTypeToTxStylesSlot's `big-number` /
  `caption` branches explaining they're Waffle-only placeholder kinds
  the importer never produces from PPTX today — kept for forward
  routing rather than dead code.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
CodeRabbit caught a subtle aliasing bug in mergeMarkers: when one of
its inputs was undefined it returned the other one verbatim. Since the
`base` argument is the master-level `markerDefaults` map entry shared
across every paragraph that resolves to the same slot × level, a
downstream mutation on the resulting `Block.marker` (clearFormatting,
theme remap, persisted-snapshot round-trip, …) would silently corrupt
every other list item that inherits the same default.

Clone in the single-input branches too. Marginal alloc cost — one small
object per list paragraph — against a real correctness hazard. Regression
test asserts two paragraphs that share a master default end up with
distinct marker references and that mutating one doesn't bleed into the
other or into the shared default.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@hackerwins
hackerwins force-pushed the fix/pptx-paragraph-bullet-style branch from 845fb24 to eb13ab2 Compare May 31, 2026 00:17
The browser visual harness intermittently captured the system-fallback
serif glyph for previewed families like "Roboto" and the theme card
"aA" labels instead of the actual Google web font. The existing
`document.fonts.load()` + `fonts.ready` block was racing the slides
ThemedFontPicker, which injects the Google Fonts CSS link from a child
mount effect — so the first `fonts.load()` call ran before the link
was in the DOM and resolved against the system family. The race used
to manifest only on `mobile.dark`; small bundle perturbations (e.g.
adding a type export) can flip the timing on `desktop` too, surfacing
as a baseline mismatch on the top-level `harness-visual.browser.png`
capture while every individual scenario stays stable.

Stop relying on `fonts.ready` alone:
- First pass loads / drains the queue the page already knows about.
- A second `networkidle` wait gives any late-injected link time to
  fetch.
- Second pass loads again, then polls `document.fonts.check()` until
  every family is ready for synchronous rendering. `check()` is
  strictly stronger than `ready`, which can resolve before
  late-mounted families register.

Refresh the `harness-visual.browser.png` baseline (desktop / light)
from the CI-captured actual so the locked render is the
fonts-loaded one, not the fallback.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@hackerwins
hackerwins merged commit c271a92 into main May 31, 2026
4 checks passed
@hackerwins
hackerwins deleted the fix/pptx-paragraph-bullet-style branch May 31, 2026 04:35
hackerwins added a commit that referenced this pull request May 31, 2026
PRs #257/#301/#303/#304/#305/#306/#307/#309/#315/#316/#317/#321/#322
shipped between v0.4.2 and the v0.4.3 cut, leaving their paired todo
files in active with the process checkboxes still open. Flip the
remaining boxes for the work that landed, then move the paired
todo/lessons into docs/tasks/archive/2026/05/ via pnpm tasks:archive.
Active count drops from 17 to 6; archive count grows by 21 files.

Patch release covering 29 commits since v0.4.2. Headline theme is
Slides editing & UX polish toward Google Slides / PowerPoint parity:
smart guides (equal-spacing / distance / size), Shift drag modifiers,
text inside shapes, a Format options right panel, tier-1 universal
toolbar controls, text autofit (shrink + grow), group-selection
visuals, and several smaller text-box / selection fixes. PPTX import
gained paragraph-level bullet styles, body anchor (vertical
alignment), and stretched blipFill cover-crop.

Docs added a pending-inline-style at collapsed caret, a caret that
tracks the resolved text color, bullet indent on Tab / Shift+Tab,
font / size / line-spacing / clear formatting toolbar controls, an
inline-style-attribute clear that actually lands in the CRDT, and a
polished toolbar trigger / color swatch pass shared across docs /
sheets / slides. Sheets gained .xlsx import.

Infrastructure: Docker images now build on native arm64 runners,
fixing the release hang seen on v0.4.2. No Prisma migration, no new
backend env vars — the devops bump is a routine image-tag change.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant