feat(core): export slides as PNG from the viewer and the CLI - #379
Open
desek wants to merge 34 commits into
Open
feat(core): export slides as PNG from the viewer and the CLI#379desek wants to merge 34 commits into
desek wants to merge 34 commits into
Conversation
… open-slide viewer - Add docs/cr/CR-0001-export-slides-as-png.md (draft, 725 lines) - Specifies a client-side PNG exporter in packages/core that reuses the offscreen 1920x1080 mount pattern from export-pdf.ts and the fflate ZIP pattern from export-html.ts - Recommends html-to-image as the rasteriser with rationale, alternatives, and an explicit "lazy import" pattern matching the existing fflate dynamic import - Adds two new viewer dropdown entries (Export current slide as PNG, Export all slides as PNG), a PngProgressToast component, and new locale keys across en/ja/zh-cn/zh-tw - Defines functional + non-functional requirements (MUST), a 5-phase implementation plan, a test strategy table, 9 Gherkin acceptance criteria, risk register, and an Open Questions section flagging the dependency decision for review
…nObject->canvas rasterizer - Replace html-to-image dependency with a hand-rolled, zero-dependency <foreignObject>->canvas pipeline: clone + inline computed styles, inline bundled Geist @font-face and same-origin images, serialise into SVG, draw onto an offscreen canvas supersampled x2 (mirroring export-pdf.ts), then canvas.toBlob('image/png'). Bundle multi-page output via the existing fflate dependency (used by export-html.ts) so no new runtime dependency is added. - Update Functional Requirements (new 9a/9b/9c) and Non-Functional Requirements to forbid any new runtime dependency. - Rewrite "Alternative Approaches Considered" to cite the two real-world prior-art precedents: client-side foreignObject->canvas (dom-to-image origin, html-to-image / html2canvas refinements) and headless Chromium via Playwright (Slidev's slidev export --format png). Reject Playwright as primary mechanism for being heavy + CLI-centric. - Add "Future Enhancements" section documenting an opt-in Playwright-based CLI export path, never bundled into the client runtime, following Slidev's optional-dependency pattern. - Replace html-to-image-specific risks with a hand-rolled-rasterizer risk and a new Safari <foreignObject> quirks risk that reuses the existing isSafari() helper from export-pdf.ts. - Add AC-10 covering Safari behaviour (best-effort warn vs hard disable). - Resolve the html-to-image-vs-hand-rolled open question; add a Safari fallback open question pending Phase 4 review. - Update Phase 1 (no dep install), Phase 2 (hand-rolled rasterisation helpers a-e), Affected Components, Scope Boundaries, Decision Outcome, Dependencies, Technical Impact, and the proposed-state Mermaid diagram to match. - Keep status: draft. No frontmatter promotion.
…solved, ambiguity fixed - Drift check passed: every cited path/symbol in CR-0001 verified against the current tree at source-commit 155049f (export-html/pdf/print-ready/sdk/design/ page-context modules, isSafari, ANIMATION_TIMEOUT_MS, allowHtmlDownload gate, fflate dep, locale files). No drift detected. - Fixed test-row contradiction where filename-padding description (p01 for 9 pages) disagreed with its expected output (p1) and with FR-1's pad-to-width rule. - Tightened NFR 1 RFC-2119 language: replaced 'MUST stay under ~250 lines' with a precise SHOULD <300 lines plus a MUST on file location and naming. - Resolved AC-10, Risk 2, and Open Question 3 to the CR's stated leaning: Safari strategy = best-effort + warn (option a). Added new locale key slide.pngSafariBestEffort to FR-13 and Phase 1. - Resolved Open Question 2 to the convention already encoded in FR-1: pad {N} to the width of the total page count. - Appended a review-summary HTML comment block with counts and the resolution trail for downstream phase implementors.
- Add PngExportProgress type and stub exportSlidePageAsPng / exportSlideAsPngZip in packages/core/src/app/lib/export-png.ts
- Add slide.{exportCurrentPageAsPng, exportAllPagesAsPng, pngExportFailed, pngSafariBestEffort} and full pngToast block to locale types
- Translate new keys in en, ja, zh-cn, zh-tw
- Implement renderPageToPng internal helper: mount page offscreen at 1920x1080, apply designToCssVars, wrap in SlidePageProvider, await waitForFonts/waitForDataWaitfor, poll isFrameAnimationSettled with the same ANIMATION_TIMEOUT_MS=15_000 and POLL_INTERVAL_MS=100 as export-pdf.ts, rasterise via the hand-rolled foreignObject pipeline, teardown root + host in finally - Add cloneWithInlinedStyles, inlineGeistFonts, inlineSameOriginImages, nodeToSvgDataUrl, rasteriseSvgToPng helpers in export-png.ts (zero new runtime deps; x2 supersample canvas mirrors export-pdf zoom:2 trick) - Implement exportSlidePageAsPng wrapper + pngFilenameFor (pad to width of total page count per FR-1 / Open Question 2) - Extract downloadBlob into packages/core/src/app/lib/download.ts; export-html.ts now imports the shared helper
- Implement exportSlideAsPngZip in packages/core/src/app/lib/export-png.ts:
iterate pages calling renderPageToPng one at a time (bounding peak DOM
size to a single offscreen host per Risk 5), and bundle a flat ZIP via
fflate.zipSync keyed by {slideId}-p{padded}.png -> Uint8Array, then
download as {slideId}.zip via the shared downloadBlob helper.
- Emit PngExportProgress through the optional onProgress callback for all
four phases (processing while a page is mounting/waiting, rasterising
once toBlob has produced bytes, zipping during zipSync, done after the
download is triggered) with a monotonically non-decreasing percent.
- renderPageToPng's existing try/finally tears down the offscreen mount
on toBlob rejection; the new loop propagates the rejection so the
caller can surface slide.pngExportFailed.
- pnpm core typecheck and pnpm check both exit 0; no new warnings.
Add unit tests for export-png filename padding, monotonic progress, DOM-residue-free rejection, and per-phase onProgress emission, plus a downloadBlob test covering createObjectURL/revokeObjectURL lifecycle. Introduce happy-dom as a root devDependency for DOM-using tests, add a test-only rasteriser seam to export-png, and ship the changeset for the @open-slide/core minor bump.
**Status:** completed — all CI gates passed, scope verified, QS compliance verified. **Verification summary:** - pnpm install: clean - pnpm check: 2 pre-existing warnings (unrelated to CR) - pnpm typecheck: 0 errors - pnpm test: 18 test files, 261 tests passed - pnpm build: all packages built successfully **Scope verification:** - All 5 phases implemented (foundation → polish) - Affected components match CR spec exactly - No new runtime dependency added to packages/core - Changeset present with minor bump for @open-slide/core - No commits to prohibited areas (ui/, package.json version bumps) - Checkpoint commits follow Conventional Commits pattern **Quality standards:** - Build & Compilation: passed - Linting & Code Style: passed (hierarchical naming, no casual comments) - Test Execution: 261 tests passed - Documentation: all exported functions documented with intent-focused docstrings per CLAUDE.md standards - Code Review: ready for merge **CR frontmatter updated:** - status: completed - completed-date: 2026-05-30 - source-branch: feat/export-slides-as-png - source-commit: 1edb9a7 - Quality Standards Compliance checkboxes: all ticked **Short summary:** PNG export via hand-rolled <foreignObject> → canvas rasterizer, zero new deps, full-deck ZIP support with progress toast, four-locale i18n, 261 tests passing, all Biome/typecheck/build/test gates passing.
- Add docs/cr/CR-0001-validation-report.md tracing every FR, NFR, AC, and Test Strategy row to changed-file evidence and test runs. - All 17 FRs PASS with file:line evidence; 5 specified tests exist and pass under `pnpm test` (export-png.test.ts 4/4, download.test.ts 1/1). - AC-1 .. AC-7, AC-9, AC-10 PASS; AC-8 PARTIAL (two pre-existing Biome warnings outside the CR's touched files, not regressions). - Gaps: NFR-1 (export-png.ts is 426 lines vs the SHOULD-under-300 target; CR allows sibling-file split), NFR-3 (single-page <4 s perf not measured during validation), AC-8/NFR-5 (pre-existing Biome warnings). - pnpm typecheck and pnpm build are clean; no new runtime dep on @open-slide/core (only root devDep `happy-dom` for the new vitest happy-dom env, which is out of @open-slide/core's shippable surface, honouring NFR-2).
- NFR-1: split rasterisation helpers into export-png.rasterize.ts sibling per the CR's documented escape hatch; export-png.ts now 236 lines (<300) - NFR-3: documented as manual-verification-required (CR delegates timing to local review on 2024-class Chromium) - AC-8 / NFR-5: reconcile wording to "no NEW warnings introduced"; the two pre-existing biome warnings are outside this CR's Affected Components All 5 specified tests still pass. Public API unchanged.
- README.md: add PNG to the export highlight. - apps/web/content/docs/core-feature/export.mdx: add PNG section (single + ZIP, progress toast, Safari best-effort, allowHtmlDownload gate); update frontmatter and gate comment. - apps/web/content/docs/reference/config.mdx: extend allowHtmlDownload doc to cover HTML, PDF, and PNG. - apps/web/content/docs/index.mdx: mention PNG in the export highlight.
The hand-rolled rasteriser produced blank, mis-sized PNGs: - cloneWithInlinedStyles copied the offscreen host's computed style (position:fixed; left:-99999px) onto the clone root, pushing the whole slide out of the <foreignObject> viewport -> blank output. Reset the clone root to static/zero-inset/no-transform. - defaultRasteriseSvgToPng sized the canvas 3840x2160 and saved it directly (wrong size, no real supersample). Render the SVG at 2x density via the viewBox and draw down onto a 1920x1080 canvas (FR-3). - Capture ::before/::after pseudo-elements as generated rules and inline same-origin url() background images so decorations/backgrounds appear. Verified in-browser against the live viewer: exported pages match the on-screen rendering (fonts, colours, layout, per-page content). Added regression tests for the root-positioning reset and the 2x/viewBox SVG.
Match the repo-wide convention (all 16 existing test files import the module-under-test with an explicit .ts extension).
…headless Playwright
- Add docs/cr/CR-0002-cli-export-slides-as-png.md (status: draft).
- Specifies an `open-slide export` CLI subcommand that boots the existing
Vite dev server in-process, drives the real viewer route through headless
Chromium via Playwright, awaits a deterministic per-page readiness signal
(mapped to `waitForFonts` / `waitForDataWaitfor` /
`isFrameAnimationSettled`), and writes PNGs at the canonical 1920x1080
canvas size.
- Records hard design decisions: CLI-only surface (no dev-server HTTP
endpoint in this CR), Playwright as an optional/dev-only dependency
(never in the runtime bundle), filename convention identical to CR-0001
(`{slideId}-p{N}.png`, zero-padded to total-page width).
- Mirrors CR-0001's structure: frontmatter, motivation, current/proposed
state diagrams, MUST/SHOULD-keyed FR/NFRs, affected components, scope
boundaries, alternatives considered (page.screenshot vs. reusing the
CR-0001 client rasterizer vs. satori/resvg vs. Puppeteer), phased
implementation approach, test strategy (unit-test preflight/flags/
filename/atomic-write; real headless render is a manual smoke step),
Gherkin ACs, quality-standards checklist, risks (Playwright missing, slim
Docker images without Chromium system libs, readiness timeout, large
decks, port conflicts), dependencies, decision outcome, and open
questions.
- Q1 (dep type): lock playwright-chromium to devDependencies only (not dependencies, not optionalDependencies). Deliberate divergence from Slidev's optionalDependencies precedent; propagated into Change Summary, FR-4, FR-5, NFR-2, NFR-8, Affected Components, Alternatives (added explicit rejected-optionalDependencies bullet), User/Technical Impact, AC-4, AC-10, AC-12, Risk 1 (likelihood raised to "very high" — default end-user path), Test Strategy row, Decision Outcome, Dependencies. - Q2 (slide enumeration): lock /__slides dev-server API as MUST in new FR-6a; disk walk recorded under Alternatives as rejected; Phase 3 step 2 updated. - Q3 (waitForPageReady extraction): make it mandatory in Phase 2, not optional; Affected Components now lists print-ready.ts + in-viewer PNG/PDF exporter call sites as migration targets; Risk 6 mitigation reflects mandatory migration. - Q4 (--concurrency): confirmed explicitly out of scope; remains under Future Enhancements only. - Open Questions section rewritten to mark each as Resolved (mirroring CR-0001 format) with "Unresolved (0)" footer. status: draft (unchanged).
…solved, ambiguity fixed - Drift: `/__slides` middleware in vite/routes/slides.ts handles per-slide mutations only; no `GET /__slides` listing exists. Rewrote FR-6a, Phase 3 step 2, Affected Components, Alternative Approaches, and Open Question 2 to name the real source of truth (`virtual:open-slide/slides` produced by `open-slide-plugin.ts`) and offer two viable concrete mechanisms. - Drift: viewer reads `?p=<N>` (1-based) per `routes/slide.tsx:91`, not `?page=<N>`. Fixed Proposed-Change step 4, the proposed-state Mermaid diagram, and Phase 3 step 3.1. - Flipped Open Question 2 from Resolved to Partially-resolved with UNRESOLVED enumeration-mechanism choice (virtual-module bridge vs. new HTTP endpoint); footer Unresolved count 0 -> 1. - Appended review-summary block: 5 findings (2 drift + 3 non-drift), 8 fixes applied, 1 unresolved item flagged for human decision.
…des endpoint
- Finalise FR-6a: CLI fetches GET /__slides from the in-process dev server,
returning [{ id, pages }] sourced from the same data that feeds the
virtual:open-slide/slides module.
- Add Phase 3 step to build the GET /__slides handler in
packages/core/src/vite/routes/slides.ts before enumerateSlides.
- Rewrite enumerateSlides() to use fetch('http://127.0.0.1:<port>/__slides').
- Update Affected Components to list routes/slides.ts (edited) and the
reused server-side enumeration helper behind open-slide-plugin.ts.
- Update Mermaid Phase 3 subgraph and Alternative-Approaches bullet to
match the resolved decision.
- Add AC-3a covering /__slides response shape and correctness.
- Mark Open Question 2 RESOLVED (option b) and flip the footer to
Unresolved (0).
- Preserve previously-applied ?p=<N> 1-based query param fixes.
…etection
- Register `open-slide export` subcommand in cli/run.ts mirroring dev/build/preview: Commander flags (--slide, --all, --page, --out, --port, --timeout) with lazy `await import('./export.ts')`.
- New cli/export.ts exporting ExportFlags interface, tryImportPlaywright() returning the `{ chromium }` namespace or null, and an exportCommand() entry that on null prints a copy-pasteable install message (pnpm add -D playwright-chromium; npx playwright install chromium) and exits code 2.
- Add playwright-chromium to packages/core devDependencies only (not dependencies, not optionalDependencies) per FR-5 / NFR-2.
- Extract waitForPageReady(frame) helper in print-ready.ts composing waitForFonts, waitForDataWaitfor, and isFrameAnimationSettled with the shared ANIMATION_TIMEOUT_MS / POLL_INTERVAL_MS constants. - Migrate export-png.ts to call waitForPageReady (refactor; same behavior). - Route export-pdf.ts through the shared constants while preserving its parallel multi-frame settle poll for progress reporting (behavior unchanged). - In slide.tsx, when ?export=png is present, await waitForPageReady on the data-osd-canvas frame then set window.__OPEN_SLIDE_EXPORT_READY = true and data-os-export-ready="true". No-op when the param is absent.
Implements Phase 3 of CR-0002:
- Adds `GET /__slides` to packages/core/src/vite/routes/slides.ts,
sourced from a new shared enumerateSlideIdsAndPages() helper in
open-slide-plugin.ts that reuses findSlides + a new
countDefaultExportPagesInSource() AST helper in slide-ops.ts.
- Implements startDevServer, enumerateSlides, resolveExportTargets,
renderOne, atomicWriteFile, and the orchestration loop in
packages/core/src/cli/export.ts. Launches a single Chromium browser
with a 1920x1080 viewport, awaits window.__OPEN_SLIDE_EXPORT_READY,
captures via page.screenshot with a 1920x1080 clip, writes
atomically as {slideId}-p{N}.png with CR-0001 padding, logs one
line per page plus a summary, and tears down browser + server in
try/finally on every exit path.
Add unit tests for the `open-slide export` CLI per the CR-0002 Test Strategy: filename padding (FR-8), slide/page resolution, the atomic- write helper (FR-11), and the missing-Playwright preflight branch (FR-4, NFR-8, AC-4) — verifying both copy-pasteable install hints and that no Vite dev server is booted on miss. Real Chromium rendering is left to manual verification per AC-1. Export `atomicWriteFile` so the helper can be exercised in isolation without going through `renderOne` (which requires a live Page). Add a single-line minor changeset for `@open-slide/core` noting that Playwright remains a devDependency only.
The CLI export subcommand dynamic-imports playwright-chromium. Because it was not in tsdown's external list, rolldown tried to bundle Playwright and failed loading its native fsevents.node binary (UNLOADABLE_DEPENDENCY). Marking it external resolves the dynamic import at runtime from the user's install instead of bundling it — which is also the intended packaging: Playwright is a devDependency and must never ship in the client bundle.
- Amend CR Phase 2 / Affected Components / Risk 6 to reflect the correct
end state for readiness sharing: PNG and headless single-frame paths use
waitForPageReady(frame); the PDF exporter (parallel multi-frame capture
with mid-run progress reporting) imports the same predicates and timing
constants from print-ready.ts while retaining its parallel orchestration
loop. Collapsing PDF into per-frame waitForPageReady would destroy
observable per-frame progress, so the right fix is the CR amendment,
not a regression of the PDF UX.
- Enrich `open-slide export --help` with an Examples block (one command
per flag) via Commander `.addHelpText('after', …)`, closing NFR-7.
- Reconcile AC-11 / NFR-4 wording with CR-0001's convention: no NEW
warnings introduced by this CR (pre-existing biome warnings in untouched
files are out of scope).
- pnpm typecheck, pnpm check (2 pre-existing warnings, unchanged), and
pnpm test (276/276) all pass.
- apps/web/content/docs/cli/export.mdx: new CLI reference (prereqs, flags, output, exit codes, in-viewer vs headless table). - apps/web/content/docs/cli/meta.json + overview.mdx: list the export subcommand. - apps/web/content/docs/core-feature/export.mdx: add headless CLI export subsection (Playwright prerequisite, distinct from CR-0001 in-viewer export). - README.md: mention the headless CLI export path.
The CLI export navigated to the viewer with ?export=png but the page still rendered the full editor chrome (toolbar + thumbnail rail), so the headless 1920x1080 clip captured the app shell instead of the slide. Route ?export=png through the chrome-less Player branch (same as !showSlideUi) so the slide fills the 1920x1080 viewport, and resolve the readiness frame via document query (the export view renders exactly one canvas) instead of the editor-only viewport ref. Verified headlessly: 121 pages across 14 demo decks export as clean, full-bleed, per-page-correct 1920x1080 PNGs.
The `playwright-chromium` devDependency added for `open-slide export` also claims the `playwright` bin, and pnpm resolves the collision in its favour — that CLI has no `test` command, so `pnpm test:e2e` died with "unknown command 'test'". Call @playwright/test's cli.js by path so the e2e suite is independent of which package wins the bin.
Both image exporters rasterise a clone of the mounted slide, and a clone re-enters the document with its CSS animations back at the 0% frame. Intro animations conventionally start at opacity 0 with fill-mode both, so a clone taken after the live animations settled still painted an empty slide — decks that animate their content in exported as bare background and footer. The image PPTX exporter already solved this with freezeForCapture, which pins the settled opacity/transform/filter/clip-path inline and disables animation and transition. Lift it into capture-freeze.ts and apply it in the PNG path too, plus neutralise animations in the generated pseudo-element rules, which freezeForCapture cannot reach. PPTX output is byte-identical after the extraction.
The point of PNG export is not only that a human can download a slide — it is that a model authoring the deck can look at one. A 1920x1080 PNG is readable by frontier models with high-resolution image understanding, so clipping, overflow, distorted aspect ratios, collisions, and unreadable type get caught by inspecting the rendered slide instead of predicting it with arithmetic. Records that motivation in the CLI export docs, the export feature page, the README, and the docs index, and builds the loop into the slide-authoring skill's self-review step so agents verify their own output rather than declaring success unseen.
Both CRs were completed against a branch ~120 commits behind upstream. Rebasing and the verification that followed changed the implementation in ways the documents did not describe. CR-0001: records agent visual verification as the primary motivation; corrects the component list; notes that the PNG entries now live in upstream's shared exportMenuItems fragment; adds Phase 6 (capture freeze) and the five tests that came with it; marks Risk 1 as materialised and resolved. CR-0002: records autonomous agent verification as the primary driver for the headless path; documents the asset-warming gate bypass the readiness signal now requires; extends the component list with slide-ops page counting, the tsdown external, and the test:e2e bin-collision fix. Both gain an Implementation Reconciliation section and updated source-commit frontmatter, since the pre-rebase hashes no longer resolve. No requirement or acceptance criterion changed — all still describe the shipped behaviour.
Docstrings and test names cited FR/NFR/AC numbers, risk and phase numbers, and CR-0002 by name. Those identifiers mean nothing to a reader holding only the source file, and they go stale the moment a CR is renumbered or superseded. Replaces each pointer with the reason it stood for: why the Playwright import must be dynamic, why a readiness timeout still captures, why the browser teardown is in a finally, why cross-origin images are skipped, why pages are rasterised one at a time. Behaviour is unchanged; test names lose their identifier suffixes. Also drops a "see CR-0002" from the published CLI export docs, which pointed readers at a document they cannot access.
The image PPTX exporter carried private copies of four things that already had canonical homes: downloadBlob (extracted to download.ts by the PNG work, already used by export-html and export-png), the ANIMATION_TIMEOUT_MS and POLL_INTERVAL_MS timing constants and sleep from print-ready.ts, and its own SLIDE_W/SLIDE_H literals shadowing CANVAS_WIDTH/CANVAS_HEIGHT in sdk.ts. The PNG exporter had a third copy of print-ready's rAF helper. Points every duplicate at its canonical definition. Net -14 lines, and the 1920x1080 canvas size now has one source of truth across all three capture paths. export-pptx keeps its own nextPaint: unlike print-ready's nextFrame it also settles on a 50ms timeout, because requestAnimationFrame never fires in a background tab and a whole-deck capture would hang there. That difference is now documented at the definition. Verified behaviour-neutral: image PPTX exports byte-identically (14925001 bytes) and PNG byte-identically (1735883 bytes) before and after.
|
@desek is attempting to deploy a commit to the open-slide Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe PR adds client-side PNG export for the viewer, a headless ChangesPNG export
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Viewer
participant PNGExporter
participant Chromium
participant FileSystem
User->>Viewer: choose PNG export
Viewer->>PNGExporter: render page or deck
PNGExporter->>Viewer: wait for ready frame
PNGExporter-->>User: download PNG or ZIP
User->>Chromium: run open-slide export
Chromium->>FileSystem: write captured PNG files
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
desek
added a commit
to desek/open-slide
that referenced
this pull request
Jul 28, 2026
Image-only branch referenced by PR 1weiho#379. Not part of any release branch and never merged; delete once the PR is closed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/package.json (1)
101-105: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAlign
playwright-chromiumwith@playwright/test.
@playwright/testis at~1.56.1, butplaywright-chromiumis^1.49.0. Playwright packages and their bundled Chromium binaries are released together; mixing major/minor versions can trigger runtime incompatibilities or protocol/API mismatches between e2e tests and anyplaywright-chromium-based exporter path. Bumpplaywright-chromiumto~1.56.1.🤖 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/core/package.json` around lines 101 - 105, Update the playwright-chromium dependency version in package.json from ^1.49.0 to ~1.56.1 so it matches the `@playwright/test` version.
🧹 Nitpick comments (4)
packages/core/src/app/lib/download.test.ts (1)
1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the descriptive test header.
It restates the test’s behavior rather than a non-obvious constraint. Keep the Vitest environment directive on Line 10.
Proposed cleanup
-/** - * Unit test for the shared `downloadBlob` helper: verifies a single - * object URL is created, the `<a download>` is removed from the DOM, and - * the URL is revoked on the next tick so the browser has time to start - * the download before release. - * - * `@agents-index` Vitest test for the shared downloadBlob helper. - */ - // `@vitest-environment` happy-dom🤖 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/core/src/app/lib/download.test.ts` around lines 1 - 8, Remove the descriptive documentation header above the test in the downloadBlob test file, including its summary and agents-index annotation. Preserve the Vitest environment directive on line 10 and leave the test implementation unchanged.Source: Coding guidelines
packages/core/src/app/components/png-progress-toast.tsx (1)
1-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant component docblocks.
These comments explain WHAT the component renders and reference related implementation, rather than preserving a non-obvious constraint. As per coding guidelines,
**/*.{ts,tsx,js,jsx}files default to no comments unless the WHY is non-obvious.Also applies to: 17-22
🤖 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/core/src/app/components/png-progress-toast.tsx` around lines 1 - 10, Remove the redundant top-level docblocks in png-progress-toast.tsx, including the component description and `@agents-index` annotation. Leave the component implementation unchanged and retain only comments that document non-obvious rationale or constraints.Source: Coding guidelines
.changeset/cli-export-png.md (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrim to one line, no rationale.
The description is two sentences and explains rationale ("so end-user installs are unaffected"). As per coding guidelines,
.changeset/*.md: "Changeset descriptions must be short and direct: one line, present-tense, describing what changed from a user's perspective. No paragraphs, no rationale, no 'this PR…'."✏️ Suggested trim
-Add `open-slide export` CLI subcommand for headless PNG export. `playwright-chromium` is a devDependency only, so end-user installs are unaffected; the subcommand preflights for it and prints copy-pasteable install instructions when absent. +Add `open-slide export` CLI subcommand for headless PNG export.🤖 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 @.changeset/cli-export-png.md at line 5, Condense the changeset description to one short, present-tense line describing the user-facing addition of the open-slide export CLI subcommand for headless PNG export; remove the rationale and installation-preflight details.Source: Coding guidelines
packages/core/src/cli/export.ts (1)
122-140: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on the
/__slidesfetch.If the in-process dev server hangs (e.g. mid-startup, or a middleware stalls), this
fetchwill block indefinitely with no diagnostic, hanging the whole CLI run despite the per-page--timeoutflag suggesting bounded runtime elsewhere.♻️ Suggested fix
export async function enumerateSlides(port: number): Promise<SlideEntry[]> { - const res = await fetch(`http://127.0.0.1:${port}/__slides`); + const res = await fetch(`http://127.0.0.1:${port}/__slides`, { + signal: AbortSignal.timeout(10_000), + });🤖 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/core/src/cli/export.ts` around lines 122 - 140, Update enumerateSlides to perform the /__slides fetch with an explicit bounded timeout, using the existing CLI timeout configuration or established timeout constant if available. Ensure the request aborts when the deadline is exceeded so the CLI cannot hang indefinitely, while preserving the current response validation and error handling.
🤖 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 @.changeset/export-slides-as-png.md:
- Line 5: Update the changeset description to use clear, user-facing
present-tense wording while preserving the “Export as PNG” viewer download menu
behavior.
In @.changeset/png-visual-verification-docs.md:
- Line 5: Rewrite the changeset description to be short and user-focused,
describing only the documentation change: PNG export for slide visual
verification. Remove internal agent rationale and the additional implementation
detail.
In `@docs/cr/CR-0001-export-slides-as-png.md`:
- Around line 1067-1070: Align the reconciliation metadata with the actual
frontmatter in both docs/cr/CR-0001-export-slides-as-png.md lines 1067-1070 and
docs/cr/CR-0002-cli-export-slides-as-png.md lines 1347-1351: either update each
source-commit assertion to match its recorded frontmatter value or revise the
frontmatter so both statements consistently describe the rebased commit.
In `@docs/cr/CR-0001-validation-report.md`:
- Line 145: Remove the local absolute REPORT_PATH footer from
docs/cr/CR-0001-validation-report.md lines 145-145 and
docs/cr/CR-0002-validation-report.md lines 149-149, or replace each value with a
repository-relative path so committed reports do not expose local account
identifiers.
In `@packages/core/src/app/lib/export-png.rasterize.ts`:
- Around line 112-136: Update capturePseudoElements to mark the generated
pseudo-element style block for processing, then extend inlineBackgroundImages to
locate that block and inline same-origin background-image URLs in its CSS text
using the existing URL inlining flow. Preserve current handling for live
elements and ensure the generated pseudo-element rules contain data URLs before
cloneWithInlinedStyles completes.
- Around line 1-24: Remove the module-level header JSDoc blocks from
packages/core/src/app/lib/export-png.rasterize.ts (lines 1-24),
packages/core/src/app/lib/capture-freeze.ts (lines 1-15),
packages/core/src/app/lib/capture-freeze.test.ts (lines 1-7),
packages/core/src/app/lib/download.ts (lines 1-10),
packages/core/src/app/lib/export-png.test.ts (lines 1-8), and
packages/core/src/app/lib/export-png.ts (lines 1-19). Preserve only concise
inline comments for genuinely non-obvious invariants, such as the foreignObject
network-isolation constraint at its usage site.
In `@packages/core/src/vite/open-slide-plugin.ts`:
- Around line 193-229: Distinguish genuine empty decks from read/parse failures
in enumerateSlideIdsAndPages by returning an explicit failure marker or status
instead of representing both as pages: 0; preserve pages: 0 for valid empty
arrays. In packages/core/src/vite/open-slide-plugin.ts lines 193-229, update
enumerateSlideIdsAndPages and its result type accordingly. In
packages/core/src/cli/export.ts lines 149-173, update resolveExportTargets or
its exportCommand caller to warn to stderr whenever a --slide or --all target
resolves to zero pages, including broken decks, while retaining successful
export behavior for valid non-empty targets.
---
Outside diff comments:
In `@packages/core/package.json`:
- Around line 101-105: Update the playwright-chromium dependency version in
package.json from ^1.49.0 to ~1.56.1 so it matches the `@playwright/test` version.
---
Nitpick comments:
In @.changeset/cli-export-png.md:
- Line 5: Condense the changeset description to one short, present-tense line
describing the user-facing addition of the open-slide export CLI subcommand for
headless PNG export; remove the rationale and installation-preflight details.
In `@packages/core/src/app/components/png-progress-toast.tsx`:
- Around line 1-10: Remove the redundant top-level docblocks in
png-progress-toast.tsx, including the component description and `@agents-index`
annotation. Leave the component implementation unchanged and retain only
comments that document non-obvious rationale or constraints.
In `@packages/core/src/app/lib/download.test.ts`:
- Around line 1-8: Remove the descriptive documentation header above the test in
the downloadBlob test file, including its summary and agents-index annotation.
Preserve the Vitest environment directive on line 10 and leave the test
implementation unchanged.
In `@packages/core/src/cli/export.ts`:
- Around line 122-140: Update enumerateSlides to perform the /__slides fetch
with an explicit bounded timeout, using the existing CLI timeout configuration
or established timeout constant if available. Ensure the request aborts when the
deadline is exceeded so the CLI cannot hang indefinitely, while preserving the
current response validation and error handling.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 89663989-a1c3-43ee-aa73-385b674dfcc7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (42)
.changeset/cli-export-png.md.changeset/export-slides-as-png.md.changeset/png-visual-verification-docs.mdREADME.mdapps/web/content/docs/cli/export.mdxapps/web/content/docs/cli/meta.jsonapps/web/content/docs/cli/overview.mdxapps/web/content/docs/core-feature/export.mdxapps/web/content/docs/index.mdxapps/web/content/docs/reference/config.mdxdocs/cr/CR-0001-export-slides-as-png.mddocs/cr/CR-0001-validation-report.mddocs/cr/CR-0002-cli-export-slides-as-png.mddocs/cr/CR-0002-validation-report.mdpackage.jsonpackages/core/package.jsonpackages/core/skills/slide-authoring/SKILL.mdpackages/core/src/app/components/png-progress-toast.tsxpackages/core/src/app/lib/capture-freeze.test.tspackages/core/src/app/lib/capture-freeze.tspackages/core/src/app/lib/download.test.tspackages/core/src/app/lib/download.tspackages/core/src/app/lib/export-html.tspackages/core/src/app/lib/export-pdf.tspackages/core/src/app/lib/export-png.rasterize.tspackages/core/src/app/lib/export-png.test.tspackages/core/src/app/lib/export-png.tspackages/core/src/app/lib/export-pptx.tspackages/core/src/app/lib/print-ready.tspackages/core/src/app/routes/slide.tsxpackages/core/src/cli/export.test.tspackages/core/src/cli/export.tspackages/core/src/cli/run.tspackages/core/src/editing/slide-ops.tspackages/core/src/locale/en.tspackages/core/src/locale/ja.tspackages/core/src/locale/types.tspackages/core/src/locale/zh-cn.tspackages/core/src/locale/zh-tw.tspackages/core/src/vite/open-slide-plugin.tspackages/core/src/vite/routes/slides.tspackages/core/tsdown.config.ts
| "@open-slide/core": minor | ||
| --- | ||
|
|
||
| Add "Export as PNG" entry to the viewer download menu. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a user-facing present-tense changeset description.
-Add "Export as PNG" entry to the viewer download menu.
+Adds PNG export options to the viewer download menu.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Add "Export as PNG" entry to the viewer download menu. | |
| Adds PNG export options to the viewer download menu. |
🤖 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 @.changeset/export-slides-as-png.md at line 5, Update the changeset
description to use clear, user-facing present-tense wording while preserving the
“Export as PNG” viewer download menu behavior.
Source: Coding guidelines
Comment on lines
+193
to
+229
| /** | ||
| * Enumerate the deck ids and page counts the viewer would render. | ||
| * | ||
| * Reuses the exact disk-walk (`findSlides`) that feeds the | ||
| * `virtual:open-slide/slides` virtual module so the dev-server's | ||
| * `GET /__slides` endpoint (consumed by `open-slide export`) sees the same | ||
| * set of decks the viewer does — no ad-hoc parallel walk of `slidesDir`. | ||
| * | ||
| * Page counts come from parsing each deck's `export default [...]` via | ||
| * `countDefaultExportPagesInSource`; a deck whose source does not parse or | ||
| * whose default export is not an array literal is reported with `pages: 0` | ||
| * rather than silently dropped, so enumeration errors surface as visible | ||
| * zero-page decks rather than missing ones. | ||
| */ | ||
| export async function enumerateSlideIdsAndPages( | ||
| userCwd: string, | ||
| slidesDir: string, | ||
| ): Promise<Array<{ id: string; pages: number }>> { | ||
| const slidesRoot = path.resolve(userCwd, slidesDir); | ||
| const files = await findSlides(userCwd, slidesDir); | ||
| const entries = await Promise.all( | ||
| files.map(async (abs) => { | ||
| const id = toId(abs, slidesRoot); | ||
| let pages = 0; | ||
| try { | ||
| const src = await fs.readFile(abs, 'utf8'); | ||
| const counted = countDefaultExportPagesInSource(src); | ||
| if (counted !== null) pages = counted; | ||
| } catch { | ||
| pages = 0; | ||
| } | ||
| return { id, pages }; | ||
| }), | ||
| ); | ||
| entries.sort((a, b) => a.id.localeCompare(b.id)); | ||
| return entries; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Decks with 0 pages (parse failure vs. genuinely empty) are silently dropped from export output with no warning. enumerateSlideIdsAndPages collapses both cases into pages: 0, and the CLI's target resolution then produces zero work items for that deck without any error or log line, so open-slide export --all/--slide exits 0 having silently skipped a broken deck.
packages/core/src/vite/open-slide-plugin.ts#L193-L229: distinguish parse/read failure from a genuinely empty array (e.g. returnpages: -1or a separateok: booleanfield) instead of collapsing both to0.packages/core/src/cli/export.ts#L149-L173: inresolveExportTargets(or the caller inexportCommand), warn to stderr when a--slide/--alltarget resolves to zero pages, so a mid-edit or broken deck doesn't silently vanish from the export run.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 217-217: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFile(abs, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
📍 Affects 2 files
packages/core/src/vite/open-slide-plugin.ts#L193-L229(this comment)packages/core/src/cli/export.ts#L149-L173
🤖 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/core/src/vite/open-slide-plugin.ts` around lines 193 - 229,
Distinguish genuine empty decks from read/parse failures in
enumerateSlideIdsAndPages by returning an explicit failure marker or status
instead of representing both as pages: 0; preserve pages: 0 for valid empty
arrays. In packages/core/src/vite/open-slide-plugin.ts lines 193-229, update
enumerateSlideIdsAndPages and its result type accordingly. In
packages/core/src/cli/export.ts lines 149-173, update resolveExportTargets or
its exportCommand caller to warn to stderr whenever a --slide or --all target
resolves to zero pages, including broken decks, while retaining successful
export behavior for valid non-empty targets.
Addresses review feedback on the PNG export PR.
Pseudo-element styles are serialised into a generated stylesheet rather
than onto an element, so the element walk in inlineBackgroundImages could
not reach their url() values. A `::before { background-image: url(...) }`
resolved to nothing in the foreignObject, silently dropping same-origin
decoration from the export. The generated stylesheet is now marked and
its urls inlined through the same cache. Covered by a regression test
that fails without the fix.
Also:
- Warn when a deck in scope reports zero pages. Enumeration deliberately
reports an unparseable deck as `pages: 0` so the failure stays visible,
but the CLI then contributed no targets and exited 0, making a broken
deck indistinguishable from one that was never there.
- Pin playwright-chromium to ~1.56.1 to match @playwright/test. They were
on 1.60.0 and 1.56.1, which pull different Chromium builds, so the
documented `npx playwright install chromium` fetched a browser the e2e
suite could not use.
- Strip absolute local checkout paths from the committed validation
report footers.
- Shorten the changeset descriptions to one user-facing line each,
matching the existing entries.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/cr/CR-0002-validation-report.md (3)
47-49: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign changeset evidence with the actual changeset.
These sections claim
.changeset/cli-export-png.mdexplicitly states thatplaywright-chromiumis a devDependency only, but the supplied changeset contains only the PNG export description. Either correct the report evidence or update the changeset while preserving the one-line changeset guideline.Also applies to: 72-74
🤖 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 `@docs/cr/CR-0002-validation-report.md` around lines 47 - 49, Update the NFR-5 evidence to match the actual contents of `.changeset/cli-export-png.md`: remove the unsupported claim about `playwright-chromium` unless the changeset is deliberately amended. Preserve the required single-line, user-perspective description and minor bump evidence.
89-89: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the test-strategy counts.
The table contains 8 test rows, not 7, and the paragraph lists 6 bonus specs, not 4. Update the summary so the validation report accurately reflects its own table.
🤖 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 `@docs/cr/CR-0002-validation-report.md` at line 89, Update the test summary in the validation report to state that all 8 “Tests to Add” rows were implemented and that 6 bonus specs extend coverage, while preserving the existing total of 13 passing tests and the referenced test file.
99-101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the stale PDF migration GAP.
The Diff Coverage table still says
export-pdf.tswas “NOT migrated” and remains a GAP, but lines 18 and 132-134 state that this was resolved by sharing the readiness predicates and timing constants. Update this row to describe the resolved end state so the report does not contradict itself.🤖 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 `@docs/cr/CR-0002-validation-report.md` around lines 99 - 101, Update the export-pdf.ts row in the Diff Coverage table to remove the “NOT migrated” and “GAP” wording, and describe its resolved state as sharing the readiness predicates and timing constants with waitForPageReady, consistent with the report’s statements at lines 18 and 132-134.
🧹 Nitpick comments (1)
packages/core/src/app/lib/export-png.test.ts (1)
55-77: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert concrete intermediate progress values.
This test would pass if every non-terminal phase returned
0; monotonicity alone does not verify useful progress reporting. Add expected percentages for representativeprocessing,rasterising, andzippingstates.🤖 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/core/src/app/lib/export-png.test.ts` around lines 55 - 77, The computePercent test should verify concrete intermediate percentages rather than only monotonicity. Update the sequence assertions in the computePercent describe block to associate representative processing, rasterising, and zipping states with their expected values, while retaining the terminal 100% assertion for the done phase.
🤖 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 @.changeset/cli-export-png.md:
- Line 5: Update the changeset description to mention only the user-facing
capability: the new open-slide export command renders deck pages as 1920x1080
PNGs. Remove the implementation detail about headless Chromium and keep the
wording concise.
---
Outside diff comments:
In `@docs/cr/CR-0002-validation-report.md`:
- Around line 47-49: Update the NFR-5 evidence to match the actual contents of
`.changeset/cli-export-png.md`: remove the unsupported claim about
`playwright-chromium` unless the changeset is deliberately amended. Preserve the
required single-line, user-perspective description and minor bump evidence.
- Line 89: Update the test summary in the validation report to state that all 8
“Tests to Add” rows were implemented and that 6 bonus specs extend coverage,
while preserving the existing total of 13 passing tests and the referenced test
file.
- Around line 99-101: Update the export-pdf.ts row in the Diff Coverage table to
remove the “NOT migrated” and “GAP” wording, and describe its resolved state as
sharing the readiness predicates and timing constants with waitForPageReady,
consistent with the report’s statements at lines 18 and 132-134.
---
Nitpick comments:
In `@packages/core/src/app/lib/export-png.test.ts`:
- Around line 55-77: The computePercent test should verify concrete intermediate
percentages rather than only monotonicity. Update the sequence assertions in the
computePercent describe block to associate representative processing,
rasterising, and zipping states with their expected values, while retaining the
terminal 100% assertion for the done phase.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d9bf2105-15f4-4bdd-b642-a07776ca4b3e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.changeset/cli-export-png.md.changeset/export-slides-as-png.md.changeset/png-visual-verification-docs.mddocs/cr/CR-0001-validation-report.mddocs/cr/CR-0002-validation-report.mdpackages/core/package.jsonpackages/core/src/app/lib/export-png.rasterize.tspackages/core/src/app/lib/export-png.test.tspackages/core/src/cli/export.test.tspackages/core/src/cli/export.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- .changeset/png-visual-verification-docs.md
- packages/core/src/cli/export.test.ts
- packages/core/src/cli/export.ts
- packages/core/src/app/lib/export-png.rasterize.ts
| "@open-slide/core": minor | ||
| --- | ||
|
|
||
| Add an `open-slide export` command that renders deck pages to 1920x1080 PNGs via headless Chromium. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the changeset description user-facing.
“Via headless Chromium” is an implementation detail. Use a concise description of the observable capability instead.
Suggested wording
-Add an `open-slide export` command that renders deck pages to 1920x1080 PNGs via headless Chromium.
+Add an `open-slide export` command to export deck pages as 1920x1080 PNGs.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Add an `open-slide export` command that renders deck pages to 1920x1080 PNGs via headless Chromium. | |
| Add an `open-slide export` command to export deck pages as 1920x1080 PNGs. |
🤖 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 @.changeset/cli-export-png.md at line 5, Update the changeset description to
mention only the user-facing capability: the new open-slide export command
renders deck pages as 1920x1080 PNGs. Remove the implementation detail about
headless Chromium and keep the wording concise.
Source: Coding guidelines
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
1 participant
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.
Adds PNG export to open-slide, from both the viewer toolbar and a new headless CLI subcommand.
Related: #364 — same motivation, broader format scope. This covers PNG; the readiness signal it adds is format-agnostic, so PDF/HTML/PPTX can build on the same seam.
Motivation
The primary use case is visual verification by an agent.
A model authoring a deck is blind to its own output. It can compute a layout but never look at it, so the
slide-authoringskill has it predict the vertical budget with arithmetic — sumfont_size × line_height × lines+ gaps + padding, and hope the page fits inside 1080px. The skill itself calls overflow "the #1 cause of broken slides", precisely because nothing in the loop checks the prediction.A 1920×1080 PNG closes that loop. Vision-capable models read the file directly and review a deck the way a human would, catching what static reasoning cannot:
object-fitBecause the export is exactly the canonical canvas size, what the model inspects is what the audience sees. The headless CLI is what makes the loop autonomous: an agent exports, reads the images back, and fixes what they reveal, with no human clicking a dropdown.
Humans get the same output for the obvious secondary uses — README hero images, blog cards, design reviews.
What's included
Viewer export. Two entries in the toolbar export menu: Export current slide as PNG (a single 1920×1080 file) and Export all slides as PNG (a ZIP, one PNG per page, named
{slideId}-p{N}.png). Rasterisation is entirely client-side through a hand-rolled<foreignObject>→ canvas path — no server, no headless browser, no new runtime dependency. A progress toast surfacesprocessing → rasterising → zipping → done, and Safari gets a best-effort notice up front. Both entries live inside the existingexportMenuItemsfragment, so they appear in the desktop and mobile menus from one definition, gated bybuild.allowHtmlDownloadlike the rest.Headless CLI.
open-slide exportboots the dev server in-process, drives headless Chromium through the real viewer route under?export=png, waits on a readiness signal (fonts,data-waitfor, animation settle), and writes 1920×1080 PNGs to disk.playwright-chromiumis a devDependency only — neverdependencies, neveroptionalDependencies— so end-user installs stay lean. A missing install is therefore the default path for users of the published package, not an edge case: the CLI preflights, exits 2, and prints a copy-pasteable install hint rather than a stack trace. Files are written atomically. Exit codes follow 0 success / 1 runtime error / 2 usage or preflight.Locale strings ship for all four supported locales, and the docs cover both paths. The
slide-authoringskill's self-review step now instructs the model to export the pages it touched and inspect them — that step is what turns the feature into a habit rather than a capability. It reaches the@open-slide/cliscaffold automatically throughsync-template-skills.Screenshots
The two new entries in the toolbar export menu, alongside the existing formats:
Progress toast during a full-deck export:
Resulting output — page 1 of the same deck at 1920×1080, gradients and all:
Implementation notes
Shared with the image PPTX exporter. Both image pipelines rasterise a clone of the mounted slide and hit the same problems, so where a solution already existed this consumes it rather than reimplementing.
freezeForCapture— which the PPTX exporter had solved privately — is lifted intocapture-freeze.tsand shared. Readiness predicates and timing constants come fromprint-ready.ts, now the single source of truth across all three capture paths.fflate,designToCssVars, andSlidePageProviderare reused as-is.The final commit closes the gaps in the other direction: the PPTX exporter carried private copies of
downloadBlob, the timing constants,sleep, and its ownSLIDE_W/SLIDE_Hliterals shadowingCANVAS_WIDTH/CANVAS_HEIGHT. Those now point at their canonical definitions — net −14 lines, one definition of the canvas size, byte-identical output.Two things stay duplicated deliberately. The mount orchestration differs by design: PPTX mounts every page at once because its progress poll needs them all, PNG mounts one at a time to bound peak memory to a single host plus one in-flight blob. And
export-pptxkeeps its ownnextPaint, which settles on a 50 ms timeout as well asrequestAnimationFrame, because rAF never fires in a background tab; that rationale is now documented at the definition.Animation freezing. Rasterising a clone means the clone re-enters the document with its CSS animations back at the 0% frame. Intro animations conventionally start at
opacity: 0withanimation-fill-mode: both, so waiting for the live DOM to settle is necessary but not sufficient — the settled state has to be pinned into the markup the rasteriser actually reads. That is whatfreezeForCapturedoes, along with equivalent handling for pseudo-elements, whose styles are emitted as CSS text and carry theanimationshorthand with them. Without it, any deck that animates its content in exports as bare background and footer.Incidental fixes
Two changes unrelated to the feature, needed to keep the branch green:
test:e2escript.playwright-chromiumalso claims theplaywrightbin, and pnpm resolves the collision in its favour — that CLI has notestcommand, so the e2e suite failed withunknown command 'test'. The script now invokes@playwright/test'scli.jsby path, independent of which package wins the bin. Without this the suite cannot run at all.[data-osd-canvas]out of the document past the CLI's readiness poll and times out every page. Export mode bypasses it — warming the whole deck's assets is pointless when the headless run loads one page per navigation, and readiness already waits on the single frame being captured.Testing
pnpm testpnpm test:e2epnpm typecheckpnpm checkpnpm buildThe 6 biome warnings pre-exist on
mainand sit in files this branch does not touch.Verified in a real browser beyond the suites: the CLI produces 12 correct 1920×1080 PNGs for a 12-page deck; both toolbar entries download a single PNG and a 12-file ZIP; and a gradient-heavy deck exports its radial gradients, gradient panels, and dark base faithfully through both paths.
Neither image exporter has unit coverage of its rendered output — the tests stub the rasteriser — so the refactors touching shared code were checked at the byte level instead. Image PPTX exports at 14925001 bytes and PNG at 1735883 bytes, identical before and after both the
freezeForCaptureextraction and the helper consolidation.A note on
docs/cr/This branch carries two Change Request documents under
docs/cr/(~2,600 lines) plus their validation reports. They come from the governance process used to build this on my side — requirements, acceptance criteria, phase breakdowns, and a record of what shipped versus what was planned.They are not part of the feature and this repo has no such convention. I have left them in because they explain a fair amount of the reasoning behind the implementation choices, but I am happy to strip them from the branch if you would rather keep the diff to code and docs — just say the word and I will force-push without them. Dropping them touches nothing else in the PR.
Limitations
<img>andurl()backgrounds are skipped in the client-side path; fetching them would fail CORS or taint the canvas and breaktoBlob. Same-origin deck assets embed normally, and the CLI path has no such limit.<foreignObject>pipeline has long-standing quirks, so the viewer export warns and proceeds best-effort there.Summary by CodeRabbit
open-slide exportCLI for generating 1920×1080 PNGs.