Skip to content

Fix text margin arrays to use documented TRBL order - #1475

Open
guiwoda wants to merge 4 commits into
gitbrent:masterfrom
guiwoda:fix/text-margin-trbl
Open

Fix text margin arrays to use documented TRBL order#1475
guiwoda wants to merge 4 commits into
gitbrent:masterfrom
guiwoda:fix/text-margin-trbl

Conversation

@guiwoda

@guiwoda guiwoda commented Aug 10, 2026

Copy link
Copy Markdown

Summary

  • Text margin arrays are documented as TRBL [top, right, bottom, left], and table/slide-number margins already map that way, but textObjectToXml still used the old LRBT mapping (lIns ← margin[0], tIns ← margin[3]).
  • That swap turns intended left padding into a top inset. Callers that convert CSS padding-left (e.g. hanging-indent bullet lists via dom-to-pptx) can then overflow a box whose height was measured without that inset.
  • Issue TRBL margin for text object #551 claimed this was fixed in v3.0; docs were updated to TRBL, but the text-body implementation was not.

Test plan

Per TESTING.md:

  • npm run ship
  • Browser or Node demos → Text slides
  • Confirm margin:[5,5,5,40] box shows a large left inset (not top)
  • Confirm bullet example with margin:[0,0,0,10] has left padding only

guiwoda and others added 2 commits August 10, 2026 12:40
textObjectToXml was still mapping margin as LRBT (lIns←[0], tIns←[3]), so left padding became a top inset and could overflow tightly sized bullet boxes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Leave running the regression via `node --test` without adding a new npm script.

Co-authored-by: Cursor <cursoragent@cursor.com>
@guiwoda

guiwoda commented Aug 10, 2026

Copy link
Copy Markdown
Author

Dropped the package.json test script (and incidental lockfile touch) — that was out of scope. The regression still lives at test/text-margin-trbl.test.mjs and can be run with:

node --test test/*.test.mjs

guiwoda and others added 2 commits August 10, 2026 12:48
Per project guidelines, contributors should only change source; maintainers rebuild dist.

Co-authored-by: Cursor <cursoragent@cursor.com>
Match project testing (demos/modules + TESTING.md): add an asymmetric text-margin case and annotate the existing left-margin bullet example.

Co-authored-by: Cursor <cursoragent@cursor.com>
@guiwoda

guiwoda commented Aug 10, 2026

Copy link
Copy Markdown
Author

Updated testing to match TESTING.md / the PR template:

  • Removed the standalone test/*.mjs unit test
  • Added a TRBL text-margin case in demos/modules/demo_text.mjs (asymmetric margin:[5,5,5,40] — left inset should be large, not top)
  • Annotated the existing bullet margin:[0,0,0,10] example

Verify with the Text demos after npm run ship (browser demo or demos/node npm run demo).

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