refactor(painters/dom): drop list-item fragment renderer (SD-2851)#3269
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Preserve list-item dispatch until all producers are removed
renderFragment now throws for any list-item fragment, but the list-item model is still produced elsewhere in the pipeline (layout-bridge/src/incrementalLayout.ts and layout-resolved/src/resolveLayout.ts still handle kind: 'list-item', and ListItemFragment remains in contracts). That means any remaining caller/flow that emits legacy list fragments will hard-fail painting at runtime instead of degrading gracefully, which is a behavior regression introduced by this commit.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Hi @luccas-harbour! Just a couple of comments here |
+1 here. |
|
hey @VladaHarbour @tupizz! thanks for the review! I just wanted to flag one thing about your comments on how there are issues with removing support to This has actually already been the case ever since the layout-engine was launched. The reason that that code was still there is because I was woking on unifying paragraphs and lists at the same time that Nick was building the layout engine. I finished my work and it got merged to main but the layout-engine still kept the code that was handling the old ListItem node (even though it didn't exist anymore). This refactor project was only focused on the painter layer so I didn't actually remove the I think a follow-up ticket would be removing Can you re-review after having this information? |
List rendering is now unified through the paragraph fragment path, so the list-item-specific renderer and its supporting helpers are no longer reachable. Removes: - `renderListItemFragment` and `applyResolvedListItemWrapperFrame` from `renderer.ts`, along with the `LIST_MARKER_GAP` constant and the `stripListIndent` helper. - list-item branches from `fragmentKey`, `deriveBlockVersion`, and the fragment-frame dispatch (now treats `list-item` as unsupported). - list-item handling in `computeBetweenBorderFlags` (border grouping now only pairs `para` fragments). - list-item fixtures and assertions from `between-borders.test.ts`, `renderer-dispatch.test.ts`, and `index.test.ts`. - the now-stale `renderer-known-divergences.test.ts`.
…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 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.
a156110 to
dd41af0
Compare
…-block-rendering refactor(painters/dom): unify image block rendering (SD-2838)
…endering refactor(painters/dom): unify SDT container rendering (SD-2838)
…raph-rendering refactor(painters/dom): unify paragraph rendering across body and table cells (SD-2838)
Agent docs auditFound deterministic findings on 1 changed agent-doc item(s).
|
…arker (SD-2656) Resolves conflicts after main's painter-dom refactor (#3269 / SD-2851): * shared/common/list-marker-utils.ts — combined main's new fields (color, letterSpacing, vanish) with our caps marks (allCaps, smallCaps) on MinimalMarkerRun. * packages/layout-engine/painters/dom/src/utils/marker-helpers.ts — accepted main's deletion; createListMarkerElement moved to paragraph/list-marker.ts. * packages/layout-engine/painters/dom/src/renderer.ts — accepted main's removal of WordLayoutMarker type and the renderListItemFragment / renderDropCap methods. Both were superseded by the new paragraph/list-marker.ts and the resolved-fragment pipeline. * packages/layout-engine/painters/dom/src/paragraph/list-marker.ts — ported our caps mark handling (textTransform: uppercase for w:caps, fontVariant: small-caps for w:smallCaps) to the new MarkerRunStyle type and createListMarkerElement implementation. * packages/layout-engine/layout-resolved/src/resolveParagraph.ts — refreshed JSDoc reference to the file's new home. Tests: - 1136 painter-dom pass - 1252 layout-bridge pass + 1 skip - 657 layout-engine pass
### Bug Fixes - copy-pasted text in suggestion mode (#3576) - center virtualized matches after mount in find nav (SD-3315) - stop find navigation jumping to the reverted caret (SD-3315) - honor the focus() contract + fix dangling docs reference (SD-3312) - don't re-center visible matches on find navigation (SD-3315) - report 'zoom' not 'mixed' for a zoom repaint (SD-3311) - wire pointer-source tracking on all init paths; update export snapshot - reset state on unload, dedupe + export payload types, add core tests - skip empty block SDT content selection - type modules.contentControls exactly (no pass-through index) - allow block SDT wrapper deletion to follow lock rules - promote image-bearing inline SDT wrappers to inline-block - keep block sdt fill behind content - hide block sdt fills in output modes - paint block SDT background on chrome layer - use logical inset for inline SDT label position - anchor inline SDT label to start of chrome - route smartTag through export and preserve smartTagPr in SDT flatten (SD-2647) - include inline SDT chrome width in block SDT bounds - keep block SDT chrome at full fragment width - render and round-trip w:smartTag content (SD-2647) - decouple base64 image helper imports - preserve block ids during metadata updates - ignore covered sdt label clicks - sync block sdt label selection updates - show empty SDT placeholder text in viewing and print modes - skip empty sdt scan on arrow right - preserve permission-only sdt placeholders - preserve comment-only sdt placeholders - preserve empty sdt bookmark placeholders - collapse hidden sdt placeholder text - keep sdt placeholder pm range atomic - trust empty sdt paragraph conversion - keep vanished sdt paragraph side effects - preserve vanished block sdt paragraphs - suppress hidden block sdt chrome - keep remeasured sdt placeholder atomic - transform sdt placeholder measure - remeasure sdt placeholders - hide sdt placeholders in print - hide sdt placeholders in viewing - ignore collapsed inline sdt cut - expose block sdt appearance - hide empty block sdt placeholder - align empty block sdt caret - use measured width for empty SDT placeholders - size SDT block labels to content width - collapse selection on sdtContentLocked delete - allow history transactions through sdt lock - drop unreachable move fallback - target marker-only textblock end - ignore empty block sdt key targets - cap block sdt label width - handle sdt marker gaps and block atoms - skip hidden field annotations in sdt navigation - skip hidden metadata sdt markers - skip hidden block sdt markers - handle marker-only sdt paragraphs - keep visible atoms in sdt navigation - skip hidden sdt navigation markers - target nearest sdt cursor position - respect inline atoms in sdt navigation - handle empty block sdt navigation - avoid restoring dragged block to its source position - exclude sdt chrome labels from caret position lookup - keep text-align enabled in locked SDT paragraphs - block disabled toolbar execution - guard unlisted locked toolbar commands - block locked sdt toolbar execution - reject malformed base64 image data URIs - reuse colliding data uri media targets - enforce upload byte cap for data uris - avoid non-image data uri extensions - reject raw raster data uri dimensions - validate oversized async svg images - register preset raster data uris in place - narrow sdt metadata overrides - normalize image data uri extensions - centralize image data url policy - reuse target image relationships - validate in-place svg payloads - validate field annotation data uri exports - reject malformed data uri files - avoid duplicate image rids - block raw raster data uri exports - reject malformed svg data uri payloads - warn on skipped image exports - reject separatorless data uri files - block non-image data uri exports - read svg data uri dimensions - share data uri media parsing - skip invalid data uri image targets - normalize svg data uri filenames - validate in-place svg image data - guard non-base64 data uri exports - export field annotation svgs as svg - allow non-base64 SVG data URLs in image rendering - mirror in-place image media to parent - decode non-base64 data URI exports - extract shared hash helpers - reuse data URI image exports - support non-base64 data URI images in registration - scope inline SDT placeholder to structuredContent metadata - register sized SVG data URI images without canvas processing - persist data URI images set via setPresetContent - share SDT lock predicates - version inline image metadata - align RTL SDT chrome to text - size SDT chrome for justified lines - honor ancestor image SDT locks - dirty inline image SDT changes - align SDT chrome within paragraph width - preserve SDT chrome continuation offsets - offset block SDT chrome for indents - suppress SDT pseudo hover in viewing mode - allow top-aligned inline images - fit block SDT chrome to actual content width - bottom-align text on lines with inline images - keep block SDT chrome and inline images out of paragraph geometry - disable image resize inside content-locked SDTs - detect inline image run changes in paragraph diff - mark block SDT selected when contained image is selected - select inline SDT content as text on Delete - intercept beforeinput insertText at inline SDT boundaries - delete contentLocked SDT wrapper in one step - bump sdBlockRev on ancestors of inline edits - select inline SDT content as text on Backspace - select inline SDT on Backspace at start of following run (SD-3165) - share structured content chrome label set - resolve block labels at node boundary - use contract label selectors - clear label gesture state on cancel - scope label clicks to owning editor - defer label selection to mouseup so native drag still fires - share structured content label classes - avoid deferred block label retry - select labels with active editor - focus editor after label selection - correct cursor placement and label interactions for structured content - handle cell-level SDT in vMerge column lookup (SD-3289) - preserve cell-level SDT wrapping table cells (SD-3289) - preserve recipient identity attrs on replay (SD-3279) - accept legacy fingerprints in compare/apply (SD-3279) - drop misleading pnpm run type-check hint from audit - structural-fail on missing dist + sync check-jsdoc header - strip session-local sdBlockId from diff fingerprint (SD-3279) - autoFit table width overflow from cell preferences (#3522) - flip public-method-coverage to strict-zero gate (SD-673) - flip jsdoc-hygiene-ts to strict-zero gate (SD-673) - rename jsdoc-hygiene-ts self-tests + wire into CI (fixes vitest discovery on #3511) - jsdoc-hygiene-ts handles private-identifier symbols + README - more bugs - add ui for overlapping delete, other fixes - replacement pair - remaining collab bugs - coalesce tracked inserts across run gaps - restore tracked change comment interactions - expose tracked mark predicate option - more cases - tc fixes - collab mode bug - floating comments fixes - make jsdoc-hygiene-ts baseline key line-independent + update wrapper docs - preserve tab underline via runProperties fallback in collab ### Changes - Merge branch 'main' into caio/sd-3315-find-replace-scroll - Merge pull request #3509 from superdoc-dev/artem/SD-3232 - Merge pull request #3555 from superdoc-dev/luccas/delete-image-content-locked-sdt - Merge branch 'main' into artem/SD-3159 - Merge pull request #3550 from superdoc-dev/luccas/sd-3302-bug-sdt-in-template-builder-shows-grey-background-highlight - Merge branch 'main' into artem/SD-3159 - SD-2676 - fix: table selection not providing a feedback (#3508) - Merge pull request #3549 from superdoc-dev/luccas/left-align-inline-sdt-label - Merge pull request #3546 from superdoc-dev/caio/sd-2647-bug-render-and-round-trip-content-wrapped-in-wsmarttag - Merge remote-tracking branch 'origin/stable' into sync/stable-to-main-20260527-230540 - Merge pull request #3539 from superdoc-dev/caio-pizzol/sd-3289-preserve-cell-level-sdt - Merge pull request #3527 from superdoc-dev/caio-pizzol/sd-3279-strip-sdblockid-from-diff-fingerprint - Merge branch 'main' into caio-pizzol/SD-js-contract-owner-audit - Merge pull request #3531 from superdoc-dev/caio-pizzol/SD-docs-snippet-typecheck-2 - Merge pull request #3526 from superdoc-dev/caio-pizzol/SD-runtime-payload-tests - Merge pull request #3521 from superdoc-dev/caio-pizzol/SD-public-method-coverage-strict-zero - Merge branch 'stable' - Merge pull request #3485 from superdoc-dev/artem/SD-3200 - Merge pull request #3517 from superdoc-dev/caio-pizzol/SD-jsdoc-hygiene-zero-flip - Merge pull request #3513 from superdoc-dev/caio-pizzol/SD-jsdoc-hygiene-cleanup - Merge branch 'stable' - Merge pull request #3511 from superdoc-dev/caio-pizzol/SD-jsdoc-hygiene-scanner - Merge pull request #3435 from superdoc-dev/nick/sd-3220-overlapping-suggestion-contract - Merge pull request #3429 from superdoc-dev/artem/underlined-tab-collab ### Documentation - note content controls in entityAt hit types (SD-3313) - clarify data uri buffer conversion - document image data uri helpers - clarify image registration comments - sync README exit semantics for js contract-owner audit - clarify wrapper-stages prose + add ts-jsdoc to check:public summary ### Features - add ui.contentControls.focus to place the caret in a control (SD-3312) - add ui.viewport.observe geometry-invalidation signal (SD-3311) - add ui.contentControls.scrollIntoView (SD-3310) - add activePath (full active stack) to content-control:active-change - expose public sdt events - select adjacent block SDT content at textblock boundaries - inherit run styles in empty block SDT placeholders - render placeholder text for empty SDTs - move caret into following block sdt on delete - move caret into preceding block sdt on backspace - disable mutation toolbar controls inside content-locked SDTs - render empty inline SDTs as a visible placeholder - add modules.contentControls.chrome - js contract-owner audit, report-only (SD-673) - add snippet typecheck for editor/superdoc/** + fix stale examples (SD-673) - add non-hover field color for sdt (#3506) - comments and tc on small screen (#3446) - overlapping tracked changes - add type-bearing JSDoc hygiene gate for .ts source (SD-673) - add anchored metadata orphan status ### Tests - resolve handleBase64 source path from package or repo root - cover locked block SDT Delete selection - cover nested block SDT boundary selection - update SDT keymap chain coverage - cover contentControlsChrome plumbing; clarify chrome-none comment - assert chrome-none hover suppression and cascade order - add v2 bridge unit + round-trip behavior coverage (SD-2647) - assert smartTag child text survives export round-trip (SD-2647) - cover nested block sdt navigation - roundtrip mixed image block sdts - cover image data uri length boundary - repaint saved sdt images through painter - cover structured content image edges - cover inline image diff fields - cover locked inline SDT beforeinput - cover inline SDT Cmd+X selection - cover inline SDT content Delete flow - clarify inline SDT boundary lock comment - cover two-step inline SDT Backspace - cover inline SDT selection meta escape - use real production payload for list-definitions-change bridge (SD-673) - tighten list-definitions-change bridge assertion (SD-673) - pin list-definitions-change bridge and DELETED comments shape (SD-673) - pin runtime event payload shapes (SD-673) ### Refactoring - consolidate type imports in dom painter - share block sdt navigation helpers - share box model between block and inline sdt labels - share structured content predicates - wrap shared tryDecodeDataUriText re-export - reuse shared data uri export policy - share image relationship export lookup - centralize image data uri parsing - share data uri text decoding - trim data uri metadata fields - extract CHECKED_FILES to shared module + drop audit from wrapper - drop list-item fragment renderer (SD-2851) (#3269) - drain 85 type-bearing JSDoc entries from .ts source (SD-673) ### Performance - avoid scanning data uri media ### Chores - 1.37.0 [skip ci] - fix import breaking delinstrtext orphans (#3535) - fix tests (#3533) - fixes - more fixes - run generate:all and fixes - more fixes - soec fixes - type fixes - ui and more - fix regression - type fixes - more fixes - more fixes - ci fixes - type fixes - add dispatch test for collab bug - review fix, type fix - add tests for metadata issue - tests for review issues
|
🎉 This PR is included in superdoc-cli v0.15.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.14.0 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.10.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.38.0 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.9.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.10.0 |
6 participants
Summary
Removes the now-unreachable
list-itemfragment path from the DOM painter. List rendering already flows through the paragraph fragment path (the PM adapter no longer registersorderedList/bulletListhandlers — list paragraphs go throughparagraphToFlowBlocks()asParagraphBlockwith marker metadata), so the dedicatedlist-itemrenderer and its helpers were dead weight duplicating paragraph behavior.What changed
In
packages/layout-engine/painters/dom/src/renderer.ts:renderListItemFragmentandapplyResolvedListItemWrapperFrame.LIST_MARKER_GAPconstant and thestripListIndenthelper.list-itembranches fromfragmentKey,deriveBlockVersion, and the fragment-frame dispatch —list-itemis now treated as unsupported (throws infragmentKeyandrenderFragment).computeBetweenBorderFlagsinfeatures/paragraph-borders/group-analysis.tsto pairparafragments only.Tests:
renderer-known-divergences.test.ts(the file existed to track list-item parity bugs; obsolete now).between-borders.test.ts,renderer-dispatch.test.ts, andindex.test.ts.test-utils/normalize-line.tsto reflect the remaining contexts (body, table-cell).Body↔table-cell justify
wordSpacingparity that incidentally lived in the deleted divergence file is still covered byrenderer-parity-contracts.test.ts:310-387. Lists rendered through the paragraph path are exercised byrenderer-marker-suffix.test.ts,renderer-marker-textwidth.test.ts, and the multi-line list integration test atindex.test.ts:1266.What's intentionally NOT in this PR
The
ListBlock/ListItemFragmenttypes and their producers incontracts/,layout-bridge/,layout-resolved/, andpm-adapter/sdt/are still present. They're now dead feeds (the painter rejects what they produce), but unwinding them touches multiple packages and is better as a follow-up so this PR stays focused on the painter cleanup.Follow-ups
ListBlock,ListMeasure, andListItemFragmentfrompackages/layout-engine/contracts/src/index.tsand propagate the removal throughlayout-bridge(cache.ts,incrementalLayout.ts),layout-resolved(resolveLayout.ts,versionSignature.ts), andpm-adapter/sdt/.RENDERING_DUPLICATION_AUDIT.md: extracting a shared paragraph content renderer that body and table-cell paths both consume.