feat(painter-dom): custom SDT styling variables under chrome:'none' (SD-3322)#3590
Conversation
…SD-3322) Under modules.contentControls.chrome:'none' the painter erased the SDT look entirely, so a consumer who wanted a custom field/clause appearance had to target the painted wrapper with !important and reach into internal state classes (.ProseMirror-selectednode, .sdt-group-hover) to keep it stable across hover and selection. That's the wrong "best practice" to teach. Make the chrome-none reset read a --sd-content-controls-custom-* variable layer with default-preserving fallbacks (0-width transparent border, no background / radius / padding). chrome:'none' stays visually empty by default - existing consumers see no change - but a consumer can now paint inline and block controls by setting variables on a data-sdt-* selector. The painter applies them across rest, hover, and selected, so the box stays stable (no jitter) and no !important or state-class selectors are needed. `border` is a full shorthand; block adds a `-border-left` accent rail; background vars cascade (hover from rest, selected from hover). - variables.css: document the custom-* surface; note the built-in chrome still uses the existing --sd-content-controls-* variables. - docs: add a "Style the controls in place" section to the custom-UI content controls guide. - test: assert the surface is wired and default-preserving; existing chrome-none selector + source-order tests are unchanged and still pass (painter-dom 1178/1178).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba1f8490d6
ℹ️ 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".
There was a problem hiding this comment.
cubic analysis
2 issues found across 4 files
Linked issue analysis
Linked issue: SD-3322: feat(ui): first-class custom styling for content controls under chrome:none
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Expose a --sd-content-controls-custom-* styling surface that the painter reads under chrome:'none' | The painter now reads --sd-content-controls-custom-* vars with empty defaults so chrome:'none' stays visually empty by default; styles.ts adds the selectors and variables, and a unit test asserts the CSS contains the expected var(...) lines. |
| ✅ | Apply custom styling for rest, hover, and active/selected states without requiring consumers to target internal state classes or use !important | The painter re-asserts the border in every state to avoid jitter and uses cascading background vars (rest -> hover -> selected). Consumers set variables on the painted wrapper (e.g. via data-sdt-*) and do not need to target .ProseMirror-selectednode or add !important. |
| ✅ | Support styling for both inline and block controls (including an optional block accent rail) | Inline and block variables and fallbacks are present; block supports a -border-left override for an accent rail. |
| Cover locked state visuals | The CSS includes selectors that respect data-lock-mode for hover/group-hover, so hover behavior in locked mode is handled, but there are no dedicated --sd-content-controls-custom-*-locked-* variables exposed in the variables list or docs to let consumers style a distinct locked state. | |
| ✅ | Docs explain the split between built-in chrome theming and the new chrome:'none' custom path | Documentation was updated with examples and explicit guidance: built-in chrome uses --sd-content-controls-* while chrome:'none' consumers should use the new --sd-content-controls-custom-* variables on painted wrappers. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…' (SD-3322) The custom hover background was overridden for LOCKED controls under chrome:'none'. The base lock-hover rules (a built-in tint on inline, transparent on block) have equal specificity to the plain custom hover rules but come later in source order, so they won; the chrome-none lock-hover reset only reset z-index, not background. Re-assert the custom hover background in that reset block - it carries the extra .superdoc-cc-chrome-none class, so it outranks the base lock-hover rules. A locked control now follows --sd-content-controls-custom-*-hover-bg. With no custom var set the default is empty, so the built-in lock-hover tint no longer leaks under chrome:'none' for locked controls (consistently empty). Only the contract-templates demo has locked chrome-none controls, and it wants the custom hover, not the tint. Add a regression test asserting the custom hover vars are re-asserted after the base lock-hover rules (source order = it wins). painter-dom 1179/1179 green.
…s (SD-3322) The content-controls theming table themes the built-in chrome. Add a one-line note that under chrome:'none' you style controls with the --sd-content-controls-custom-* variables instead, linking the custom UI guide.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…D-3322) Rewrite the contract-templates demo's SDT styling onto SuperDoc's public --sd-content-controls-custom-* variables (from #3590), proving the new API in the real legal-template use case. The demo now styles its inline fields and block clauses with zero !important and zero internal state selectors (.ProseMirror-selectednode, .sdt-group-hover); the painter applies the variables across rest, hover, selected, and locked-hover. This is the copy-pasteable pattern for styling custom SDTs under chrome:'none'. - style.css: replace the per-state !important rules with one variable-setting rule per tag (inline + block); update the host-owned-styling comment. - test: add state coverage - the custom hover background drives a painted field (and wins over the built-in lock-hover tint), the border stays constant across states (no jitter), and no built-in label/chrome leaks. Demo suite 13/13. - docs (Document API > Content controls): correct the contentLocked wording (it rejects Document API content writes too, not just the editor); document the locked-template pattern (unlock -> write -> relock, incl. a locked parent for nested fields); add the single-use governed clause-library pattern alongside versioned reusable sections (kept - it's a valid pattern). - docs (Custom UI > Content controls): add a "Build a custom field system" walkthrough; describe the demo as a full custom contract-template UI. - README: note the demo styles through the public custom variables. Stacked on #3590 (the painter variable layer); retarget to main once it merges.
…ty (SD-3066) (#3565) * fix(super-converter): bibliography/index/TOA field-code import fidelity (SD-3066) Real Word documents surfaced several import defects in the block field-code family (BIBLIOGRAPHY, INDEX, XE, TOA). This addresses them and unifies the shared shape behind one set of helpers. Fixes: - Multi-run instruction aggregation joined fragments with an injected separator space, corrupting instructions Word splits across runs (e.g. `XE " Building Standard "`). Join verbatim; literal spacing is preserved. - Table of Authorities was dropped on import: the v2 importer had no `sd:tableOfAuthorities` handler, so the node was silently discarded. Register tableOfAuthoritiesImporter alongside index/bibliography. - A content control wrapping a block field imported as an inline `structuredContent` node; inside a block-only documentPartObject this threw "Invalid content for node type documentPartObject" and the editor failed to mount. Classify an SDT whose content is a block field as structuredContentBlock. - Bibliography neither captured nor replayed instructionTokens (unlike index/toa), so split BIBLIOGRAPHY instructions did not round-trip. Add the attribute and wire it through preprocessor, encode and decode. Refactors (DRY/KISS): - Extract buildBlockFieldNode (shared by the bibliography/index/toa preprocessors) and wrapParagraphsAsComplexField (shared by their translator decoders), mirroring the existing inline-field helpers. - Centralize BLOCK_FIELD_XML_NAMES so the paragraph importer and SDT classifier agree on which sd:* nodes are block content. Adds RED→GREEN unit tests for each fix and the new shared helpers. Full super-converter suite passes (2990). Linear: SD-3066 (parent of SD-3005) * fix(pm-adapter): render bibliography/index/TOA inside documentPartObject (SD-3066) (#3566) Word wraps a generated bibliography (and other block fields) in a docPartObject SDT, sometimes via a nested content control. The handler only converted paragraph and tableOfContents children to flow blocks, so the field's entry paragraphs were silently dropped — the heading rendered but the entries did not. - documentPartObject now renders bibliography/index/tableOfAuthorities children via the shared paragraph-container handler, and structuredContentBlock children via their handler. - structuredContentBlock recurses block-field children (transparent wrapper), rendering their paragraphs. Section-counting invariant: findParagraphsWithSectPr recurses bibliography (so the handler advances currentParagraphIndex per entry) but not structuredContentBlock (so the scb path renders without advancing). Both paths were validated against the invariant; the prior code dropped entries AND under-counted, which could drift section breaks. Also extracts the shared handleParagraphContainerNode used by the bibliography, index and tableOfAuthorities handlers (previously three byte-identical copies). Regression tests cover both nesting shapes and the counter behavior. pm-adapter suite passes (302); layout corpus comparison shows no regressions attributable to this change. Linear: SD-3066 * feat(painter-dom): custom SDT styling variables under chrome:'none' (SD-3322) Under modules.contentControls.chrome:'none' the painter erased the SDT look entirely, so a consumer who wanted a custom field/clause appearance had to target the painted wrapper with !important and reach into internal state classes (.ProseMirror-selectednode, .sdt-group-hover) to keep it stable across hover and selection. That's the wrong "best practice" to teach. Make the chrome-none reset read a --sd-content-controls-custom-* variable layer with default-preserving fallbacks (0-width transparent border, no background / radius / padding). chrome:'none' stays visually empty by default - existing consumers see no change - but a consumer can now paint inline and block controls by setting variables on a data-sdt-* selector. The painter applies them across rest, hover, and selected, so the box stays stable (no jitter) and no !important or state-class selectors are needed. `border` is a full shorthand; block adds a `-border-left` accent rail; background vars cascade (hover from rest, selected from hover). - variables.css: document the custom-* surface; note the built-in chrome still uses the existing --sd-content-controls-* variables. - docs: add a "Style the controls in place" section to the custom-UI content controls guide. - test: assert the surface is wired and default-preserving; existing chrome-none selector + source-order tests are unchanged and still pass (painter-dom 1178/1178). * fix(painter-dom): custom hover wins on locked SDTs under chrome:'none' (SD-3322) The custom hover background was overridden for LOCKED controls under chrome:'none'. The base lock-hover rules (a built-in tint on inline, transparent on block) have equal specificity to the plain custom hover rules but come later in source order, so they won; the chrome-none lock-hover reset only reset z-index, not background. Re-assert the custom hover background in that reset block - it carries the extra .superdoc-cc-chrome-none class, so it outranks the base lock-hover rules. A locked control now follows --sd-content-controls-custom-*-hover-bg. With no custom var set the default is empty, so the built-in lock-hover tint no longer leaks under chrome:'none' for locked controls (consistently empty). Only the contract-templates demo has locked chrome-none controls, and it wants the custom hover, not the tint. Add a regression test asserting the custom hover vars are re-asserted after the base lock-hover rules (source order = it wins). painter-dom 1179/1179 green. * docs(theming): point chrome:'none' styling at the custom SDT variables (SD-3322) The content-controls theming table themes the built-in chrome. Add a one-line note that under chrome:'none' you style controls with the --sd-content-controls-custom-* variables instead, linking the custom UI guide. * demo/docs: contract-templates use the custom SDT styling variables (SD-3322) Rewrite the contract-templates demo's SDT styling onto SuperDoc's public --sd-content-controls-custom-* variables (from #3590), proving the new API in the real legal-template use case. The demo now styles its inline fields and block clauses with zero !important and zero internal state selectors (.ProseMirror-selectednode, .sdt-group-hover); the painter applies the variables across rest, hover, selected, and locked-hover. This is the copy-pasteable pattern for styling custom SDTs under chrome:'none'. - style.css: replace the per-state !important rules with one variable-setting rule per tag (inline + block); update the host-owned-styling comment. - test: add state coverage - the custom hover background drives a painted field (and wins over the built-in lock-hover tint), the border stays constant across states (no jitter), and no built-in label/chrome leaks. Demo suite 13/13. - docs (Document API > Content controls): correct the contentLocked wording (it rejects Document API content writes too, not just the editor); document the locked-template pattern (unlock -> write -> relock, incl. a locked parent for nested fields); add the single-use governed clause-library pattern alongside versioned reusable sections (kept - it's a valid pattern). - docs (Custom UI > Content controls): add a "Build a custom field system" walkthrough; describe the demo as a full custom contract-template UI. - README: note the demo styles through the public custom variables. Stacked on #3590 (the painter variable layer); retarget to main once it merges.
…D-3322) Rewrite the contract-templates demo's SDT styling onto SuperDoc's public --sd-content-controls-custom-* variables (from superdoc-dev#3590), proving the new API in the real legal-template use case. The demo now styles its inline fields and block clauses with zero !important and zero internal state selectors (.ProseMirror-selectednode, .sdt-group-hover); the painter applies the variables across rest, hover, selected, and locked-hover. This is the copy-pasteable pattern for styling custom SDTs under chrome:'none'. - style.css: replace the per-state !important rules with one variable-setting rule per tag (inline + block); update the host-owned-styling comment. - test: add state coverage - the custom hover background drives a painted field (and wins over the built-in lock-hover tint), the border stays constant across states (no jitter), and no built-in label/chrome leaks. Demo suite 13/13. - docs (Document API > Content controls): correct the contentLocked wording (it rejects Document API content writes too, not just the editor); document the locked-template pattern (unlock -> write -> relock, incl. a locked parent for nested fields); add the single-use governed clause-library pattern alongside versioned reusable sections (kept - it's a valid pattern). - docs (Custom UI > Content controls): add a "Build a custom field system" walkthrough; describe the demo as a full custom contract-template UI. - README: note the demo styles through the public custom variables. Stacked on superdoc-dev#3590 (the painter variable layer); retarget to main once it merges.
Under
modules.contentControls.chrome: 'none', the painter erased the SDT look entirely, so customizing a field or clause meant targeting the painted wrapper with!importantand reaching into internal state classes (.ProseMirror-selectednode,.sdt-group-hover) to keep it stable across hover and selection. That is the wrong best practice to teach. This adds a--sd-content-controls-custom-*variable layer that the chrome-none reset reads, so a consumer paints their own look by setting variables on adata-sdt-*selector. The painter applies them across rest, hover, and selected, so the box stays stable and no!importantor state-class selectors are needed.0 solid transparentis layout-identical toborder: none(the computed border-style differs, but nothing renders and nothing shifts). Existing consumers see no change.borderis a full CSS shorthand (e.g.1px solid #1355ff). Block controls add a-border-leftaccent rail. Background vars cascade: hover from rest, selected from hover.chrome: 'default') is untouched and still uses the existing--sd-content-controls-*variables.!important+ state selectors for these variables) is a follow-up, once this lands and ships insuperdoc.Review: block custom styling here is wrapper-level (element border/background). Built-in block chrome uses
::before/::afterpartly to join multi-fragment block SDTs; this first pass styles the element, which is correct for single-paragraph clauses but does not join borders across fragments of a multi-fragment block. Either accept that as v1 or track pseudo-level custom block chrome as a follow-up.Verified:
cd packages/layout-engine/painters/dom && npx vitest run-> 1178/1178, including a new test that the surface is wired and default-preserving; the existing chrome-none selector and source-order tests are unchanged.