Skip to content

fix: emit pic and tbl placeholder types - #1526

Open
adamstankiewicz wants to merge 1 commit into
gitbrent:masterfrom
adamstankiewicz:fix/placeholder-pic-tbl-type
Open

fix: emit pic and tbl placeholder types#1526
adamstankiewicz wants to merge 1 commit into
gitbrent:masterfrom
adamstankiewicz:fix/placeholder-pic-tbl-type

Conversation

@adamstankiewicz

@adamstankiewicz adamstankiewicz commented Aug 18, 2026

Copy link
Copy Markdown

Submission Guidelines

  • Only modified src/*.ts (no dist or src/bld)
  • No new or changed properties, so src/core-interfaces.ts and types/index.d.ts are unchanged
  • Demo added under demos/modules/

Change Summary

pic and tbl placeholders never receive a type attribute in the generated XML, so the layout emits a bare <p:ph idx="N"/>. ECMA-376 declares type as use="optional" default="obj", so the requested placeholder type is lost and the slot falls back to a generic content placeholder.

Change Description

genXmlPlaceholder() resolves the placeholder type through PLACEHOLDER_TYPES twice:

const placeholderType = placeholderTyp && PLACEHOLDER_TYPES[placeholderTyp] ? PLACEHOLDER_TYPES[placeholderTyp].toString() : ''
...
${placeholderType && PLACEHOLDER_TYPES[placeholderType] ? ` type="${placeholderType}"` : ''}

The enum is keyed by friendly name, but its values are the OOXML ones:

export enum PLACEHOLDER_TYPES { 'title' = 'title', 'body' = 'body', 'image' = 'pic', 'chart' = 'chart', 'table' = 'tbl', 'media' = 'media' }
export type PLACEHOLDER_TYPE = 'title' | 'body' | 'pic' | 'chart' | 'tbl' | 'media'

PlaceholderProps.type is typed as the OOXML value, so a caller passes 'pic'. PLACEHOLDER_TYPES['pic'] is undefined, the guard fails, and the attribute is dropped. It works today only for the four types whose enum key happens to equal its value (title, body, chart, media). Passing the enum key 'image' fails too, on the second lookup.

This resolves the value once. Object.values(...).includes(...) matches the existing idiom at src/gen-objects.ts#L335.

Verified output

Every accepted input, before and after, read from ppt/slideLayouts/slideLayout2.xml:

type passed before after
title / body / chart / media correct unchanged
pic (no type attr) type="pic"
tbl (no type attr) type="tbl"
image / table (enum keys) (no type attr) type="pic" / type="tbl"
unrecognized / undefined (no type attr) (no type attr)

Verified in a consumer

Two decks built from a master with unfilled title + pic + tbl placeholders, one from master and one from this branch, both opened in Google Slides:

pic box tbl box
before "Click to add text" "Click to add text"
after renders as a picture placeholder (frame + image icon) "Unsupported placeholder"

So the type now reaches the consumer: Slides renders a real picture placeholder, and for tbl it reports the type as one it does not implement rather than silently treating it as text. Worth flagging that second cell — with this change a tbl placeholder is labeled "Unsupported placeholder" in Google Slides where it previously looked like an ordinary text box. That is Slides being accurate about a table placeholder it has no feature for, but it is a visible difference for anyone who was (unknowingly) getting a text placeholder from type: 'tbl'.

Reproduce with:

const pptx = new PptxGenJS()
pptx.defineSlideMaster({
  title: 'PIC_MASTER',
  objects: [{ placeholder: { options: { name: 'picture', type: 'pic', x: 1, y: 1, w: 5, h: 4 }, text: '' } }],
})
pptx.addSlide({ masterName: 'PIC_MASTER' })
await pptx.writeFile({ fileName: 'ph.pptx' })
// unzip -p ph.pptx ppt/slideLayouts/slideLayout2.xml | grep -o '<p:ph[^/]*/>'
// before: <p:ph idx="100" hasCustomPrompt="1" />
// after:  <p:ph idx="100" type="pic" hasCustomPrompt="1" />

Change Type

  • Bug fix
  • New feature
  • Documentation update

Related Issue

Follow-on to #921, which corrected the PlaceholderProps.type declaration to the OOXML values. The runtime still treats the incoming value as an enum key, so the two halves disagree.

Motivation and Context

Picture placeholders are how a generated deck leaves image slots for someone to fill in later. Today they cannot be produced at all: the requested type is silently dropped, so the slot is written as an untyped placeholder rather than a picture one, and the consumer shows a text box where an image belongs (see the Google Slides comparison above).

Two notes for your call, since both are judgment rather than bug:

  • The enum-key fallback is optional. PLACEHOLDER_TYPE only permits the OOXML values, so accepting 'image'/'table' is purely defensive for untyped JS callers. Happy to drop those two lines if you would rather the accepted input stay exactly what the type declares.
  • This is a behavior change for existing pic/tbl callers. Anyone who passed type: 'pic' and then filled it with addText() was relying on the type being dropped. That seems very unlikely to be deliberate, but it is a change.

Checklist before requesting a review

  • If it is a core feature, I have added new code under /demos/modules/ (a PLACEHOLDER_TYPES_SLIDE master plus a demo slide leaving pic/tbl unfilled)
  • My code follows the style guidelines of this project
  • My changes generate no new eslint warnings (npx eslint src/gen-xml.ts demos/modules/masters.mjs demos/modules/demo_master.mjs clean; tsc --noEmit clean)
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have included code/tests that prove my fix is effective (table and repro above, read from the generated OOXML)
  • I have used the "Run All Demos" feature on the browser demo — run in Chromium against a local gulp build && gulp bundle, with demos/browser/index.html pointed at ./js/pptxgen.bundle.js and ../modules/demos.mjs instead of the CDN (that local redirect is not part of this PR). Result: the full suite completed, 0 console errors and 0 page errors, and produced a 14 MB deck whose slideLayout6.xml contains <p:ph idx="101" type="pic"/> and <p:ph idx="102" type="tbl"/>. Also run under Node (node demo.js master).

Note on scope of claims: the XML assertions are read from the generated OOXML, and the rendering behavior above was observed in Google Slides. I have not opened the result in desktop PowerPoint, so I make no claim about its exact placeholder prompts.

genXmlPlaceholder resolved the placeholder type through PLACEHOLDER_TYPES
twice. The enum is keyed by friendly name (image -> 'pic', table -> 'tbl')
while PlaceholderProps.type is typed as the OOXML value itself, so 'pic' and
'tbl' missed the lookup and the type attribute was omitted entirely: the
layout emitted a bare <p:ph idx="N"/>. ECMA-376 declares that attribute
'use="optional" default="obj"', so the requested placeholder type was lost.
Only the four types whose enum key equals its value (title/body/chart/media)
survived the round trip.

Resolve the value once, accepting the documented OOXML value and, for untyped
JS callers, the enum key as well. Adds a demo master exercising pic/tbl.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant