fix(templates): make the CV template's second hardcoded color themeable - #3699
Conversation
theme-style.mjs's STYLE_VAR_MAP exposes only accent_color, font_family, font_size, and margin, but cv-template.html actually uses two distinct colors: --accent-color (themeable) and a second color hardcoded as the literal hsl(270, 70%, 45%) for company/institution names and the header gradient's second stop. That second color was invisible to the theming system, so a style: override (e.g. accent_color: "#000000" for a monochrome CV) could never fully recolor the CV. Adds a secondary_color token mapping to --secondary-color, gives it a :root default matching the previous hardcoded value, and replaces every hardcoded hsl(270, 70%, 45%) in cv-template.html, cv-template.zh-minimal.html and resume-template.html (the two other shipped templates with the same pattern) with var(--secondary-color). No rendered-output change without a profile override. Closes #3695 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
I see this, @Schlaflied, and the diagnosis holds: |
Scott-Emberson
left a comment
There was a problem hiding this comment.
Reviewed at head 246c4b7. The owned test tests/theme-style.test.mjs pins this with a literal-count guard rather than a generic var() check, which is the robust way to do it. Across the three templates it requires the hsl(270, 70%, 45%) literal to survive only as the :root default (exactly one occurrence), so the moment any rule reverts to the hardcoded purple the count goes above one and it reddens; it also requires var(--secondary-color) to be present and guards the dead-fallback circular case. The diff confirms every previously-hardcoded occurrence (.job-org, .edu-org, .cert-org, .award-org, and the header gradient in both directions) is swapped to the variable, so the count-equals-one invariant genuinely binds those rules. The style-token round trip gained a secondary_color assertion mirroring accent_color, and the earlier zh-TW / ja / ko / zh CJK regression blocks are outside the diff and unchanged.
The substance here is templates/cv-template.html, outside our owned paths, so this is a test-axis review: the owned test is sound and the revert-detection is real, but the merge of the template change is the maintainer's call. Nothing blocking from the tests/ side.
|
Another solid one, @Schlaflied. Promoting the second hardcoded colour to |
What does this PR do?
theme-style.mjs's "Themeable tokens" feature (#1837) exposes exactly 4style:keys inSTYLE_VAR_MAP:accent_color,font_family,font_size,margin. Buttemplates/cv-template.htmlactually uses two distinct colors, not one —--accent-color(themeable) and a second color hardcoded as the literalhsl(270, 70%, 45%), used directly (not through any CSS variable) in.job-company,.project-title,.edu-org,.cert-org,.award-org, and the.header-gradientbackground's second stop (plus itshtml[lang="ar"]RTL mirror).Because that second color was never routed through a CSS custom property, it was invisible to the theming system — a user asking for a monochrome CV via
style: { accent_color: "#000000" }still got purple company names and a purple header gradient. This PR addssecondary_color→--secondary-colortoSTYLE_VAR_MAP, gives it a:rootdefault matching the previous hardcoded value, and replaces every hardcoded occurrence incv-template.htmlwithvar(--secondary-color).templates/cv-template.zh-minimal.htmlandtemplates/resume-template.htmlhad the identical hardcoded-purple pattern (verified by grep — no other shipped template does), so the same fix is applied there for consistency. Neither of those two templates currently exposes--accent-colortheming at all (that's a separate, pre-existing gap, out of scope here), so each gets its own minimal:root { --secondary-color: ... }declaration rather than reusingcv-template.html's block.templates/cover-letter-template.htmland the other CV variants (compact/executive/jake/leadership/modern) don't use this color at all and are untouched.Related issue
Closes #3695
Type of change
Details
theme-style.mjssecondary_color: '--secondary-color'toSTYLE_VAR_MAP.style:block.templates/cv-template.html--secondary-color: hsl(270, 70%, 45%)to the existing:roottheme-defaults block.hsl(270, 70%, 45%)occurrences withvar(--secondary-color): the header gradient's second stop, itshtml[lang="ar"]RTL mirror, and the.job-company/.project-title/.edu-org/.cert-org/.award-orgcolors.templates/cv-template.zh-minimal.html,templates/resume-template.html:root { --secondary-color: hsl(270, 70%, 45%); }block plusvar()substitution everywhere the literal previously appeared, including thehtml[lang="ar"]header-gradient mirror.config/profile.example.ymlsecondary_coloralongside the existing 4 tokens.tests/theme-style.test.mjsstyleTokensFromkey-count assertion 4 → 5, now also assertingsecondary_colormaps correctly.readStyleTokenscase readingsecondary_colorfrom a profile file.buildThemeStyleBlock/injectThemeStyleround-trip assertions for--secondary-color, mirroring the existing--accent-colorcoverage.cv-template.html,cv-template.zh-minimal.html, andresume-template.html, asserts each declares a--secondary-color:rootdefault, reads it viavar()everywhere, has no leftover hardcodedhsl(270, 70%, 45%), and no circularvar()default.No rendered-output change without a
style.secondary_coloroverride inconfig/profile.yml— the new:rootdefaults reproduce the exact previous hardcoded values.Checklist
node test-all.mjs— 7790 passed, 5 failed. All 5 failures are pre-existing and unrelated to this change (js-yaml is not installed at .../node_modules/js-yaml— a stalenode_modulesin this worktree checkout, affectingcv-section-order.test.mjs'slinkRepoPackagehelper and thegenerate-pdf-batch/generate-pdf-page-budgetsuites; plus one unrelatedfollowup-cadence.mjse2e crash).theme-style.mjs's own suite — which also importsjs-yamland is unaffected by the missing-module issue — passed all 24 assertions, including every newsecondary_colorcase.