Repository navigation
feat(exporters): export the active design as PNG - #471
Conversation
Add PNG to the Export menu. It reuses the PDF export document and the installed Chrome, then saves a full-page screenshot at 2x. Existing exporters are unchanged.
github-actions
Bot
added
docs
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
-
[Minor] The "no Chrome" hint no longer covers the new format — PNG export requires Chrome, but the shared
EXPORTER_NO_CHROMEstring still says Chrome only enables PDF. A user without Chrome who picks PNG sees "Install it to enable PDF export," which is misleading. Evidence:packages/i18n/src/locales/en.json:1532(same string inpackages/i18n/src/locales/es.json:1355,packages/i18n/src/locales/pt-BR.json:1345,packages/i18n/src/locales/zh-CN.json:1528). Since this PR already edits each of these locale blocks, update the copy in all four locales.
Suggested fix: changeEXPORTER_NO_CHROMEto e.g. "Chrome or Chromium was not found. Install it to enable PDF and PNG export." (localized) in each locale. -
[Nit]
finallycleanup can mask the real outcome —browser.close()andrm(userDataDir)are awaited inside thefinally(packages/exporters/src/png.ts:61-64). If either rejects after a successful screenshot + write,exportPngrejects even though the PNG was saved; if it rejects while an error is propagating, the caller receives the close/rm error instead ofEXPORTER_PNG_FAILED. If this mirrors the existingpdf.tslifecycle, matching it is acceptable for consistency; otherwise make cleanup non-fatal.
Suggested fix: guard the teardown so it cannot override the primary result, e.g.try { await browser.close(); } finally { await rm(userDir, { recursive: true, force: true }).catch(() => {}); }.
Questions
- None.
Summary
Review mode: initial
The feature is wired end-to-end and internally consistent: format list + type (packages/exporters/src/index.ts:13), subpath export (packages/exporters/package.json:12), lazy dispatch (packages/exporters/src/index.ts:67), main-process filter + parseRequest guard (apps/desktop/src/main/exporter-ipc.ts:30, :99), renderer ExportFormat union and menu order, new error code + descriptions, i18n in all four locales, and a changeset. Lazy-loading is preserved (exportPng dynamically imports puppeteer-core and ./chrome-discovery), no new dependencies are added, and the runtime dep is the already-shipped puppeteer-core (MIT-compatible). No security, data-loss, or release/distribution concerns found. The PR states "No linked issue," so there is no completion claim to validate.
Residual risk (acknowledged and out of scope per the description): deck-style documents are captured as a single tall image, and fullPage: true relies on document height, so fixed/100vh layouts can clip to the viewport. Worth a follow-up issue if per-slide PNGs are desired.
The PR body notes that real-Chrome Vitest cases (PromptInput.browser, preview-runtime, source-edit-engine.text-coverage) time out under parallel macOS runs and pass in isolation. That is pre-existing suite flakiness and is not introduced by this diff.
Testing
- Added coverage looks proportionate:
packages/exporters/src/png.test.ts(2x viewport, caller-provided viewport,EXPORTER_PNG_FAILEDwith Chrome closed) andapps/desktop/src/main/exporter-ipc.test.ts(pngparseRequest,ensureExportExtension). - Not covered: renderer menu rendering of the new PNG item, and
exportArtifact('png', ...)dispatch inpackages/exporters/src/index.ts. Low risk. - Not run (automation) beyond what CI executes.
Open-CoDesign Bot
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Deck exports can omit slides, and profile cleanup errors can override export results.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds PNG export to Open CoDesign’s existing export flow using installed Chrome, without new dependencies.
Changes:
- Adds lazy-loaded, full-page PNG capture at 2× resolution.
- Connects PNG to the desktop Export menu and save dialog.
- Adds translations, error handling, tests, and release metadata.
| File | Description |
|---|---|
packages/shared/src/error-codes.ts |
Adds PNG failure code and message. |
packages/i18n/src/locales/zh-CN.json |
Adds Chinese PNG translations. |
packages/i18n/src/locales/pt-BR.json |
Adds Portuguese PNG translations. |
packages/i18n/src/locales/es.json |
Adds Spanish PNG translations. |
packages/i18n/src/locales/en.json |
Adds English PNG text. |
packages/exporters/src/png.ts |
Implements Chrome-based PNG capture. |
packages/exporters/src/png.test.ts |
Tests capture options, output, and failures. |
packages/exporters/src/index.ts |
Registers lazy-loaded PNG export. |
packages/exporters/package.json |
Exposes the PNG module. |
apps/desktop/src/renderer/src/components/PreviewToolbar.tsx |
Adds PNG to the Export menu. |
apps/desktop/src/preload/index.ts |
Extends the export format type. |
apps/desktop/src/main/exporter-ipc.ts |
Accepts PNG requests and save filters. |
apps/desktop/src/main/exporter-ipc.test.ts |
Tests PNG requests and filename extensions. |
.changeset/png-export.md |
Records the user-visible addition. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (opts.settleMs && opts.settleMs > 0) { | ||
| await new Promise((resolve) => setTimeout(resolve, opts.settleMs)); | ||
| } | ||
| const png = await page.screenshot({ type: 'png', fullPage: true }); |
| ); | ||
| } finally { | ||
| if (browser) await browser.close(); | ||
| await rm(userDataDir, { recursive: true, force: true }); |
PNG export also needs a system Chrome, so the shared error text and its four translations now name both PDF and PNG export.
There was a problem hiding this comment.
Findings
No new findings in this follow-up. The single Minor from the previous review — the EXPORTER_NO_CHROME hint no longer covering the new format — is resolved across all four locales and in the shared error description.
Summary
Review mode: follow-up after new commits
The only changes since the last bot review are the localized EXPORTER_NO_CHROME strings and the shared userFacing description, each now naming PNG alongside PDF: packages/i18n/src/locales/en.json:1532, packages/i18n/src/locales/es.json:1355, packages/i18n/src/locales/pt-BR.json:1345, packages/i18n/src/locales/zh-CN.json:1528, and packages/shared/src/error-codes.ts:380. This directly addresses the earlier Minor and keeps the four locale files in sync with the shared error code metadata.
The rest of the feature wiring is unchanged and remains internally consistent: format list + type (packages/exporters/src/index.ts:13), subpath export (packages/exporters/package.json:12), lazy dispatch (packages/exporters/src/index.ts:67), main-process filter + parseRequest guard (apps/desktop/src/main/exporter-ipc.ts:30, :99), renderer ExportFormat union and menu order, new error code, i18n menu entries, and a changeset. Lazy-loading is preserved (exportPng dynamically imports puppeteer-core and ./chrome-discovery), no new dependency is added, and the runtime dep is the already-shipped MIT-compatible puppeteer-core. No security, data-loss, or release/distribution concerns found. The PR states "No linked issue," so there is no completion claim to validate.
Residual observations (existing, non-blocking, not re-filed as findings):
- The
finallyteardown inpackages/exporters/src/png.ts:61-64still awaitsbrowser.close()andrm(userDataDir), so a teardown rejection can mask a successful write or replaceEXPORTER_PNG_FAILED. This mirrors the existingpdf.tslifecycle, so matching it is acceptable for consistency; a follow-up could make cleanup non-fatal (e.g. swallow close/rm errors). - Deck-style documents are still captured as one tall image, and
fullPage: truerelies on document height, so fixed/100vhlayouts can clip to the viewport. The PR body explicitly defers per-slide PNGs, so this is a follow-up topic rather than a blocker.
Testing
- Coverage added in the earlier commit is unchanged and proportionate:
packages/exporters/src/png.test.ts(2x viewport, caller-provided viewport,EXPORTER_PNG_FAILEDwith Chrome closed) andapps/desktop/src/main/exporter-ipc.test.ts(pngparseRequest,ensureExportExtension). - Still not covered: renderer menu rendering of the new PNG item and
exportArtifact('png', ...)dispatch inpackages/exporters/src/index.ts. Low risk. - Not run (automation) beyond what CI executes.
Open-CoDesign Bot
Removing Chrome's temporary profile is now best-effort, as in PDF export, so a locked profile directory no longer turns a saved PNG into a failure or replaces the original export error.
There was a problem hiding this comment.
Findings
No new findings in this follow-up. The previous residual observation about teardown masking a completed export is now partially addressed by this diff, and no new issue is introduced.
Summary
Review mode: follow-up after new commits
Since the last bot review (e6ff5edd), the only changes are in packages/exporters/src/png.ts and packages/exporters/src/png.test.ts:
packages/exporters/src/png.ts:64-68now wrapsrm(userDataDir, { recursive: true, force: true })in atry/catch, so a failed temporary-Chrome-profile removal can no longer replace a successful result or maskEXPORTER_PNG_FAILED. The added comment at:63documents the best-effort intent.packages/exporters/src/png.test.ts:98-107adds a test that pins this behavior: it stubsrmvia anode:fs/promisesmodule mock (:16-20) to reject once withEBUSYand asserts the export still resolves with{ bytes, path }. The mock usesvi.hoistedcorrectly and re-installs the real implementation viamockImplementation(actual.rm), so the other tests are unaffected and the once-rejection is consumed within this test.
Everything else is unchanged and remains internally consistent: format list + type (packages/exporters/src/index.ts:13), subpath export (packages/exporters/package.json:12), lazy dispatch (packages/exporters/src/index.ts:67), main-process filter + parseRequest guard (apps/desktop/src/main/exporter-ipc.ts:30, :99), renderer ExportFormat union and menu order, the new EXPORTER_PNG_FAILED code, the four locale entries plus shared userFacing covering both PDF and PNG, and the changeset. Lazy-loading is preserved (exportPng dynamically imports puppeteer-core and ./chrome-discovery), no new dependency is added, and the runtime dep is the already-shipped MIT-compatible puppeteer-core. No security, data-loss, or release/distribution concerns found. The PR states "No linked issue," so there is no completion claim to validate.
Residual observations (non-blocking, carried from the previous review, not re-filed as findings):
packages/exporters/src/png.ts:62still awaitsbrowser.close()unguarded in thefinallyblock, so a rejected close can still replace a successful write or theEXPORTER_PNG_FAILEDerror. This mirrors the existingpdf.tslifecycle, so matching it is acceptable; if you want the PR body's "cleanup cannot change the export result" statement to hold fully, guard this call the same way asrm(e.g.try { await browser.close(); } catch { /* noop */ }).- Deck-style documents are still captured as one tall image, and
fullPage: truedepends on document height, so fixed/100vhlayouts can clip to the viewport. The PR body explicitly defers per-slide PNGs, so this is a follow-up topic rather than a blocker.
Testing
- New coverage in this diff is proportionate: the
EBUSYprofile-removal test (packages/exporters/src/png.test.ts:98) plus the existing 2x/caller-viewport andEXPORTER_PNG_FAILED-with-Chrome-closed tests. - Still not covered: renderer menu rendering of the new PNG item and
exportArtifact('png', ...)dispatch inpackages/exporters/src/index.ts. Low risk. - Not run (automation) beyond what CI executes.
Open-CoDesign Bot
Decks such as the built-in 16:9 scaffold hide inactive slides, so the PNG only showed the first one. Before the screenshot, each slide now gets its own copy of the deck container and the copies are stacked in the page, as PDF export does for decks. Pages whose sections are all visible are left unchanged. A browser test covers the scaffold and a plain page.
There was a problem hiding this comment.
Findings
- [Minor] PR description now contradicts the diff — the body states the PNG “shows the page as it renders on load” and that “exporting every slide as an image is left out of this change,” but this revision injects
EXPAND_DECK_SCRIPTbefore capture (packages/exporters/src/png.ts:51) and stacks every slide of the deck container. A maintainer or tester reading the body will expect one visible slide and see all slides.
Suggested fix: update the PR description (and, if you want the release note to match, the changeset.changeset/png-export.md) to say decks that show one<section>at a time are expanded and stacked, slides are captured in order, and the page is no longer captured strictly “as it renders on load.”
Questions
- Have you verified a long deck (roughly 15+ 16:9 slides) exports correctly? With
DEVICE_SCALE_FACTOR = 2and all slides stacked into a singlepage.screenshot({ fullPage: true }), page height grows by ~1440 device pixels per slide; Chrome's screenshot capture has a known maximum dimension past whichPage.captureScreenshotreturns a clipped or blank image.page.pdf()paginates and does not have this constraint, so the deck expansion path is PNG-specific. If that case is untested, consider a height/scale guard for tall pages.
Summary
Review mode: follow-up after new commits
Since the previous bot review (68ce5b57), the only changes are in packages/exporters/src/png.ts and packages/exporters/src/png.test.ts. The PR now expands decks instead of capturing only the visible slide, which supersedes the earlier residual note about single-slide deck output. The new code is consistent with the rest of the change:
packages/exporters/src/png.ts:51runspage.evaluate(EXPAND_DECK_SCRIPT)aftersetContent/fonts.ready/settleMsand before the screenshot, so the mutation happens at capture time.- The script reuses the same near-16:9 size checks and body-reset approach the PR body attributes to PDF export, clones the deck container per slide, preserves the visible slide's computed
display, and copies body-level styles into<head>before replacing body children — internally coherent. - Lazy-loading is preserved (
puppeteer-coreand./chrome-discoveryremain dynamic imports), no new dependency is added, and the runtime dep is the already-shipped MIT-compatiblepuppeteer-core. No security, data-loss, or release/distribution concerns. - The PR body states “No linked issue,” so there is no completion claim to validate.
Residual observations (non-blocking):
- The new browser tests are gated behind
describe.runIf(process.env['CODESIGN_EXPORT_BROWSER_TESTS'] === '1')(packages/exporters/src/png.test.ts), so they do not run in CI. The always-on coverage only asserts the script string is passed toevaluate, not that the stacking logic produces the right layout. Treat the deck tests as manual, or wire them into a Chrome-enabled CI job. - Slide detection is limited to section parents that are not
document.bodyand to a width/height ratio of 1.5–1.95, so 4:3 decks (~1.33) and body-level decks still export a single slide. This matches the stated PDF checks, so it is likely acceptable, but worth a follow-up if that is not intended. packages/exporters/src/png.ts:62still awaitsbrowser.close()unguarded infinally, so a rejected close can replace a successful write or theEXPORTER_PNG_FAILEDerror. This mirrors the unchangedpdf.tslifecycle; carrying it from the earlier review rather than re-filing as a finding.
Testing
- Not run (automation). Suggested: exercise
exportPngon a deck with more slides than fit under Chrome's capture limit, and confirm the renderer menu renders the newpreview.export.pngitem.
Open-CoDesign Bot
Without a layout after the slides are stacked, the full-page capture could measure the page at its old size and return an image clipped to the viewport; a 20-slide deck failed every time. The expansion now forces layout before returning. The Chrome tests move to their own unmocked file and add a 20-slide deck whose full height is checked, and the changeset mentions deck expansion.
There was a problem hiding this comment.
Findings
No new blocking or material findings in this follow-up diff. The changes since 2fa9e4c4 are the changeset wording (.changeset/png-export.md:8), the forced-layout line in packages/exporters/src/png.ts:108, and the extracted browser test file packages/exporters/src/png.chrome.test.ts.
Questions
None blocking. One optional clarification below.
Summary
Review mode: follow-up after new commits
The prior review's open items are addressed:
- The PR description/changeset now agree with the code. The body states decks that show one
<section>at a time are expanded so every slide is captured in order, and that “the page is therefore not captured strictly as it renders on load for such decks”;.changeset/png-export.md:8matches. The earlier[Minor]contradiction is resolved. - The long-deck question is now backed by a test:
packages/exporters/src/png.chrome.test.tsasserts full-height capture of a 20-slide stacked deck ((24 + 20 * (675 + 24)) * 2= 28,008 px), which matches the PR body's reported height. Thevoid document.documentElement.scrollHeight;line (packages/exporters/src/png.ts:108) is a synchronous reflow that forces the stacked layout beforefullPagemeasurement; harmless and consistent with the stated rationale. - Extracting the browser-gated cases into
png.chrome.test.tsavoids depending on thevi.mock('puppeteer-core')/./chrome-discoverymocks inpng.test.tsand uses the realfindSystemChrome/puppeteerdirectly, which is the correct setup for those assertions.
Residual observations (non-blocking, carried from the prior review unless noted):
- The Chrome browser tests remain opt-in behind
describe.runIf(process.env['CODESIGN_EXPORT_BROWSER_TESTS'] === '1')(packages/exporters/src/png.chrome.test.ts:33), so the deck-stacking and full-height behavior is not exercised in CI. The always-on suite still only asserts the script string is passed toevaluate. This is acceptable as manual coverage, but wiring a Chrome-enabled job would prevent silent regressions. - The new full-height assertion compares exact device pixels (
packages/exporters/src/png.chrome.test.ts:85), which can be brittle across Chrome versions/platforms. Consider tolerating a small delta if this is ever promoted to CI. - Slide detection still only considers non-
bodysection parents with a 1.5–1.95 aspect ratio (packages/exporters/src/png.tsEXPAND_DECK_SCRIPT), so 4:3 decks and body-level decks export a single slide. This mirrors the stated PDF behavior; fine as-is, worth a follow-up only if unintended. packages/exporters/src/png.ts:62still awaitsbrowser.close()unguarded infinally, so a rejected close could mask a successful write or theEXPORTER_PNG_FAILEDerror. This mirrors the unchangedpdf.tslifecycle and is not re-filed as a finding.
No linked issue is claimed (“No linked issue”), so there is no completion claim to validate. No new dependency, no direct provider SDK import, lazy-loading of puppeteer-core/./chrome-discovery is preserved, and the reworded EXPORTER_NO_CHROME / new EXPORTER_PNG_FAILED strings are added in packages/shared/src/error-codes.ts and all four locales.
Testing
- Not run (automation). Suggested: run
CODESIGN_EXPORT_BROWSER_TESTS=1 pnpm --filter @open-codesign/exporters teston a machine with system Chrome to confirmpng.chrome.test.ts, and verify the renderer Export menu renders the newpreview.export.pngitem.
Open-CoDesign Bot
|
在导出菜单中明确说明：支持的多页幻灯片会导出为一张纵向拼接的 2 倍分辨率 PNG，方便用户预期导出结果。 |
- A failed Chrome close no longer replaces the saved PNG or the original error, and the temporary profile is still removed. - Each stacked slide now carries the shown slide's marks (classes such as "active", data-active, aria-hidden), so styles tied to them, such as opacity, apply to every slide and not only display. - The Export menu hint says slide decks are stacked in order into one tall image.
There was a problem hiding this comment.
Findings
No Blocker, Major, or Minor findings in this follow-up diff. The changes since 1a2f4c38 are confined to packages/exporters/src/png.ts (guarded close cleanup + enhanced EXPAND_DECK_SCRIPT active-slide marking), packages/exporters/src/png.chrome.test.ts (new active-slide styling test), packages/exporters/src/png.test.ts (two close-failure tests), and the four locale hints.
Prior bot-review item resolved:
packages/exporters/src/png.tsnow wrapsbrowser.close()intry/catchinside thefinallyblock, so a rejected close can no longer mask a successful write or the capturedEXPORTER_PNG_FAILEDerror. The two new cases inpackages/exporters/src/png.test.tspin exactly that: a close failure still resolves with the saved PNG and removes the temp profile (codesign-png-*), and a screenshot failure combined with a close failure still rejects with the originalPNG export failed: boom.- The enhanced
EXPAND_DECK_SCRIPTmoves the active-slide marks (classes such asactive/hiddenand differing attributes such asdata-active/aria-hidden) onto each per-slide copy instead of only forcingdisplay, which is the right generalization for decks whose visible slide is driven by more thandisplay. The new Chrome test (packages/exporters/src/png.chrome.test.ts) exercisesopacity,visibility, and an attribute-drivencolorstyle, so this path has real coverage when the browser gate is on.
Questions
None.
Summary
Review mode: follow-up after new commits
The follow-up is directionally sound and the previously noted browser.close() regression risk is fixed with matching tests. The core PNG lifecycle (lazy puppeteer-core / ./chrome-discovery imports, 2x full-page capture, EXPORTER_PNG_FAILED wrapping, best-effort profile cleanup) is unchanged and consistent with the project constraints: no new dependency, no direct provider SDK import, lazy-loading preserved, and the new/reworded error strings are added in packages/shared/src/error-codes.ts and all four locales. No linked-issue completion claim is made (“No linked issue”), so there is nothing to validate there.
Residual observations (non-blocking, mostly carried from the prior review and not re-filed as findings):
- The browser tests remain opt-in behind
describe.runIf(process.env['CODESIGN_EXPORT_BROWSER_TESTS'] === '1')(packages/exporters/src/png.chrome.test.ts), so deck stacking and full-height capture are manual coverage only; the always-on suite asserts the script is passed toevaluate. Wiring a Chrome-enabled CI job would catch silent regressions. - The full-height assertion compares exact device pixels (
packages/exporters/src/png.chrome.test.ts,(24 + 20 * (675 + 24)) * 2); it can be brittle across Chrome versions/platforms if it is ever promoted to CI. - Slide detection still only considers non-
bodysection parents within a 1.5–1.95 aspect ratio and roughly ≥480×270, so 4:3 decks and body-level decks export a single slide — this mirrors the stated PDF behavior. - The class/attribute migration assumes the non-active slides share a common marker (e.g. both carry
hidden); if distinguishing classes differ across non-active slides, onlydisplayis forced and class-driven visibility/opacity on those slides would remain. The covered scenarios and the built-in scaffold are fine; worth a follow-up only if that shape appears in practice.
Testing
- Not run (automation). Suggested: run
CODESIGN_EXPORT_BROWSER_TESTS=1 pnpm --filter @open-codesign/exporters teston a machine with system Chrome to confirmpng.chrome.test.ts(including the new active-slide styling case), and sanity-check the renderer Export menu renders the newpreview.export.pngitem between PDF and PPTX.
Open-CoDesign Bot
|
这次已处理清理异常和长图说明，感谢补充。新增的激活状态复制逻辑似乎范围过宽：对于两页幻灯片，页面独有的 id、主题类或内容属性也可能被识别为激活标记，导致第二页的身份和样式被改写。 |
The previous change treated every class or attribute that only the shown slide carried as its "active" mark. In a two-slide deck that includes the slide's id, its own layout class and content attributes, so the second copy took on the first slide's identity; the built-in 16:9 scaffold lost its content-slide layout on page 2. A class or attribute now counts as the mark only when giving the shown slide the other slides' value hides it (display none, visibility hidden, or opacity 0, checked with transitions off and the slide restored afterwards).
There was a problem hiding this comment.
Findings
No Blocker, Major, Minor, or Nit findings in this follow-up. The diff since the previous review (21b9329a) is confined to packages/exporters/src/png.ts (rewritten EXPAND_DECK_SCRIPT mark detection) and packages/exporters/src/png.chrome.test.ts (helper extraction + two new cases). No new material regression is introduced, and the earlier browser.close() fix is unchanged.
What the new diff does, and why it holds up:
packages/exporters/src/png.ts:99-134—hidesActive(change)snapshots the active slide's attributes, appliestransition: noneplus the change, reads computeddisplay/visibility/opacity, then fully restores. The restore loop (getAttributeNames()snapshot → remove all → re-apply saved) removes the injectedstyleattribute when the element had none, so notransition: noneleaks into the captured page. Detection now requires a class/attribute to actually hide the shown slide, which correctly keeps ids, per-slide classes (intro/market), and content attributes (data-title) in place while still promotingactive/hidden/aria-hidden/data-activemarks.packages/exporters/src/png.chrome.test.ts:74-138— the newstackedSlideshelper plus thekeeps the id, own classes and content attributescase exercise exactly that guarantee, and the16:9 scaffoldcase now asserts the resulting class list (['slide title-slide', 'slide content-slide']) instead of only geometry.
The prior review's resolved item (guarded close() in finally so a rejected close cannot mask a saved PNG or the original EXPORTER_PNG_FAILED) still stands in packages/exporters/src/png.ts:47-60 with matching coverage in packages/exporters/src/png.test.ts.
Questions
None.
Summary
Review mode: follow-up after new commits
No material issues found. The follow-up tightens the deck-expansion heuristic (only classes/attributes that demonstrably hide the active slide are propagated to per-slide copies) and adds real Chrome coverage for the id/class/attribute preservation case. The PR stays consistent with project constraints: no new dependency (puppeteer-core already present), lazy dynamic import of puppeteer-core and ./chrome-discovery preserved, no direct provider SDK import, EXPORTER_PNG_FAILED added to packages/shared/src/error-codes.ts and all four locales, and a changeset is present. The PR makes no linked-issue completion claim ("No linked issue"), so there is nothing to validate there. It looks ready to merge apart from the non-blocking residuals below.
Residual observations (non-blocking; carried from the prior review where still applicable):
- The Chrome tests remain opt-in behind
describe.runIf(process.env['CODESIGN_EXPORT_BROWSER_TESTS'] === '1')(packages/exporters/src/png.chrome.test.ts:35), so deck stacking and full-height capture are manual coverage only. A Chrome-enabled CI job would catch silent regressions. - The full-height assertion compares exact device pixels (
(24 + 20 * (675 + 24)) * 2,packages/exporters/src/png.chrome.test.ts), which is brittle across Chrome versions/platforms if promoted to CI. - Slide detection still only considers non-
body<section>parents at a 1.5–1.95 aspect ratio and roughly ≥480×270, so 4:3 decks and body-level decks export a single slide — this mirrors the stated PDF behavior. hidesActivetreats onlydisplay: none,visibility: hidden, andopacity: 0as "hidden". A marker expressed purely through other hiding mechanisms would not be recognized, though such decks are generally already skipped by the single-visible-slide detection. Worth a follow-up only if that shape appears in practice.
Testing
- Not run (automation). Suggested: run
CODESIGN_EXPORT_BROWSER_TESTS=1 pnpm --filter @open-codesign/exporters teston a machine with system Chrome to confirmpng.chrome.test.ts(including the new id/class/attribute case), and sanity-check the renderer Export menu renders the newpreview.export.pngitem between PDF and PPTX.
Open-CoDesign Bot
Summary
Sharing a design in chat, docs, or slides usually needs an image, and the Export menu only offered HTML, PDF, PPTX, ZIP, and Markdown. This adds PNG. It renders the same export document as PDF with the installed Chrome (
puppeteer-core, already a dependency) and saves a full-page screenshot at 2x. The existing exporters are unchanged; the missing-Chrome message now names both PDF and PNG export in all four locales. Decks that display one<section>slide at a time, such as the built-in 16:9 scaffold, would otherwise export only the visible slide. Before the screenshot, each slide gets its own copy of the deck container and the copies are stacked in the page, as PDF export does for decks, using the same slide-size checks; pages whose sections are all visible are left unchanged. In each copy, the mark that shows a slide (for example anactiveclass ordata-active="true") moves to that copy's slide, so styles tied to it, such as opacity, apply to every slide. A class or attribute counts as that mark only if giving the shown slide the other slides' value hides it; ids, per-slide layout classes, and content attributes stay on their own slide. The Export menu hint for PNG says that slide decks are stacked in order into one tall image. The page is therefore not captured strictly as it renders on load for such decks. Layout is forced after stacking so the full-page capture measures the new height. Closing Chrome and removing its temporary profile cannot change the export result: a failed close neither replaces a saved PNG nor hides the original error, and the profile is still removed afterwards (puppeteer has already fallen back to killing Chrome by the timeclose()rejects).Type of change
Linked issue
No linked issue.
Checklist
pnpm lint && pnpm typecheck && pnpm testpasses locallypnpm changeset) if user-visibleDependency additions (if any)
None.
Screenshots / recordings (UI changes)
Not attached. The Export menu gains a PNG item between PDF and PPTX.
Checks
pnpm lint(680 files),pnpm typecheck(10 tasks)pnpm -r typecheck && pnpm lint && pnpm testwithCI=1(the repo's two-worker limit): 10 tasks passed; desktop 178 files, 2,610 tests. Without the worker limit, a different real-Chrome test timed out on each of three macOS runs (PromptInput.browser,preview-runtime,source-edit-engine.text-coverage); each passed when rerun alone.packages/exporters/src/png.test.ts: 2x viewport and full-page capture, caller viewport, a saved PNG kept when profile removal fails, failure wrapped asEXPORTER_PNG_FAILEDwith Chrome closed, a saved PNG kept and the profile removed when Chrome fails to close, and the capture error reported (not the close error) when both fail. The two close cases fail without the fix.CODESIGN_EXPORT_BROWSER_TESTS=1browser tests inpng.chrome.test.ts(same gate as the existing HTML browser test): the built-in 16:9 scaffold lays out both slides top to bottom; a page with three visible sections is left unchanged; a 20-slide deck exports at its full height (28,008 px at 2x); a deck whose shown slide is marked by anactiveclass, withopacityandvisibilityrules, ends up with all three slides visible and fully opaque (with display-only stacking, slides 2 and 3 had opacity 0); a two-slide deck keeps each slide's id, own class,data-title, and colour; the 16:9 scaffold keepstitle-slideandcontent-slideon their own pages. The last two fail when every differing class or attribute is moved. All pass locally on macOS. Without the forced layout, the 20-slide export came back clipped to the viewport (1,600 px) on every run, and an 8-slide export did so intermittently.@font-facefile and a local<img>, exported as one 2560 × 6912 PNG (the expected full height). Sampling the slide backgrounds from top to bottom gives slides 1–5 in order, and every slide shows the local font and image with the same layout as the first.