feat(theme): replace color scheme system with light/dark + accent skins (PR #627 by @aronprins)

Independent review by @nesquena confirmed all blockers resolved. Theme×skin two-axis system replaces old monolithic color schemes. Closes #627. Co-Authored-By: aronprins <aronprins@users.noreply.github.com>
This commit is contained in:
Aron Prins
2026-04-18 08:37:09 +02:00
committed by GitHub
parent f3f23abd4e
commit 7cb5547056
18 changed files with 870 additions and 482 deletions

View File

@@ -119,7 +119,7 @@ class TestSystemTheme:
def test_apply_theme_resolves_system(self):
src = read("static/boot.js")
assert "name==='system'" in src or "=== 'system'" in src, (
assert "normalized.theme==='system'" in src or "=== 'system'" in src, (
"_applyTheme must branch on 'system' to resolve via matchMedia"
)
@@ -131,23 +131,23 @@ class TestSystemTheme:
def test_load_settings_calls_apply_theme(self):
src = read("static/boot.js")
assert "_applyTheme(_theme)" in src, (
assert "_applyTheme(appearance.theme)" in src, (
"loadSettings must call _applyTheme() instead of direct data-theme assignment"
)
def test_system_option_in_theme_select(self):
def test_system_option_in_theme_picker(self):
html = read("static/index.html")
assert 'value="system"' in html, (
"Theme <select> must include <option value=\"system\">"
assert "_pickTheme('system')" in html, (
"Theme picker must include a system theme button"
)
assert "System (auto)" in html, (
"Theme picker must show 'System (auto)' label"
assert ">System<" in html, (
"Theme picker must show 'System' label"
)
def test_theme_select_uses_apply_theme_onchange(self):
def test_theme_picker_uses_pick_theme(self):
html = read("static/index.html")
assert "_applyTheme(this.value)" in html, (
"Theme <select> onchange must call _applyTheme(this.value)"
assert "_pickTheme(" in html, (
"Theme buttons must call _pickTheme()"
)
def test_flicker_script_resolves_system(self):
@@ -156,6 +156,9 @@ class TestSystemTheme:
assert "==='system'" in html or "=== 'system'" in html, (
"Flicker-prevention head script must resolve 'system' before setting data-theme"
)
assert "legacy={slate:['dark','slate']" in html, (
"Flicker-prevention head script must normalize legacy theme names on first paint"
)
def test_system_in_commands_themes_list(self):
src = read("static/commands.js")
@@ -165,8 +168,14 @@ class TestSystemTheme:
def test_commands_uses_apply_theme(self):
src = read("static/commands.js")
assert "_applyTheme(themeName)" in src, (
"cmdTheme must call _applyTheme() to handle system resolution"
assert "_applyTheme(appearance.theme)" in src, (
"cmdTheme must call _applyTheme() with the normalized canonical theme"
)
def test_commands_accept_legacy_theme_aliases(self):
src = read("static/commands.js")
assert "const legacyThemes=Object.keys(_LEGACY_THEME_MAP||{});" in src, (
"cmdTheme must accept legacy theme aliases and map them onto canonical appearance values"
)
def test_panels_reverts_via_apply_theme(self):
@@ -197,3 +206,21 @@ class TestSystemTheme:
f"cmd_theme description should mention 'system' in all 5 locales; "
f"found {count}"
)
def test_theme_listener_cleanup_uses_stable_handler(self):
src = read("static/boot.js")
assert "_systemThemeMq&&_onSystemThemeChange" in src, (
"_applyTheme must track the active OS-theme listener so it can be removed cleanly"
)
assert "removeEventListener('change',_onSystemThemeChange)" in src, (
"_applyTheme must remove the previous OS-theme listener before adding a new one"
)
def test_panels_hydrates_appearance_before_models_fetch(self):
src = read("static/panels.js")
skin_idx = src.index("const skinVal=(settings.skin||'default').toLowerCase();")
models_idx = src.index("const models=await api('/api/models');")
assert skin_idx < models_idx, (
"loadSettingsPanel must hydrate theme/skin before awaiting /api/models, "
"otherwise a slow model fetch can clobber an in-progress skin selection"
)

View File

@@ -19,39 +19,38 @@ COMPOSE = (REPO_ROOT / "docker-compose.yml").read_text(encoding="utf-8")
# ── #594: light theme dialog overrides ───────────────────────────────────────
def test_594_app_dialog_has_light_theme_override():
"""style.css must have a light theme rule targeting .app-dialog background."""
assert ':root[data-theme="light"] .app-dialog{' in STYLE_CSS or \
":root[data-theme='light'] .app-dialog{" in STYLE_CSS, (
"Missing light theme override for .app-dialog — dialogs appear dark on light theme"
def test_594_app_dialog_has_light_mode_override():
"""style.css must have a light mode rule targeting .app-dialog background."""
assert ':root:not(.dark) .app-dialog{' in STYLE_CSS, (
"Missing light mode override for .app-dialog — dialogs appear dark on light theme"
)
def test_594_app_dialog_input_has_light_theme_override():
"""style.css must have a light theme rule for .app-dialog-input."""
assert ":root[data-theme=\"light\"] .app-dialog-input{" in STYLE_CSS, (
"Missing light theme override for .app-dialog-input"
def test_594_app_dialog_input_has_light_mode_override():
"""style.css must have a light mode rule for .app-dialog-input."""
assert ":root:not(.dark) .app-dialog-input{" in STYLE_CSS, (
"Missing light mode override for .app-dialog-input"
)
def test_594_app_dialog_btn_has_light_theme_override():
"""style.css must have a light theme rule for .app-dialog-btn."""
assert ":root[data-theme=\"light\"] .app-dialog-btn{" in STYLE_CSS, (
"Missing light theme override for .app-dialog-btn"
def test_594_app_dialog_btn_has_light_mode_override():
"""style.css must have a light mode rule for .app-dialog-btn."""
assert ":root:not(.dark) .app-dialog-btn{" in STYLE_CSS, (
"Missing light mode override for .app-dialog-btn"
)
def test_594_app_dialog_close_has_light_theme_override():
"""style.css must have a light theme rule for .app-dialog-close."""
assert ":root[data-theme=\"light\"] .app-dialog-close{" in STYLE_CSS, (
"Missing light theme override for .app-dialog-close"
def test_594_app_dialog_close_has_light_mode_override():
"""style.css must have a light mode rule for .app-dialog-close."""
assert ":root:not(.dark) .app-dialog-close{" in STYLE_CSS, (
"Missing light mode override for .app-dialog-close"
)
def test_594_file_rename_input_has_light_theme_override():
"""style.css must have a light theme rule for .file-rename-input."""
assert ":root[data-theme=\"light\"] .file-rename-input{" in STYLE_CSS, (
"Missing light theme override for .file-rename-input"
def test_594_file_rename_input_has_light_mode_override():
"""style.css must have a light mode rule for .file-rename-input."""
assert ":root:not(.dark) .file-rename-input{" in STYLE_CSS, (
"Missing light mode override for .file-rename-input"
)

View File

@@ -28,7 +28,7 @@ def test_msg_body_table_tr_stripe_present():
def test_msg_body_light_theme_overrides():
css = _read_css()
assert ':root[data-theme="light"] .msg-body th' in css, \
'Light-theme override for .msg-body th missing from style.css'
assert ':root[data-theme="light"] .msg-body td' in css, \
'Light-theme override for .msg-body td missing from style.css'
assert ':root:not(.dark) .msg-body th' in css, \
'Light-mode override for .msg-body th missing from style.css'
assert ':root:not(.dark) .msg-body td' in css, \
'Light-mode override for .msg-body td missing from style.css'

View File

@@ -694,12 +694,12 @@ def test_style_css_has_session_actions_dropdown(cleanup_test_sessions):
".session-action-menu must use position:fixed to avoid sidebar clipping"
def test_style_css_active_session_uses_gold(cleanup_test_sessions):
"""Active session style should use gold/amber color (#e8a030) not just blue."""
def test_style_css_active_session_uses_accent(cleanup_test_sessions):
"""Active session style should use accent color variable, not hardcoded hex."""
src = REPO_ROOT / "static" / "style.css"
code = src.read_text()
assert "#e8a030" in code, \
"Active session gold color (#e8a030) not found in style.css"
assert "var(--accent" in code and ".session-item.active" in code, \
"Active session must use var(--accent) variables in style.css"
def test_sessions_js_uses_action_menu_not_per_row_buttons(cleanup_test_sessions):

View File

@@ -120,15 +120,15 @@ def test_mic_btn_recording_state_css():
assert '.mic-btn.recording' in css
def test_mic_recording_color_red():
""".mic-btn.recording must use the red accent color #e94560."""
def test_mic_recording_color_error():
""".mic-btn.recording must use the error color variable or red."""
css, _ = get_text("/static/style.css")
recording_idx = css.find('.mic-btn.recording')
# Find the rule block after the selector
brace_open = css.find('{', recording_idx)
brace_close = css.find('}', brace_open)
rule = css[brace_open:brace_close]
assert '#e94560' in rule or 'e94560' in rule
assert 'var(--error)' in rule or '#e94560' in rule
def test_mic_recording_has_animation():

View File

@@ -109,14 +109,14 @@ def test_send_btn_no_old_padding():
assert 'padding:7px' not in rule and 'padding: 7px' not in rule
def test_send_btn_blue_background():
"""send-btn background must use the blue accent (#7cb9ff or similar)."""
def test_send_btn_accent_background():
"""send-btn background must use the accent color variable."""
css, _ = get_text("/static/style.css")
send_idx = css.find('.send-btn{')
brace_open = css.find('{', send_idx)
brace_close = css.find('}', brace_open)
rule = css[brace_open:brace_close]
assert '7cb9ff' in rule or '5ba8f5' in rule or 'var(--blue)' in rule
assert 'var(--accent)' in rule or 'var(--blue)' in rule or '7cb9ff' in rule
def test_send_btn_has_transition():

View File

@@ -1,11 +1,19 @@
"""
Sprint 26 Tests: pluggable UI themes — settings persistence, theme default,
custom theme names accepted.
Sprint 26 Tests: canonical appearance settings persist and legacy theme names
map onto the new theme + skin system.
"""
import json, urllib.error, urllib.request
import pathlib
import sys
from tests._pytest_port import BASE
REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent
if str(REPO_ROOT) not in sys.path:
sys.path.insert(0, str(REPO_ROOT))
from api import config
def get(path):
with urllib.request.urlopen(BASE + path, timeout=10) as r:
@@ -32,7 +40,7 @@ def test_settings_default_theme():
assert d.get("theme") == "dark"
def test_settings_set_theme_light():
def test_settings_set_theme_light_persists():
"""Setting theme to 'light' should persist and round-trip."""
try:
d, status = post("/api/settings", {"theme": "light"})
@@ -44,55 +52,103 @@ def test_settings_set_theme_light():
post("/api/settings", {"theme": "dark"})
def test_settings_set_theme_solarized():
"""Setting theme to 'solarized' should persist."""
def test_settings_set_theme_light():
"""Setting theme to 'light' should persist."""
try:
post("/api/settings", {"theme": "solarized"})
post("/api/settings", {"theme": "light"})
d, _ = get("/api/settings")
assert d.get("theme") == "solarized"
assert d.get("theme") == "light"
finally:
post("/api/settings", {"theme": "dark"})
def test_settings_set_theme_monokai():
"""Setting theme to 'monokai' should persist."""
def test_settings_set_theme_system():
"""Setting theme to 'system' should persist."""
try:
post("/api/settings", {"theme": "monokai"})
post("/api/settings", {"theme": "system"})
d, _ = get("/api/settings")
assert d.get("theme") == "monokai"
assert d.get("theme") == "system"
finally:
post("/api/settings", {"theme": "dark"})
def test_settings_set_theme_nord():
"""Setting theme to 'nord' should persist."""
def test_settings_set_skin():
"""Setting skin should persist."""
try:
post("/api/settings", {"theme": "nord"})
post("/api/settings", {"skin": "ares"})
d, _ = get("/api/settings")
assert d.get("theme") == "nord"
assert d.get("skin") == "ares"
finally:
post("/api/settings", {"theme": "dark"})
post("/api/settings", {"skin": "default"})
def test_settings_set_theme_slate():
"""Setting theme to 'slate' should persist."""
def test_settings_set_skin_poseidon():
"""Setting skin to 'poseidon' should persist."""
try:
post("/api/settings", {"theme": "slate"})
post("/api/settings", {"skin": "poseidon"})
d, _ = get("/api/settings")
assert d.get("theme") == "slate"
assert d.get("skin") == "poseidon"
finally:
post("/api/settings", {"theme": "dark"})
post("/api/settings", {"skin": "default"})
def test_settings_custom_theme_accepted():
"""Custom theme names should be accepted (no enum gate)."""
def test_settings_legacy_theme_maps_to_dark_skin_pair():
"""Legacy theme names should map to the closest supported theme + skin."""
try:
d, status = post("/api/settings", {"theme": "slate"})
assert status == 200
d2, _ = get("/api/settings")
assert d2.get("theme") == "dark"
assert d2.get("skin") == "slate"
finally:
post("/api/settings", {"theme": "dark", "skin": "default"})
def test_settings_legacy_monokai_maps_to_sisyphus_skin():
"""Monokai should migrate onto the closest supported accent skin."""
try:
d, status = post("/api/settings", {"theme": "monokai"})
assert status == 200
d2, _ = get("/api/settings")
assert d2.get("theme") == "dark"
assert d2.get("skin") == "sisyphus"
finally:
post("/api/settings", {"theme": "dark", "skin": "default"})
def test_settings_unknown_theme_falls_back_to_dark_default():
"""Unknown themes should normalize to a safe canonical appearance."""
try:
d, status = post("/api/settings", {"theme": "my-custom-theme"})
assert status == 200
d2, _ = get("/api/settings")
assert d2.get("theme") == "my-custom-theme"
assert d2.get("theme") == "dark"
assert d2.get("skin") == "default"
finally:
post("/api/settings", {"theme": "dark"})
post("/api/settings", {"theme": "dark", "skin": "default"})
def test_settings_invalid_skin_falls_back_to_default():
"""Unknown skin names should normalize back to the default accent."""
try:
d, status = post("/api/settings", {"skin": "not-a-skin"})
assert status == 200
d2, _ = get("/api/settings")
assert d2.get("skin") == "default"
finally:
post("/api/settings", {"skin": "default"})
def test_load_settings_normalizes_legacy_theme_from_file(monkeypatch, tmp_path):
"""Existing settings.json files with legacy theme names should normalize on load."""
settings_path = tmp_path / "settings.json"
settings_path.write_text(json.dumps({"theme": "solarized"}), encoding="utf-8")
monkeypatch.setattr(config, "SETTINGS_FILE", settings_path)
loaded = config.load_settings()
assert loaded["theme"] == "dark"
assert loaded["skin"] == "poseidon"
def test_theme_does_not_break_other_settings():
@@ -100,10 +156,10 @@ def test_theme_does_not_break_other_settings():
d_before, _ = get("/api/settings")
send_key_before = d_before.get("send_key")
try:
post("/api/settings", {"theme": "nord"})
post("/api/settings", {"theme": "light"})
d_after, _ = get("/api/settings")
assert d_after.get("send_key") == send_key_before
assert d_after.get("theme") == "nord"
assert d_after.get("theme") == "light"
finally:
post("/api/settings", {"theme": "dark"})
@@ -111,9 +167,9 @@ def test_theme_does_not_break_other_settings():
def test_theme_survives_round_trip():
"""Theme set via POST should appear in subsequent GET."""
try:
post("/api/settings", {"theme": "monokai"})
post("/api/settings", {"theme": "light"})
d, status = get("/api/settings")
assert status == 200
assert d["theme"] == "monokai"
assert d["theme"] == "light"
finally:
post("/api/settings", {"theme": "dark"})

View File

@@ -40,7 +40,7 @@ class TestActiveSessionTitleThemeColor(unittest.TestCase):
def test_active_session_title_uses_theme_variable(self):
"""
.session-item.active .session-title must use var(--gold) not a hardcoded hex.
The light-theme override line (data-theme="light") is allowed to keep its own
The light-mode override line (:not(.dark)) is allowed to keep its own
hardcoded color; we only check the base/dark rule.
"""
# Find all lines that match the active session title selector
@@ -48,7 +48,7 @@ class TestActiveSessionTitleThemeColor(unittest.TestCase):
base_rule_lines = [
line for line in lines
if ".session-item.active .session-title" in line
and 'data-theme="light"' not in line
and ':not(.dark)' not in line
]
self.assertTrue(
@@ -57,10 +57,9 @@ class TestActiveSessionTitleThemeColor(unittest.TestCase):
)
for line in base_rule_lines:
self.assertIn(
"var(--gold)",
line,
f"Expected var(--gold) in active session title rule, got: {line.strip()}"
self.assertTrue(
"var(--gold)" in line or "var(--accent-text)" in line,
f"Expected var(--gold) or var(--accent-text) in active session title rule, got: {line.strip()}"
)
self.assertNotIn(
"#e8a030",
@@ -69,6 +68,21 @@ class TestActiveSessionTitleThemeColor(unittest.TestCase):
)
class TestDarkTopbarSelector(unittest.TestCase):
def test_topbar_dark_border_uses_root_dark_selector(self):
self.assertIn(
":root.dark .topbar{border-bottom:1px solid rgba(255,255,255,.07);}",
STYLE_CSS,
"Topbar dark border override must target :root.dark after the theme-class migration",
)
self.assertNotIn(
'[data-theme="dark"] .topbar',
STYLE_CSS,
"Topbar dark border override must not keep the removed data-theme selector",
)
if __name__ == "__main__":
unittest.main()

View File

@@ -23,9 +23,11 @@ def test_tool_card_detail_uses_transitionable_collapsed_state():
def test_thinking_card_toggle_and_body_use_animation_friendly_state():
assert ".thinking-card-toggle{margin-left:auto;font-size:10px;display:inline-flex;" in COMPACT_CSS
assert ".thinking-card-body{display:block;max-height:0;opacity:0;overflow:hidden;" in COMPACT_CSS
# Body uses div default (display:block); canonical rule lives in the
# consolidated block. Open state caps at 260px (intentional "quieter" sizing).
assert ".thinking-card-body{max-height:0;opacity:0;overflow:hidden;" in COMPACT_CSS
assert re.search(
r"\.thinking-card\.open\s+\.thinking-card-body\s*\{[^}]*max-height:\s*300px;[^}]*opacity:\s*1;",
r"\.thinking-card\.open\s+\.thinking-card-body\s*\{[^}]*max-height:\s*260px;[^}]*opacity:\s*1;",
STYLE_CSS,
)
@@ -36,8 +38,10 @@ def test_tool_card_toggle_uses_same_chevron_icon_markup_as_thinking_card():
def test_thinking_card_uses_panel_chrome_with_gold_palette():
# Canonical thinking-card rule lives in the consolidated block (border-radius
# tightened from 10px → 8px as part of the "quieter card" design pass).
assert re.search(
r"\.thinking-card\s*\{[^}]*background:\s*rgba\(201,168,76,.05\);[^}]*border:\s*1px\s+solid\s+rgba\(201,168,76,.18\);[^}]*border-radius:\s*8px;",
r"\.thinking-card\s*\{[^}]*background:\s*var\(--accent-bg\);[^}]*border:\s*1px\s+solid\s+var\(--accent-bg-strong\);[^}]*border-radius:\s*8px;",
STYLE_CSS,
)
assert "border-left: 2px solid rgba(201,168,76,.4);" not in STYLE_CSS