Skip to content

Refactor light-dark theming switch to always use light-dark CSS function#425

Merged
thibaudcolas merged 1 commit intomainfrom
light-dark
Mar 11, 2026
Merged

Refactor light-dark theming switch to always use light-dark CSS function#425
thibaudcolas merged 1 commit intomainfrom
light-dark

Conversation

@thibaudcolas
Copy link
Member

Streamlining to reduce the number of overrides needed. The stylesheets use a lot of nesting, so avoiding one layer project-wide makes a big difference.

Copilot AI review requested due to automatic review settings March 11, 2026 15:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors component styling to rely on the CSS light-dark() function (and related color-scheme mechanisms) instead of .theme-dark override blocks, reducing nested selector complexity across the frontend SCSS.

Changes:

  • Replaced many hard-coded light/dark color overrides with light-dark(...) across multiple components.
  • Removed several .theme-dark & { ... } blocks in favor of scheme-aware color definitions.
  • Updated theme toggle styling logic to switch its “dark state” UI via a document-level selector.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
apps/frontend/static_src/scss/components/theme-toggle.scss Uses light-dark(...) for colors; changes dark-state selector logic for toggle UI.
apps/frontend/static_src/scss/components/section.scss Converts heading/description/icon colors to light-dark(...); removes theme-dark block.
apps/frontend/static_src/scss/components/primary-nav.scss Uses light-dark(...) for background; removes theme-dark override.
apps/frontend/static_src/scss/components/navigation.scss Uses light-dark(...) for link colors/backgrounds; removes theme-dark override.
apps/frontend/static_src/scss/components/nav-buttons.scss Adjusts link border color with light-dark(...); removes theme-dark override.
apps/frontend/static_src/scss/components/language-selector.scss Moves dropdown/toggle colors to light-dark(...); removes theme-dark override.
apps/frontend/static_src/scss/components/introduction.scss Converts text color to light-dark(...); removes theme-dark override.
apps/frontend/static_src/scss/components/heading.scss Removes theme-dark heading overrides; changes divider line to currentColor.
apps/frontend/static_src/scss/components/header.scss Converts logo/search icon/gradient colors to light-dark(...); removes theme-dark overrides.
apps/frontend/static_src/scss/components/footer.scss Converts footer background to light-dark(...); removes theme-dark override.
apps/frontend/static_src/scss/components/feedback-emoji.scss Switches SVG fill to currentColor and removes hover/focus styling.
apps/frontend/static_src/scss/components/dropdown-toggle.scss Uses light-dark(...) for caret color; removes theme-dark override.
apps/frontend/static_src/scss/components/copy-button.scss Converts button/dropdown colors to light-dark(...); removes theme-dark override.
apps/frontend/static_src/scss/components/burger.scss Converts burger line colors to light-dark(...); removes theme-dark override.
apps/frontend/static_src/scss/components/base.scss Converts body + link colors to light-dark(...); removes theme-dark override.
apps/frontend/static_src/scss/components/alert.scss Converts alert backgrounds to light-dark(...); removes theme-dark override.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

.theme-dark & {
:has([name='color-scheme'][content^='dark']) & {
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dark-state styling here relies on the :has(...) selector. Browsers without :has support will never apply these rules (so the toggle thumb position and .dark-only/.light-only visibility won’t reflect the active theme). Since the theme script already toggles body.theme-dark, consider using .theme-dark & (or adding it as a fallback selector alongside :has(...)) to avoid a hard dependency on :has.

Copilot uses AI. Check for mistakes.
Comment on lines 14 to 26
&--one {
@include fs(xxxl);
line-height: 1.1;
margin-bottom: ($gutter * 2);

&::after {
display: block;
content: '';
width: 70px;
height: 1px;
margin-top: ($gutter * 2);
background-color: $color--black;
background-color: currentColor;
}
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heading--one no longer sets a dark-mode text color. Previously it was forced to $color--white in .theme-dark, but now it will just inherit the body color (currently light-dark($color--off-black, $color--light-grey)), which changes the heading’s appearance/contrast in dark mode. Consider explicitly setting color using light-dark(...) (or reinstating the dark override) so dark mode keeps the intended heading color.

Copilot uses AI. Check for mistakes.
Comment on lines +3 to 4
fill: currentColor;
}
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change removes the emoji hover/focus styling (and the fill transition) entirely. If the intent was only to refactor light/dark theming, this is a functional regression: the SVG no longer provides any visual feedback on hover/focus unless some other rule changes color on the button. Consider restoring hover/focus styling (potentially via color + currentColor, or light-dark(...)), so the interaction feedback remains.

Suggested change
fill: currentColor;
}
fill: currentColor;
transition: color 150ms ease-in-out, fill 150ms ease-in-out;
}
.feedback-emoji:hover,
.feedback-emoji:focus-visible {
color: light-dark(#111827, #f9fafb);
}

Copilot uses AI. Check for mistakes.
@thibaudcolas thibaudcolas merged commit 081535e into main Mar 11, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants