Skip to content

Conversation

@john-sp
Copy link
Member

@john-sp john-sp commented Oct 24, 2025

No description provided.

Copilot AI review requested due to automatic review settings October 24, 2025 20:17
@github-actions
Copy link

🚀 Draft Preview: https://pr180-draft.lc0.org/

Copy link
Contributor

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

This PR refactors the theme management system to use a data-attribute-based approach instead of CSS classes. The changes streamline theme application logic and implement a three-state toggle (light → dark → system) that respects user preferences while maintaining localStorage persistence.

Key Changes:

  • Replaced class-based theme switching (.theme-light, .theme-dark) with data-attribute approach (data-theme='light', data-theme='dark')
  • Consolidated theme variables into a centralized palette structure with explicit light/dark prefixes
  • Implemented helper functions (applyTheme, saveTheme) to reduce code duplication in theme toggle logic

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
themes/leela/assets/js/main.js Refactored theme toggle logic to use data attributes and extracted reusable functions for applying and saving themes
themes/leela/assets/css/syntax.css Updated dark mode media query selector to match new data-attribute approach
themes/leela/assets/css/main.css Restructured CSS variables into a centralized palette with light/dark prefixes and updated selectors to use data attributes

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

htmlEl.classList.add("theme-light");
localStorage.setItem("theme", "light");
themeIcon.className = "ri--sun-line";
const currentTheme = htmlEl.getAttribute("data-theme");
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The variable currentTheme is declared but never used. Since the toggle logic only relies on savedTheme, this line should be removed to avoid confusion.

Suggested change
const currentTheme = htmlEl.getAttribute("data-theme");

Copilot uses AI. Check for mistakes.
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.

1 participant