feat: dark Theme support #1379
feat: dark Theme support #1379
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive dark theme support to OpenBoard with automatic system theme detection and a user preference option. The implementation introduces a new UBThemeManager singleton that manages theme switching, applies appropriate color palettes, and loads theme-specific stylesheets.
Key changes include:
- New
UBThemeManagerclass for centralized theme management with system detection (Windows/macOS/Linux) - Theme preference UI in settings with Auto/Light/Dark options
- Replacement of hardcoded colors with palette-aware references throughout the codebase
- Dark/light CSS stylesheets for startup hints with JavaScript-based theme switching
Reviewed Changes
Copilot reviewed 102 out of 102 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/core/UBThemeManager.{h,cpp} | New theme manager singleton with system detection and widget update logic |
| src/core/UBApplication.cpp | Initializes theme manager and connects to Qt 6.5+ color scheme signals |
| src/core/UBSettings.{h,cpp} | Adds appThemeMode setting for user preference (Auto/Light/Dark) |
| src/core/UBPreferencesController.cpp | Wires theme combo box to settings and applies changes |
| src/gui/*.cpp | Replaces hardcoded colors with palette references |
| resources/style.qss | Updates widget styles to use palette() references |
| resources/{darkTheme,lightTheme}.qss | New theme-specific stylesheets |
| resources/startupHints/*.html | Adds JavaScript theme detection and CSS switching |
| resources/forms/preferences.ui | Adds theme selection combo box to preferences |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mShowNextTime->setChecked(UBSettings::settings()->appStartupHintsEnabled->get().toBool()); | ||
| connect(mShowNextTime,SIGNAL(stateChanged(int)),this,SLOT(onShowNextTimeStateChanged(int))); | ||
|
|
||
| connect(UBThemeManager::instance(), &UBThemeManager::themeChanged, mpSankoreAPI, &UBWidgetUniboardAPI::themeChanged); |
There was a problem hiding this comment.
The connect call references mpSankoreAPI which is not visible in this diff. Ensure that mpSankoreAPI is properly initialized before this connection is made, as connecting to a null pointer will cause a crash. Consider adding a null check before making the connection.
| <script> | ||
| var params = new URLSearchParams(window.location.search); | ||
| var theme = params.get('theme') || 'light'; | ||
| document.write('<link rel="stylesheet" href="css/' + theme + '.css" type="text/css">'); | ||
| </script> |
There was a problem hiding this comment.
Using document.write() is considered a bad practice in modern web development as it can cause issues with parsing and security. Consider using DOM manipulation methods instead:
var link = document.createElement('link');
link.rel = 'stylesheet';
link.href = 'css/' + theme + '.css';
link.type = 'text/css';
document.head.appendChild(link);| if (window.sankore.themeChanged && typeof window.sankore.themeChanged.connect === 'function') { | ||
| window.sankore.themeChanged.connect(applyTheme); |
There was a problem hiding this comment.
[nitpick] The condition typeof window.sankore.themeChanged.connect === 'function' appears to check for a Qt signal connection method. However, this pattern may not work reliably across all Qt versions. Consider documenting the minimum Qt version required for this feature or adding error handling if the connection fails.
| if (window.sankore.themeChanged && typeof window.sankore.themeChanged.connect === 'function') { | |
| window.sankore.themeChanged.connect(applyTheme); | |
| // Qt signal/slot connection: requires Qt >= 4.7 (QtWebKit with signal/slot support in JS) | |
| if (window.sankore.themeChanged && typeof window.sankore.themeChanged.connect === 'function') { | |
| try { | |
| window.sankore.themeChanged.connect(applyTheme); | |
| } catch (e) { | |
| console.warn("Failed to connect to sankore.themeChanged signal. This feature may require a newer Qt version.", e); | |
| } | |
| } else if (window.sankore.themeChanged) { | |
| console.warn("sankore.themeChanged.connect is not a function. Theme change signal may not be supported in this Qt version."); |
| QLineEdit[invalid="true"] { | ||
| background: #FFB3C8; | ||
| } |
There was a problem hiding this comment.
The hard-coded color #FFB3C8 for invalid input is not theme-aware and will look out of place in dark mode. Consider using a palette-relative color or a CSS variable that adapts to the theme.
| currentTheme = isDark ? 'dark' : 'light'; | ||
| // Reflect theme on outer frame (basic.css uses data-theme on :root) | ||
| document.documentElement.setAttribute('data-theme', currentTheme); | ||
| if (typeof current !== 'undefined' && current > 0) { |
There was a problem hiding this comment.
The comparison current !== 'undefined' is checking if the string "undefined" equals current, but should likely be typeof current !== 'undefined' to check the type. This could cause the function to execute with invalid values.
| @@ -0,0 +1,355 @@ | |||
| /* | |||
| * Copyright (C) 2015-2025 Département de l'Instruction Publique (DIP-SEM) | |||
There was a problem hiding this comment.
[nitpick] The copyright year shows 2025, but according to the context, it's currently November 2025. Consider whether this should be 2024-2025 instead to reflect the actual year the work was done.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Only tested on Windows at this time.
Todo :