refactor(painters/dom): unify paragraph rendering across body and table cells (SD-2838)#3309
Merged
harbournick merged 50 commits intoMay 26, 2026
Conversation
luccas-harbour
requested review from
VladaHarbour
and removed request for
a team and
caio-pizzol
May 20, 2026 18:46
tupizz
reviewed
May 21, 2026
tupizz
reviewed
May 21, 2026
| type MinimalWordLayout, | ||
| type ResolvedListMarkerGeometry, | ||
| } from '@superdoc/common/list-marker-utils'; | ||
| import { applySourceAnchorDataset } from '../renderer.js'; |
Contributor
There was a problem hiding this comment.
This import creates a runtime cycle through the new extraction: renderer.ts imports renderParagraphFragment, which reaches renderParagraphContent, which imports this module, which then imports back from renderer.ts. It also makes the paragraph helper depend on the renderer orchestration module for a small DOM utility. Please move applySourceAnchorDataset into a shared utility module (or pass it in) so the extracted paragraph/list-marker code remains leaf-level and avoids circular initialization risk.
Contributor
Author
There was a problem hiding this comment.
That makes sense. I fixed it.
tupizz
requested changes
May 21, 2026
tupizz
left a comment
tupizz
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewed current head. The RenderParagraphContentParams test type issue remains, and I added an inline comment for a new renderer -> paragraph helper circular import.
VladaHarbour
approved these changes
May 22, 2026
…le cells Extract the paragraph-rendering pipeline (block styles, decoration layers, list markers, indentation, line walking, drop caps) into a shared paragraph/renderParagraphContent module consumed by both the body-fragment path in renderer.ts and the table-cell path in renderTableCell.ts. The two sites had drifted into parallel ~500-line implementations; collapsing them removes ~1k lines net and gives both paths the same marker/indent/border behavior. Borders and shading now live on dedicated .superdoc-paragraph-border / .superdoc-paragraph-shading layers (already used by the body path) instead of being stamped onto the cell-paragraph wrapper, so renderTableCell tests were updated to assert against those layers.
Move `features/paragraph-borders/` to `paragraph/borders/` and fold `utils/marker-helpers.ts` into `paragraph/list-marker.ts` so all paragraph rendering pieces live under `paragraph/`. Update the feature registry and imports to match.
Pull line- and run-level rendering out of renderer.ts (~2500 lines) into a dedicated runs/ directory split by concern: text, image, math, tab, field annotation, links, SDT, tracked changes, formatting marks, and the top-level render-line/render-run orchestrators. Run hash helpers move from paragraph-hash-utils.ts to runs/hash.ts. Public re-exports (RenderedLineInfo, sanitizeUrl, linkMetrics, applyRunDataAttributes) now resolve through runs/index.ts. No behavior change — pure colocation of the run pipeline ahead of unifying body and table-cell paragraph rendering.
…agraph/ module Moves paragraph fragment rendering, block versioning, and PM attribute application out of renderer.ts into dedicated paragraph/ modules: - paragraph/renderParagraphFragment.ts — fragment rendering entry point - paragraph/block-version.ts — block version derivation + table-version hash - paragraph/frame.ts — PM attribute application on the paragraph element Trims renderer.ts by ~490 lines with no behavior change.
…tests The contextSection option no longer exists on renderParagraphContent inputs, so the 'body' value in these test setups is dead weight.
Move SDT boundary computation, dataset application, snapshot collection, and inline wrapper helpers out of renderer.ts into dedicated modules under sdt/. Renames runs/sdt.ts to sdt/inline.ts and re-exports remain stable. No behavior change.
Extract image <img> element creation, clip-path resolution, and hyperlink-anchor wrapping into shared helpers under `painters/dom/src/images/`, and route the renderer's block/drawing image paths plus renderTableCell's flowing, drawing, and anchored image paths through them. Eliminates five near-identical copies of the same img setup (objectFit, cover→left-top, clip-path, filters, hyperlink wrap) so behavior stays consistent across all image surfaces. Adds coverage for clip-path, filters, and hyperlinks on both flowing and anchored images inside table cells (SD-2838).
Removes the now-dead deriveBlockVersion machinery from renderer.ts (paragraph/image/drawing/table version hashing, SDT metadata helpers, list-marker guards) along with renderedBlockImageVersion in image-block.ts. No remaining call sites consume these helpers after the image-block rendering unification.
Move image fragment and drawing image creation out of renderer.ts into dedicated modules under images/. Renderer now delegates: - renderImageFragment → images/image-fragment.ts (with the buildImageGeometryTransform / applyImageGeometryTransform helpers it owned) - createDrawingImageElement, createShapeGroupImageElement, and the inline shape-text image element creation → images/drawing-image.ts No behavior change — pure extraction. Trims ~157 lines from renderer.ts.
Move image-clip-path and image-selectors (plus their tests) from utils/ into the images/ directory alongside image-block, drawing-image, hyperlink, and image-fragment. Update import paths in callers. No behavior change.
Updates the comment to reflect that the painter-dom copy has been removed — this is now the single source of truth, not a duplicate to keep in sync.
luccas-harbour
force-pushed
the
luccas/sd-2851-remove-list-item-code-from-painter
branch
from
May 22, 2026 18:49
a156110 to
dd41af0
Compare
luccas-harbour
force-pushed
the
luccas/sd-2838-unify-paragraph-rendering
branch
from
May 22, 2026 18:49
3572c77 to
5e1e81b
Compare
…-block-rendering refactor(painters/dom): unify image block rendering (SD-2838)
…endering refactor(painters/dom): unify SDT container rendering (SD-2838)
harbournick
merged commit May 26, 2026
2bf27fa
into
luccas/sd-2851-remove-list-item-code-from-painter
5 checks passed
Contributor
Agent docs auditFound deterministic findings on 1 changed agent-doc item(s).
|
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
5 participants
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.
Summary
Collapses two parallel paragraph-rendering implementations (body fragments in
renderer.tsand table cells inrenderTableCell.ts) into a single shared module, then reorganizes the painter so paragraph and run rendering live in dedicated directories instead of being interleaved with ~3k lines ofrenderer.ts.What changed
1. Unified paragraph pipeline (
paragraph/)New
paragraph/renderParagraphContent.tsowns the full per-paragraph pipeline — block styles, decoration layers, list markers, indentation, line walking, and drop caps — and is consumed by both the body-fragment path inrenderer.tsand the table-cell path inrenderTableCell.ts. The two sites had drifted into parallel ~500-line implementations with subtly different marker/indent/border behavior; one module makes them identical by construction.Supporting modules:
paragraph/indentation.ts— line indent and available-width resolutionparagraph/list-marker.ts— list marker rendering (absorbs the deletedutils/marker-helpers.ts)paragraph/styles.ts— block-level paragraph stylesparagraph/borders/— moved fromfeatures/paragraph-borders/so all paragraph pieces live together; feature registry updatedparagraph/renderParagraphContent.test.ts— new coverage for the resolved-paragraph pathTable-cell side effect: borders and shading now use the same
.superdoc-paragraph-border/.superdoc-paragraph-shadinglayers as the body path instead of being stamped onto the cell-paragraph wrapper.renderTableCell.test.tswas updated to assert against those layers.2. Run pipeline extracted into
runs/Pulled all line- and run-level rendering (~2500 lines) out of
renderer.tsintoruns/, split by concern:text-run,image-run,math-run,tab-run,field-annotation-runlinks,sdt,tracked-changes,formatting-marksrender-line,render-runhash— run hashing helpers moved out ofparagraph-hash-utils.tstypes,indexPublic re-exports (
RenderedLineInfo,sanitizeUrl,linkMetrics,applyRunDataAttributes) now resolve throughruns/index.ts. No behavior change in this step — pure colocation in preparation for the body/table unification.3. Shared list-marker types
shared/common/list-marker-utils.ts:MinimalMarkerRungainscolor,letterSpacing,vanishso callers don't redeclare a parallel marker-run typeMinimalMarker.justification/suffixtightened fromstringto literal unionsMinimalWordLayoutgainsindentLeftPx4. Bug fixes folded in along the way
fix(painters/dom): keep list marker right indent— right-indent was being dropped on the unified pathfix(painters/dom): keep split paragraph fragment height— split fragments lost their measured heightfix(painters/dom): mark remeasured paragraph final lines— final-line marking was missed after remeasure