fix(painters/dom, super-editor): set explicit document text color floor (SD-3456)#3525
Open
gpardhivvarma wants to merge 1 commit into
Open
Conversation
94dab3f to
0fd2ea7
Compare
Contributor
Author
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
39d4255 to
93323a0
Compare
…or (SD-3456) `.superdoc-page` (layout-engine page surface) and `.sd-editor-scoped .ProseMirror` (editor isolation surface) both relied on the browser default `color`. On dark-themed OSes that resolves to the `canvastext` system color (white), so any document text without an explicit `<w:color>` — auto-numbered list markers in particular — rendered invisible on the white page. Set an explicit `color: var(--sd-layout-page-text, #000)` floor on both surfaces. OOXML-explicit colors still win (inline `style.color` on runs has higher specificity). Consumers customize via the `--sd-layout-page-text` token, documented alongside `--sd-layout-page-bg` in variables.css. Adds CSS-content regression tests guarding both surfaces against future removal of the floor.
93323a0 to
72f1188
Compare
Contributor
Author
|
hey @caio-pizzol just wanted to remind you |
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.
Closes #3456.
Summary
.superdoc-page(layout-engine page surface) and.sd-editor-scoped .ProseMirror(editor isolation surface) both relied on the browser defaultcolor. On dark-themed OSes that resolves to thecanvastextsystem color (white), so any document text without an explicit<w:color>— auto-numbered list markers in particular — rendered invisible on the white page. There is no source CSS rule that setscolor: rgb(255,255,255)in the repo; the reporter's devtools trace pointed at.superdoc-pagebecause that was the highest-up element in the cascade where the color happened to resolve (via the user-agent default), not the rule that set it.This PR sets an explicit
color: var(--sd-layout-page-text, #000)floor on both surfaces. OOXML-explicit colors still win (inlinestyle.coloron runs has higher specificity than the inherited page color). Consumers customize via the--sd-layout-page-texttoken, documented invariables.cssalongside--sd-layout-page-bg/--sd-layout-page-shadow.Why two surfaces in one PR
.superdoc-pagecovers presentation-mode rendering (where the reported bug manifests)..sd-editor-scoped .ProseMirrorcovers the editor isolation surface for super-editor direct mounts (header/footer section editors, popovers, rich-text Field editors, etc.). Both are reverted tocanvastextbyisolation.css(all: revert) and both need the same floor. Bundling them avoids shipping the symptom-fix while leaving a sibling surface broken for the same reason.Files
packages/layout-engine/painters/dom/src/styles.tsPageStyles.color?: string;DEFAULT_PAGE_STYLES.color = 'var(--sd-layout-page-text, #000)';pageStyles()returnscolor: merged.colorin the inline-style payload.packages/layout-engine/painters/dom/src/styles.test.tsDEFAULT_PAGE_STYLESexposure.packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.csscolor: var(--sd-layout-page-text, #000)to.sd-editor-scoped .ProseMirrorwith an inline comment linking it to the page-painter fix.packages/superdoc/src/assets/styles/elements/editor-scoped-color-floor.test.jssearch-match-precedence.test.js).packages/superdoc/src/assets/styles/helpers/variables.css--sd-layout-page-text: #000;to the "Styles: layout" section.Test plan
pnpm --filter @superdoc/painter-dom test -- styles.test.ts— 8/8 (3 new + 5 existing)pnpm --filter superdoc test— 1030/1030 (4 new + 1026 existing)1.,1.1,1.1.1) are visible against the white page.color: ...property on the page element's inline style, no pixel deltas on Linux CI.Reviewer notes / consumer impact
.superdoc-page { color: white }without!importantto force dark text on the page will now lose to the inline style. The intended way to customize is--sd-layout-page-text(orpageStyles.colorvia the SuperDoc constructor).!importantoverrides still win.--sd-layout-page-textis named after the layout page but is reused inside the editor isolation surface. Considered introducing a separate--sd-editor-texttoken but kept one shared token so theme authors set it once for both surfaces.canvastextis already black, so a pixel-diff baseline would never catch this class. The CSS-content tests added here are the right safety net.Follow-ups (not in this PR)
architecture-boundaries.test.ts) enforcing "every container hosting inherited document text must declarecolorexplicitly inDEFAULT_*_STYLES."