Skip to content

Fix dark mode colour scheme selector #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
vredchenko wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix dark mode colour scheme selector #133

vredchenko wants to merge 1 commit into from

Conversation

@vredchenko
Copy link
Collaborator

@vredchenko vredchenko commented Feb 4, 2026 •
edited
Loading

Summary

  • Change colorSchemeSelector in BaseTheme from "class" to "data-mode" so MUI toggles the data-mode attribute on the root element when ColourSchemeButton switches dark mode

Note: The ds-theme branch's ThemeProvider also needs a ColorSchemeSync fix (its useLayoutEffect sets class and color-scheme inline style from a static mode prop, which goes stale after toggling). That fix will go into ds-theme directly when it rebases on top of this.

Fixes #130

@vredchenko
Fix dark mode colour scheme selector (#130)
84b61b9 
Change colorSchemeSelector from "class" to "data-mode" so that toggling
dark mode updates the data-mode attribute and color-scheme CSS property
on the root element, not just the CSS class.
@vredchenko vredchenko marked this pull request as draft February 4, 2026 14:23
@vredchenko vredchenko marked this pull request as ready for review February 4, 2026 14:27
@vredchenko vredchenko force-pushed the fix/dark-mode-colour-scheme branch from ab0e25a to 84b61b9 Compare February 4, 2026 14:31
@akademy akademy self-requested a review February 5, 2026 11:49
akademy
akademy reviewed Feb 5, 2026
View reviewed changes
Copy link
Member

@akademy akademy left a comment

Choose a reason for hiding this comment

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

Currently discussing if this is necessary.

@vredchenko
Copy link
Collaborator Author

vredchenko commented Feb 5, 2026

Dark/Light Mode Toggling - Class/Style vs. Data Attribute

When implementing a dark/light mode toggle, the core idea is to apply different styles based on the active theme. This can be achieved by manipulating HTML attributes on a high-level element (like <html> or <body>).

1. Changing class or style attributes (e.g., <html class="dark"> or <body style="background: black;">)

Pros:

  • Simplicity and Familiarity: Using classes like dark or light is a very common and intuitive pattern for applying different styles via CSS. Directly manipulating style attributes is also straightforward for simple, dynamic changes.
  • Direct CSS Integration: CSS rules can directly target these classes (e.g., .dark body { ... }).
  • Minimal Overhead: For very simple themes without complex variable structures, changing a single class name can be lightweight.

Cons:

  • Less Semantic for State: While functional, a class name like dark doesn't inherently convey "theme state" as clearly as a dedicated data attribute.
  • Potential for Class Name Collisions: In large projects or when integrating third-party libraries, there's a risk of class name clashes if not carefully managed (though less common with modern CSS-in-JS or module approaches).
  • Global Scope Concerns (for document.body.className): If you're directly manipulating document.body.className, other scripts that also modify the body's classes might interfere.
  • Limited for Complex Theming: While possible, managing a complex theme with many variables solely through class changes can lead to more verbose and harder-to-maintain CSS without the aid of CSS custom properties (variables).

2. Changing data attributes (e.g., <html data-theme="dark">)

Pros:

  • Semantic Clarity: data-* attributes are specifically designed to store custom, non-standard data private to the page or application. Using data-theme="dark" clearly communicates that this attribute is defining the current theme state.

  • Avoids Class Collisions: Data attributes operate in their own namespace, significantly reducing the risk of clashes with existing CSS classes or other component styles.

  • Excellent with CSS Variables (Custom Properties): This is where data attributes shine. You can define CSS variables that change their values based on the data-theme attribute, and then use these variables throughout your CSS. This creates a highly organized, modular, and maintainable theming system.

    /* Example CSS with data attributes and variables */
:root[data-theme='light'] {
  --primary-color: #007bff;
  --text-color: #333;
}
:root[data-theme='dark'] {
  --primary-color: #66b3ff;
  --text-color: #f0f0f0;
}

body {
  background-color: var(--primary-color);
  color: var(--text-color);
}

  • Easily Accessible by JavaScript: The data-theme attribute can be easily read and manipulated by JavaScript to understand the current theme state or trigger theme-specific logic.

Cons:

  • Slightly More Verbose Selectors: CSS selectors like [data-theme="dark"] are slightly longer than .dark, but this is a very minor aesthetic concern.
  • Initial Learning Curve (for CSS Variables): If a team is not already using CSS variables, adopting them alongside data attributes might introduce a small learning curve, but the benefits in maintainability usually outweigh this.

Best Practices and Recommendation

For modern web development, especially with frameworks like React, the data attribute approach combined with CSS variables (custom properties) is generally considered the superior and more robust solution.

It offers:

  1. Cleaner Separation of Concerns: HTML handles semantic state (data-theme), while CSS variables manage the actual style values.
  2. Improved Maintainability: Centralized theme definitions make it easy to modify or expand themes.
  3. Reduced Side Effects: Less chance of interfering with other parts of the application.

Additional Best Practices:

  • Persistence: Store the user's theme preference in localStorage to remember it across sessions.
  • System Preference: Respect the user's operating system preference using the prefers-color-scheme media query as a default.
  • Prevent FOUC (Flash of Unstyled Content): Include a small, fast inline script in the <head> of your index.html to apply the correct data-theme attribute before your main JavaScript bundle loads. This prevents a brief flash of the default theme before the user's preference is applied.

Given the context of a React UI, adopting the data-theme attribute along with CSS variables would align with modern and scalable theming patterns, making the application more maintainable and robust in the long run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akademy akademy akademy left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@vredchenko vredchenko

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dark mode not applied correctly through the NavBar

2 participants

@vredchenko @akademy