refactor(css): introduce CSS variables for maintainability#8
Open
codaMW wants to merge 4 commits intoMostroP2P:mainfrom
Open
refactor(css): introduce CSS variables for maintainability#8codaMW wants to merge 4 commits intoMostroP2P:mainfrom
codaMW wants to merge 4 commits intoMostroP2P:mainfrom
Conversation
- Added :root block with semantic color variables - Added transition timing variables - Replaced 50+ hardcoded color values with CSS variables - Replaced transition values with reusable variables - Zero visual changes - purely internal refactoring - Improves theme customization and maintainability
WalkthroughIntroduces a theme-driven refactor of Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
style/styles.css (3)
1-29: Well-structured variable definitions.The
:rootblock is cleanly organized with semantic grouping. A few observations:
The transition shorthand variables (
all .13s) omit the timing function, defaulting toease. This is fine but worth being intentional about — consider documenting the impliciteaseor adding it explicitly for clarity.Several hardcoded color values remain unconverted throughout the file (e.g.,
#000on lines 282/290,#cccon line 659,#000on line 61). For full consistency with this refactor's goals, consider adding variables like--text-blackand--text-mutedto capture these.Colors used with alpha (e.g.,
rgb(29 33 44 / 50%)on line 161) can't directly reference these variables without decomposing them into channel components or usingcolor-mix(). This is a known CSS limitation and acceptable to leave as-is for now.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` around lines 1 - 29, Add explicit variables in the :root block and make transition timing explicit: define new color variables (e.g., --text-black for `#000` and --text-muted for `#ccc`) and replace occurrences across the stylesheet with these names, and change transition variables (--transition-fast, --transition-normal, --transition-slow) to include an explicit timing function (for example "all .13s ease") so the shorthand no longer relies on the implicit default; update usages of these symbols throughout the CSS to reference --text-black/--text-muted and the updated --transition-* variables to maintain consistency.
657-661: Missed hardcoded color:#cccin.faq-answer.Line 659 uses
color:#ccc`` which could be captured in a CSS variable (e.g.,--text-muted) for consistency with the refactoring goals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` around lines 657 - 661, The .faq-answer rule uses a hardcoded color (`#ccc`); replace that with a CSS variable (e.g., color: var(--text-muted)) and ensure the variable --text-muted is defined in your root or theme variables (or reuse the existing text-muted variable name if one exists) so the color adheres to the refactor for centralized color tokens; update the .faq-answer selector to reference var(--text-muted) instead of `#ccc` and add/verify the variable in :root or your central variables file.
7-7: Near-duplicate color variables:--green-primaryvs--green-sponsor.
--green-primary:#8CC63Fand `--green-sponsor: `#8CC541differ by only ~1 unit in green and blue channels (rgb(140,198,63) vs rgb(140,197,65)), making them visually indistinguishable. Unless these track distinct brand assets, consider consolidating to reduce the variable surface area.Also applies to: 11-11
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` at line 7, Two CSS color variables, --green-primary and --green-sponsor, are nearly identical (rgb(140,198,63) vs rgb(140,197,65)); either consolidate them into a single variable (pick one name, replace references to the other across the codebase) or choose a visually distinct hex for one to reflect separate semantics; update all usages of --green-primary and --green-sponsor accordingly (search for these variable names) and remove the redundant variable if consolidating.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@style/styles.css`:
- Around line 674-701: The added responsive media queries (breakpoints 1280px,
1024px, 480px) and their rules (.navLang, .logo, .container, h1, h2, h3 and the
other responsive blocks referenced) introduce layout/UX changes beyond the "CSS
variables only" refactor; remove these new media-query blocks from this change
and move them into a separate PR (or a new CSS file/commit) dedicated to
responsive layout updates so the current PR remains visual-change-free and
reviewable; ensure you keep only variable replacements in this PR and preserve
the original styles for .navLang/.logo/.container/h1/h2/h3 here, then create a
follow-up PR containing the full responsive rule sets for 1280px/1024px/480px.
- Around line 25-28: The transition CSS variables (--transition-normal,
--transition-fast, --transition-slow) include the keyword "all", which breaks
uses like "transition: background var(--transition-normal)" and "transition:
color var(--transition-fast)"; change these variables to represent only
durations/timing (e.g., --transition-normal-duration: .3s;
--transition-fast-duration: .13s;) and optionally add separate full-transition
variables if you need the shorthand (e.g., --transition-normal-all: all .3s;);
then update uses—replace "transition: background var(--transition-normal)" with
"transition: background var(--transition-normal-duration)" or "transition:
var(--transition-normal-all)" and similarly replace "transition: color
var(--transition-fast)" with the appropriate duration or full-transition
variable.
- Around line 516-525: Remove the vendor-prefixed gradient declaration in the
`#footer`:after rule: delete the background: -webkit-linear-gradient(...) line and
keep the existing unprefixed background: linear-gradient(...) only; also sweep
for the same pattern (e.g., the .copyright gradient) and remove any
-webkit-linear-gradient occurrences to satisfy Stylelint's
value-no-vendor-prefix rule.
- Around line 316-329: Remove the dead utility classes .margin-top and
.margin-bottom from style/styles.css since they are not used anywhere; keep only
the actively used .text-center definition (or alternatively, add these margin
utilities only when you actually introduce elements that use .margin-top or
.margin-bottom). Update the CSS by deleting the .margin-top and .margin-bottom
blocks (leaving .text-center intact) so the new stylesheet does not introduce
unused rules or unexpected visual changes.
- Around line 117-149: You added a global heading reset and explicit h1–h3
styles (selectors h1,h2,h3,h4,h5,h6 and individual h1,h2,h3 rules) which will
change site-wide typography; either revert these global rules if the PR must
make no visual changes, or restrict them by scoping (e.g., apply under a
root/container class like .site or .app: .site h1, .site h2, etc.), and ensure
they don’t override component-specific selectors such as .faq-section h2 by
checking and adjusting specificity or moving component rules after these
globals.
---
Nitpick comments:
In `@style/styles.css`:
- Around line 1-29: Add explicit variables in the :root block and make
transition timing explicit: define new color variables (e.g., --text-black for
`#000` and --text-muted for `#ccc`) and replace occurrences across the stylesheet
with these names, and change transition variables (--transition-fast,
--transition-normal, --transition-slow) to include an explicit timing function
(for example "all .13s ease") so the shorthand no longer relies on the implicit
default; update usages of these symbols throughout the CSS to reference
--text-black/--text-muted and the updated --transition-* variables to maintain
consistency.
- Around line 657-661: The .faq-answer rule uses a hardcoded color (`#ccc`);
replace that with a CSS variable (e.g., color: var(--text-muted)) and ensure the
variable --text-muted is defined in your root or theme variables (or reuse the
existing text-muted variable name if one exists) so the color adheres to the
refactor for centralized color tokens; update the .faq-answer selector to
reference var(--text-muted) instead of `#ccc` and add/verify the variable in :root
or your central variables file.
- Line 7: Two CSS color variables, --green-primary and --green-sponsor, are
nearly identical (rgb(140,198,63) vs rgb(140,197,65)); either consolidate them
into a single variable (pick one name, replace references to the other across
the codebase) or choose a visually distinct hex for one to reflect separate
semantics; update all usages of --green-primary and --green-sponsor accordingly
(search for these variable names) and remove the redundant variable if
consolidating.
Comment on lines
674
to
701
|
|
||
| /* 1280px breakpoint */ | ||
| @media screen and (max-width: 1280px) { | ||
| .navLang { | ||
| right: 20px; | ||
| } | ||
|
|
||
| .logo { | ||
| left: 20px; | ||
| } | ||
|
|
||
| .container { | ||
| max-width: 980px; | ||
| padding: 0 20px; | ||
| } | ||
|
|
||
| h1 { | ||
| font-size: 52px; | ||
| } | ||
|
|
||
| h2 { | ||
| font-size: 58px; | ||
| } | ||
|
|
||
| h3 { | ||
| font-size: 32px; | ||
| } | ||
| } |
There was a problem hiding this comment.
New responsive breakpoints go beyond the stated "CSS variables only" refactor scope.
The 1280px, 1024px, and 480px media query blocks are entirely new additions (not variable replacements). The 1024px breakpoint alone introduces significant layout changes: reduced header height, mobile navigation restructuring, font-size scaling, hero section reflow, and sponsor banner adjustments. These are substantive responsive design changes that could affect user experience.
While these may be valuable improvements, they contradict the PR's claim of "zero visual changes" and should ideally be split into a separate PR for clearer review and regression tracking. Bundling layout changes with a theming refactor makes it harder to isolate the cause of any visual regressions.
Also applies to: 703-848, 950-964
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@style/styles.css` around lines 674 - 701, The added responsive media queries
(breakpoints 1280px, 1024px, 480px) and their rules (.navLang, .logo,
.container, h1, h2, h3 and the other responsive blocks referenced) introduce
layout/UX changes beyond the "CSS variables only" refactor; remove these new
media-query blocks from this change and move them into a separate PR (or a new
CSS file/commit) dedicated to responsive layout updates so the current PR
remains visual-change-free and reviewable; ensure you keep only variable
replacements in this PR and preserve the original styles for
.navLang/.logo/.container/h1/h2/h3 here, then create a follow-up PR containing
the full responsive rule sets for 1280px/1024px/480px.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
style/styles.css (2)
7-11: Verify whether--green-sponsoris intentionally distinct from--green-primary.
--green-primary:#8CC63Fand `--green-sponsor: `#8CC541differ by only 1 in the green channel and 2 in the blue channel — a nearly imperceptible contrast (ΔE < 1). If these are meant to be the same hue, alias one to the other to avoid silent drift when updating brand colors.♻️ Proposed consolidation (if same color is intended)
--green-primary: `#8CC63F`; --green-secondary: `#9CD84F`; --green-dark: `#304F00`; --green-hover: `#739C3D`; - --green-sponsor: `#8CC541`; + --green-sponsor: var(--green-primary);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` around lines 7 - 11, The two CSS variables --green-primary and --green-sponsor are effectively identical (nearly imperceptible difference); confirm intended behavior and if they should be the same then remove the duplicate hex and make --green-sponsor reference --green-primary (use var(--green-primary)) so future brand updates stay in sync; if they must differ, pick a clearly distinct hex or add a comment explaining why --green-sponsor intentionally differs from --green-primary to avoid accidental drift.
31-33: Legacy "all-inclusive" transition variables create an ongoing footgun alongside the new duration-only variables.The comment says these are kept "for backward compatibility where all properties are needed," but every current standalone use (
transition: var(--transition-fast), etc.) already works correctly. Having two parallel APIs —--transition-duration-*(safe for composition) and--transition-fast/normal/slow(must be used standalone) — means a future contributor addingtransition: color var(--transition-normal)will silently produce invalid CSS (transition: color all .3s), recreating the exact bug that was fixed in the previous commit. Iftransition: all Xis genuinely needed, document the distinction explicitly or drop these three variables entirely.♻️ Proposed cleanup (remove legacy vars; all current usages are standalone and already valid without them)
/* Transitions */ --transition-duration-fast: .13s; --transition-duration-normal: .3s; --transition-duration-slow: 1s; - - /* Keep original for backward compatibility where all properties are needed */ - --transition-fast: all .13s; - --transition-normal: all .3s; - --transition-slow: all 1s;Then replace standalone usages with explicit
allform:- transition: var(--transition-fast); + transition: all var(--transition-duration-fast);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` around lines 31 - 33, Remove the legacy all-inclusive CSS variables (--transition-fast, --transition-normal, --transition-slow) from the stylesheet and replace any usage of them by either (a) composing with the duration-only variables (e.g., change "transition: var(--transition-normal)" to "transition: all var(--transition-duration-normal)" when the intent is to animate all properties) or (b) using the duration variable directly in composed transitions (e.g., "transition: color var(--transition-duration-normal)") to avoid the silent "all" injection; search for occurrences of --transition-fast/normal/slow and update each use accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@style/styles.css`:
- Around line 656-666: The h2 font-size is larger than h1 at the 1280px and
1024px breakpoints causing inverted heading hierarchy; update the CSS rules for
the h1 and h2 selectors at those breakpoints so h1 has the larger font-size and
h2 the smaller one (i.e., swap the values currently applied to h1 and h2 for the
1280px and 1024px media-query blocks) to restore correct visual/semantic order.
---
Duplicate comments:
In `@style/styles.css`:
- Around line 641-667: The new media query blocks for 1280px, 1024px, and 480px
(the `@media` rules that modify .navLang, .logo, .container, h1, h2, h3 and other
layout/sponsor/header rules) introduce visual/layout changes and must be removed
from this "CSS variables only" refactor; revert or extract those entire
breakpoint blocks from style/styles.css and move them into a separate follow-up
PR so this change set only contains variable replacements, leaving symbols like
.navLang, .logo, .container, h1, h2, and h3 unaffected in this branch.
---
Nitpick comments:
In `@style/styles.css`:
- Around line 7-11: The two CSS variables --green-primary and --green-sponsor
are effectively identical (nearly imperceptible difference); confirm intended
behavior and if they should be the same then remove the duplicate hex and make
--green-sponsor reference --green-primary (use var(--green-primary)) so future
brand updates stay in sync; if they must differ, pick a clearly distinct hex or
add a comment explaining why --green-sponsor intentionally differs from
--green-primary to avoid accidental drift.
- Around line 31-33: Remove the legacy all-inclusive CSS variables
(--transition-fast, --transition-normal, --transition-slow) from the stylesheet
and replace any usage of them by either (a) composing with the duration-only
variables (e.g., change "transition: var(--transition-normal)" to "transition:
all var(--transition-duration-normal)" when the intent is to animate all
properties) or (b) using the duration variable directly in composed transitions
(e.g., "transition: color var(--transition-duration-normal)") to avoid the
silent "all" injection; search for occurrences of --transition-fast/normal/slow
and update each use accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
style/styles.css (1)
30-33: Optional: rename the all-property shortcuts to distinguish them from duration-only tokens.The comment "backward compatibility" is misleading — these variables aren't compatibility shims; they're the full
transitionshorthand form. A contributor picking a variable for, say,transition: color var(--transition-fast)would unknowingly producetransition: color all .13s(invalid). A clearer naming convention avoids that footgun:♻️ Proposed naming change
- /* Keep original for backward compatibility where all properties are needed */ - --transition-fast: all .13s; - --transition-normal: all .3s; - --transition-slow: all 1s; + /* Full shorthand — use only as transition: var(--transition-all-*) */ + --transition-all-fast: all .13s; + --transition-all-normal: all .3s; + --transition-all-slow: all 1s;And update all call sites (e.g.,
transition: var(--transition-fast)→transition: var(--transition-all-fast)).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` around lines 30 - 33, The CSS token names --transition-fast / --transition-normal / --transition-slow are full transition shorthands and should be renamed to make that explicit (e.g., --transition-all-fast, --transition-all-normal, --transition-all-slow) and the misleading "backward compatibility" comment removed; update all call sites that currently do things like transition: var(--transition-fast) to use transition: var(--transition-all-fast) (or switch to a duration-only variable if the intent is just the timing value).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@style/styles.css`:
- Around line 840-850: h1 and h2 are both set to 38px at the 640px breakpoint,
flattening heading hierarchy; update the CSS so h1 remains larger than h2 by
reducing h2's font-size (or increasing h1) within the same breakpoint rules that
contain the h1 and h2 selectors (refer to the h1 and h2 rules and the
.main-heading rule) so the visual hierarchy h1 > h2 is enforced at 640px.
- Around line 652-662: Add explicit base rules for the h1, h2, and h3 selectors
before any max-width media queries so desktop/large-viewport sizes are defined
(e.g., set intended desktop font-size for h1, h2, h3) to avoid smaller browser
defaults on wide screens; then keep the existing max-width media query overrides
for narrower viewports so the responsive sizes still apply. Target the h1/h2/h3
selectors that currently only appear inside the max-width media queries and add
the desktop/base font-size declarations above those queries.
---
Duplicate comments:
In `@style/styles.css`:
- Around line 637-663: The new media query blocks (e.g., `@media` screen and
(max-width: 1280px) and the similar 1024px/480px blocks) introduce visual/layout
changes — adjusting .navLang, .logo, .container, h1, h2, h3, hero and sponsor
styles — which violates the CSS-variables-only, zero-visual-changes requirement;
revert these additions or move only variable substitutions (no direct
size/padding/position changes) into the stylesheet by replacing hard-coded
values with existing CSS variables used globally (or add new variables if
needed) and remove any direct overrides that change layout (selectors to check:
.navLang, .logo, .container, h1, h2, h3, mobile nav/hero/sponsor selectors
mentioned in the diff and the other affected ranges).
---
Nitpick comments:
In `@style/styles.css`:
- Around line 30-33: The CSS token names --transition-fast / --transition-normal
/ --transition-slow are full transition shorthands and should be renamed to make
that explicit (e.g., --transition-all-fast, --transition-all-normal,
--transition-all-slow) and the misleading "backward compatibility" comment
removed; update all call sites that currently do things like transition:
var(--transition-fast) to use transition: var(--transition-all-fast) (or switch
to a duration-only variable if the intent is just the timing value).
Comment on lines
+652
to
+662
| h1 { | ||
| font-size: 58px; | ||
| } | ||
|
|
||
| h2 { | ||
| font-size: 52px; | ||
| } | ||
|
|
||
| h3 { | ||
| font-size: 32px; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if base h1/h2/h3 styles exist outside media queries
echo "=== All h1/h2/h3 rules in CSS files ==="
fd -e css | while read f; do
if rg 'h[123]\s*\{|h[123]\s*,' "$f" >/dev/null 2>&1; then
echo "File: $f"
rg -n 'h[123]\s*\{|h[123]\s*,' "$f" 2>/dev/null
fi
done
echo ""
echo "=== Checking context around h1/h2/h3 rules (checking if inside `@media` blocks) ==="
fd -e css -exec cat -n {} + | grep -A 2 -B 2 'h[123]\s*\{'Repository: MostroP2P/MostroP2P.github.io
Length of output: 951
🏁 Script executed:
#!/bin/bash
# Check context of h1/h2/h3 rules in style/styles.css to see if inside `@media` blocks
echo "=== Full context around lines 640-700 (around first h1 at 652) ==="
sed -n '640,700p' style/styles.css
echo ""
echo "=== Full context around lines 770-810 (around second set at 777+) ==="
sed -n '770,810p' style/styles.css
echo ""
echo "=== Full context around lines 830-860 (around third set at 840+) ==="
sed -n '830,860p' style/styles.css
echo ""
echo "=== Check if there are any h1/h2/h3 rules BEFORE line 640 ==="
sed -n '1,639p' style/styles.css | tail -50Repository: MostroP2P/MostroP2P.github.io
Length of output: 2835
🏁 Script executed:
#!/bin/bash
# Check for base h1/h2/h3 styles in guide-styles.css and HTML files
echo "=== guide-styles.css lines 201-230 (check for base h1/h2/h3 rules outside `@media`) ==="
sed -n '201,230p' guide/guide-styles.css
echo ""
echo "=== Check if guide-styles.css has h1/h2/h3 rules BEFORE line 201 ==="
sed -n '1,200p' guide/guide-styles.css | grep -n 'h[123]\s*{'
echo ""
echo "=== Search HTML files for inline styles or <style> tags with h1/h2/h3 ==="
fd -e html -x sh -c 'if grep -l "h[123]" "$1" >/dev/null 2>&1; then echo "=== File: $1 ==="; grep -n "<style" "$1" || echo "(no <style> tag)"; fi' _ {}
echo ""
echo "=== Check if any HTML has inline style attributes on h1/h2/h3 elements ==="
fd -e html -x grep -n 'style\s*=' {} + | grep -i 'h[123]'Repository: MostroP2P/MostroP2P.github.io
Length of output: 7014
Add base h1/h2/h3 font sizes outside media queries to prevent inverted responsive sizing.
The stylesheet defines h1/h2/h3 font sizes only inside max-width media queries. At viewports wider than 1280px, browser defaults apply (typically ~32px for h1), while at ≤1280px, custom sizes apply (58px for h1). This inverts expected responsive behavior: headings are larger on narrower screens than on wide ones.
Add base rules before the media queries to establish desktop sizing:
Fix — add base h1/h2/h3 styles
+h1 { font-size: 72px; }
+h2 { font-size: 64px; }
+h3 { font-size: 40px; }
+
/* 1280px breakpoint */
`@media` screen and (max-width: 1280px) {
h1 { font-size: 58px; }
h2 { font-size: 52px; }
h3 { font-size: 32px; }
}(Adjust base values to match intended desktop design.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@style/styles.css` around lines 652 - 662, Add explicit base rules for the h1,
h2, and h3 selectors before any max-width media queries so
desktop/large-viewport sizes are defined (e.g., set intended desktop font-size
for h1, h2, h3) to avoid smaller browser defaults on wide screens; then keep the
existing max-width media query overrides for narrower viewports so the
responsive sizes still apply. Target the h1/h2/h3 selectors that currently only
appear inside the max-width media queries and add the desktop/base font-size
declarations above those queries.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
style/styles.css (4)
849-851: Remove the inline dev annotation before merging.The
/* WAS 38px, NOW 34px - smaller than h1 */comment is a development note, not a useful long-term CSS comment. The hierarchy fix itself (h1=38px, h2=34px) is correct and properly addresses the previous flag — the comment can go.♻️ Proposed cleanup
h2 { - font-size: 34px; /* WAS 38px, NOW 34px - smaller than h1 */ + font-size: 34px; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` around lines 849 - 851, Remove the development annotation comment in the h2 rule so only the intended style remains; locate the h2 selector (h2 { font-size: 34px; /* WAS 38px, NOW 34px - smaller than h1 */ }) and delete the inline comment, leaving just the font-size declaration to keep the CSS clean.
248-261: Hardcoded#000in the expanded hamburger state wasn't migrated to a variable.Lines 253 and 261 both set
background:#000`` but no black is defined in:root. Consider a semantic token:♻️ Proposed refactor
:root { /* Text colors */ --text-light: `#fff`; --text-gray: `#92949A`; --text-dark: `#373737`; + --text-contrast: `#000`; }.menu-button.expand span:before { top: 0px; -webkit-transform: rotate(45deg); -ms-transform: rotate(45deg); transform: rotate(45deg); - background: `#000`; + background: var(--text-contrast); } .menu-button.expand span:after { top: 0px; -webkit-transform: rotate(-45deg); -ms-transform: rotate(-45deg); transform: rotate(-45deg); - background: `#000`; + background: var(--text-contrast); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` around lines 248 - 261, The two background declarations in the selectors .menu-button.expand span:before and .menu-button.expand span:after use a hardcoded `#000`; add a semantic CSS variable (e.g., --icon-black or --icon-contrast) in :root and replace those background values with var(--icon-black) (optionally with a fallback like var(--icon-black, `#000`)) so the expanded hamburger uses the theme token instead of a hardcoded color.
618-622:.faq-answercolor:#ccc`` was not migrated to a CSS variable.
#ccc(#CCCCCC) is visually distinct from--text-gray:#92949A`` (darker), so a direct substitution would change appearance. A new semantic token is the clean fix:♻️ Proposed refactor
:root { --text-gray: `#92949A`; + --text-muted: `#ccc`; }.faq-answer { padding: 0 24px 20px 24px; - color: `#ccc`; + color: var(--text-muted); line-height: 1.7; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` around lines 618 - 622, The .faq-answer rule uses a hardcoded color `#ccc` instead of a CSS variable; add a new semantic variable (for example --text-muted: `#CCCCCC`) alongside existing tokens (e.g., in :root or the theme vars) and replace color: `#ccc` in the .faq-answer selector with color: var(--text-muted) so the FAQ styling uses the new token and remains visually identical while supporting theming.
132-132: Several semi-transparent backgrounds still use hardcodedrgb()/rgba()values that correspond to defined palette variables.
Location Hardcoded value Maps to header(line 132)rgb(29 33 44 / 50%)--bg-dark.dark-section(line 302)rgb(16 18 25 / 50%)--bg-darker.copyrightgradient (line 438)rgba(16, 18, 25, 1/0)--bg-darker#footer:aftergradient (line 485)rgb(29 33 44 / 0%/100%)--bg-dark.sponsor-banner(line 493)rgb(16 18 25 / 60%)--bg-darker.faq-item(line 576)rgb(16 18 25 / 50%)--bg-darkerCSS custom properties can't supply per-channel values inside
rgb()without CSS Relative Color Syntax (rgb(from var(--bg-dark) r g b / 50%)). If the project targets Chrome 119+/Firefox 128+/Safari 16.4+, RCS is an option; otherwise, consider defining opacity-variant tokens directly in:root:♻️ Alternative: opacity-variant tokens
:root { --bg-dark: `#1D212C`; --bg-darker: `#101219`; + + /* Semi-transparent variants */ + --bg-dark-50: rgb(29 33 44 / 50%); + --bg-darker-50: rgb(16 18 25 / 50%); + --bg-darker-60: rgb(16 18 25 / 60%); }Then, e.g.:
header { - background: rgb(29 33 44 / 50%); + background: var(--bg-dark-50);Also applies to: 302-302, 438-438, 493-493, 576-576
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style/styles.css` at line 132, Replace the hardcoded semi-transparent colors in the listed selectors (header, .dark-section, .copyright, `#footer`:after, .sponsor-banner, .faq-item) with palette-aware variables: either use CSS Relative Color Syntax (e.g., rgb(from var(--bg-dark) ... / 50%) or rgb(from var(--bg-darker) ... / 60%)) if your browser targets support it, or add opacity-variant tokens in :root (e.g., --bg-dark-50, --bg-darker-60) that represent the appropriate semi-transparent colors derived from --bg-dark / --bg-darker and then replace the hardcoded rgb()/rgba() usages with those new variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@style/styles.css`:
- Around line 636-663: The 1280px media-query and its rules (.navLang, .logo,
.container, h1, h2, h3) introduce layout and typography changes beyond the
intended "CSS variables only" scope; remove these non-variable rules from this
media-query block and revert the file to include only CSS variable adjustments
here, or move the layout/typography rules into a dedicated responsive-layout
change (separate PR or stylesheet) so visual behavior stays unchanged in this
variables-only PR. Ensure the media query in this PR contains only variable
declarations (no selectors like .navLang, .logo, .container or h1/h2/h3) and add
the removed rules to the new responsive PR or file for proper regression review.
- Around line 651-662: Add base heading rules for selectors h1, h2, and h3 in
styles.css outside any `@media` block so desktop (wide) viewports use the intended
sizes, then move or change the existing `@media` (max-width: 1280px) overrides to
set smaller sizes for narrower screens; specifically ensure the top-level
selectors h1, h2, h3 define the large/default font-sizes and the media query
only reduces them for <=1280px so sizing no longer inverts.
---
Nitpick comments:
In `@style/styles.css`:
- Around line 849-851: Remove the development annotation comment in the h2 rule
so only the intended style remains; locate the h2 selector (h2 { font-size:
34px; /* WAS 38px, NOW 34px - smaller than h1 */ }) and delete the inline
comment, leaving just the font-size declaration to keep the CSS clean.
- Around line 248-261: The two background declarations in the selectors
.menu-button.expand span:before and .menu-button.expand span:after use a
hardcoded `#000`; add a semantic CSS variable (e.g., --icon-black or
--icon-contrast) in :root and replace those background values with
var(--icon-black) (optionally with a fallback like var(--icon-black, `#000`)) so
the expanded hamburger uses the theme token instead of a hardcoded color.
- Around line 618-622: The .faq-answer rule uses a hardcoded color `#ccc` instead
of a CSS variable; add a new semantic variable (for example --text-muted:
`#CCCCCC`) alongside existing tokens (e.g., in :root or the theme vars) and
replace color: `#ccc` in the .faq-answer selector with color: var(--text-muted) so
the FAQ styling uses the new token and remains visually identical while
supporting theming.
- Line 132: Replace the hardcoded semi-transparent colors in the listed
selectors (header, .dark-section, .copyright, `#footer`:after, .sponsor-banner,
.faq-item) with palette-aware variables: either use CSS Relative Color Syntax
(e.g., rgb(from var(--bg-dark) ... / 50%) or rgb(from var(--bg-darker) ... /
60%)) if your browser targets support it, or add opacity-variant tokens in :root
(e.g., --bg-dark-50, --bg-darker-60) that represent the appropriate
semi-transparent colors derived from --bg-dark / --bg-darker and then replace
the hardcoded rgb()/rgba() usages with those new variables.
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
1 participant
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.
Summary
This PR introduces CSS variables (custom properties) for consistent theming without changing any visual appearance or layout behavior.
Changes
:rootblock with organized color palette variablesBenefits
Testing Done
Before/After
Visual appearance is identical. Only internal code structure changed.
Future Work
This lays the groundwork for potential future enhancements:
Happy to make any requested changes!
Summary by CodeRabbit