Repository navigation
Conversation
Visual PR DiffCompared Renderer Change Ranking
Missing baseline coverage: theme-style-refs (missing-baseline) Most Improved Vs Office Reference
No renderer deltas were detected in this PR. Open the published reports for full slide-by-slide inspection. Artifacts include HTML side-by-side reports under |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbbdbc07c3
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function shouldApplyThemeLineFallback(sp: Element): boolean { | ||
| const ln = sp.querySelector(":scope > spPr > ln"); | ||
| if (!ln) return true; | ||
|
|
||
| return !ln.querySelector(":scope > noFill"); |
There was a problem hiding this comment.
Prevent line fallback from overriding explicit line fill
shouldApplyThemeLineFallback currently enables style-matrix fallback for any <a:ln> that does not contain <a:noFill>, so shapes with explicit line fills like <a:gradFill> or <a:pattFill> still get lnRef defaults applied. In this commit, parseShape/parseTextShapeFrame then use that fallback when parseStrokeColor cannot parse the explicit non-solid fill, which silently replaces the authored line style with theme color/dash/width. This regresses fidelity for non-solid line definitions and should be gated the same way as fill fallback (only when no explicit line fill content exists).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68b3b19e6a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fillType: themeFill?.fillType, | ||
| gradient: themeFill?.gradient, | ||
| strokeColor: parseStrokeColor(sp, theme.colors) || themeLine?.strokeColor, | ||
| strokeWidth: parseStrokeWidth(sp) || themeLine?.strokeWidth, |
There was a problem hiding this comment.
Preserve explicit zero-width lines over theme fallback
The strokeWidth fallback uses parseStrokeWidth(sp) || themeLine?.strokeWidth, so an explicit OOXML line width of w="0" is treated as missing and replaced by the lnRef theme width. This causes shapes with an intentionally zero-width line to render a border when a style matrix line ref is present. Please use nullish fallback (??) so 0 remains authoritative; the same pattern is also present in parseTextShapeFrame.
Useful? React with 👍 / 👎.
Summary
useBgFillornoFilltheme-style-refsvisual case to measure this fidelity surface end to endValidation
pnpm run fixtures:generatepnpm run verifypnpm run pack:checkpnpm run test:browserPPTX_VISUAL_CASES=complex-sanitized,theme-colors,theme-luminance,theme-style-refs pnpm run test:visualVisual Notes
complex-sanitizedremains stable at0.2895mean difftheme-colorsto0.0462andtheme-luminanceto0.0523theme-style-refscase improves from0.1345onmainto0.0214on this branch