fix: emit pic and tbl placeholder types - #1526
Open
adamstankiewicz wants to merge 1 commit into
Open
Conversation
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.
adamstankiewicz
force-pushed
the
fix/placeholder-pic-tbl-type
branch
from
August 19, 2026 01:08
928dd72 to
87ffeb1
Compare
2 tasks
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
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.
Submission Guidelines
src/*.ts(nodistorsrc/bld)src/core-interfaces.tsandtypes/index.d.tsare unchangeddemos/modules/Change Summary
picandtblplaceholders never receive atypeattribute in the generated XML, so the layout emits a bare<p:ph idx="N"/>. ECMA-376 declarestypeasuse="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 throughPLACEHOLDER_TYPEStwice:The enum is keyed by friendly name, but its values are the OOXML ones:
PlaceholderProps.typeis typed as the OOXML value, so a caller passes'pic'.PLACEHOLDER_TYPES['pic']isundefined, 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 atsrc/gen-objects.ts#L335.Verified output
Every accepted input, before and after, read from
ppt/slideLayouts/slideLayout2.xml:typepassedtitle/body/chart/mediapictype="pic"tbltype="tbl"image/table(enum keys)type="pic"/type="tbl"undefinedVerified in a consumer
Two decks built from a master with unfilled
title+pic+tblplaceholders, one frommasterand one from this branch, both opened in Google Slides:picboxtblboxSo the type now reaches the consumer: Slides renders a real picture placeholder, and for
tblit reports the type as one it does not implement rather than silently treating it as text. Worth flagging that second cell — with this change atblplaceholder 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 fromtype: 'tbl'.Reproduce with:
Change Type
Related Issue
Follow-on to #921, which corrected the
PlaceholderProps.typedeclaration 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:
PLACEHOLDER_TYPEonly 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.pic/tblcallers. Anyone who passedtype: 'pic'and then filled it withaddText()was relying on the type being dropped. That seems very unlikely to be deliberate, but it is a change.Checklist before requesting a review
/demos/modules/(aPLACEHOLDER_TYPES_SLIDEmaster plus a demo slide leavingpic/tblunfilled)npx eslint src/gen-xml.ts demos/modules/masters.mjs demos/modules/demo_master.mjsclean;tsc --noEmitclean)gulp build && gulp bundle, withdemos/browser/index.htmlpointed at./js/pptxgen.bundle.jsand../modules/demos.mjsinstead 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 whoseslideLayout6.xmlcontains<p:ph idx="101" type="pic"/>and<p:ph idx="102" type="tbl"/>. Also run under Node (node demo.js master).