feat(theme): add light/dark/system appearance preference (#96)#111
Closed
thedavidweng wants to merge 17 commits into
Closed
feat(theme): add light/dark/system appearance preference (#96)#111thedavidweng wants to merge 17 commits into
thedavidweng wants to merge 17 commits into
Conversation
Contributor
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/scrollbar-visibility-97 #111 +/- ##
==============================================================
+ Coverage 77.3% 78.1% +0.8%
==============================================================
Files 126 127 +1
Lines 6266 6386 +120
Branches 2022 2042 +20
==============================================================
+ Hits 4846 4992 +146
+ Misses 1376 1352 -24
+ Partials 44 42 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
Greptile SummaryThis PR adds a persisted light, dark, and system appearance preference. The main changes are:
Confidence Score: 5/5Safe to merge based on the reviewed changed paths. The theme preference is wired through config, IPC, frontend types, store hydration, settings UI, startup gating, CSS tokens, and focused tests. No verified correctness or security issue was found, and the audience color-scheme issue is addressed. No files require special attention.
 T-Rex LogsWhat T-Rex did
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant SettingsUI as Settings Appearance Radio
participant Store as settings-store
participant IPC as set_theme_preference IPC
participant Config as AppConfig
participant ThemeRuntime as theme-runtime
participant Window as Tauri Window
User->>SettingsUI: choose system/light/dark
SettingsUI->>Store: setThemePreference(preference)
Store->>Store: optimistic update + generation
Store->>IPC: invoke set_theme_preference
IPC->>Config: parse and persist ThemePreference
Config-->>IPC: AppSettings snapshot
IPC-->>Store: updated settings
Store->>ThemeRuntime: themePreference changes
ThemeRuntime->>ThemeRuntime: resolve system via prefers-color-scheme
ThemeRuntime->>ThemeRuntime: apply data-theme + color-scheme
ThemeRuntime->>Window: setTheme(light/dark/null)
Window-->>ThemeRuntime: settled or timeout guard
ThemeRuntime-->>Store: startupThemeReady permits windowReady gate
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant SettingsUI as Settings Appearance Radio
participant Store as settings-store
participant IPC as set_theme_preference IPC
participant Config as AppConfig
participant ThemeRuntime as theme-runtime
participant Window as Tauri Window
User->>SettingsUI: choose system/light/dark
SettingsUI->>Store: setThemePreference(preference)
Store->>Store: optimistic update + generation
Store->>IPC: invoke set_theme_preference
IPC->>Config: parse and persist ThemePreference
Config-->>IPC: AppSettings snapshot
IPC-->>Store: updated settings
Store->>ThemeRuntime: themePreference changes
ThemeRuntime->>ThemeRuntime: resolve system via prefers-color-scheme
ThemeRuntime->>ThemeRuntime: apply data-theme + color-scheme
ThemeRuntime->>Window: setTheme(light/dark/null)
Window-->>ThemeRuntime: settled or timeout guard
ThemeRuntime-->>Store: startupThemeReady permits windowReady gate
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This was referenced Jul 13, 2026
Owner
Author
Maintainer acceptance — merge slot 5, strictly after #105BLOCKED — changes required. Blocking first-paint race
Initialize the system preference synchronously from the media query (with the existing unavailable fallback), or otherwise resolve it before the layout/ready gates. Add a test for first hydrated paint with Then rebase onto #105, re-run CI, and attach the required packaged macOS/Windows/Linux manual matrix. This is a major visual change and must not be merged before that evidence. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Owner
Author
Re-review: code blockers are addressed; required dependency and physical-platform acceptance remainThe current head fixes the initial system/light preference initialization and avoids restoring an entire stale settings snapshot on failure. This PR is still not ready to merge because its predecessor and the required native-WebView validation are incomplete. Complete these steps:
Run the existing unit/browser tests after rebasing and reply with the package build IDs, manual matrix, and commit SHA. #112 must not be merged before this PR is accepted. |
thedavidweng
force-pushed
the
feat/light-mode-96
branch
from
July 15, 2026 09:15
25499fa to
190b278
Compare
thedavidweng
force-pushed
the
feat/scrollbar-visibility-97
branch
from
July 16, 2026 03:44
c89f8c2 to
8fd568c
Compare
thedavidweng
force-pushed
the
feat/light-mode-96
branch
from
July 16, 2026 03:51
190b278 to
6a789ff
Compare
Owner
Author
|
@greptile review |
Contributor
Linux VM theme acceptance (Cursor Cloud)Commit: Matrix (web content)
ScreenshotsLight shell Dark shell System + OS light System + OS dark Audience / fullscreen RecordingWebKit frames also under Gaps still requiring human / native runners
|
Format base-test.ts and scrollbar.spec.ts to pass oxfmt format-check in pre-push hook. These were pre-existing uncommitted changes on the branch that failed format-check during push.
Avoid depending on the i18n "Preferences" heading so settings scroll overflow checks stay stable across locales and settings sections.
Two scrollbar spec tests registered a second page.addInitScript that called window.__OPENKARA_E2E__.setMockSongs(), depending on the fixture's init script having already defined that object. Playwright does not guarantee evaluation order for multiple init scripts, so if the override ran first it would throw before the app loaded and make the overflow tests flaky. Move both overrides after page.goto: call tauriMock.setMockSongs() (which runs the same helper through page.evaluate) and trigger a library reload via the openkara.library BroadcastChannel — the same cross-webview sync path the app already subscribes to. This keeps the mock data setup in the same execution context that defines window.__OPENKARA_E2E__ and eliminates the ordering hazard.
cursor
Bot
force-pushed
the
feat/scrollbar-visibility-97
branch
from
July 17, 2026 02:13
6d0e6d1 to
4a1ec11
Compare
thedavidweng
force-pushed
the
feat/light-mode-96
branch
from
July 17, 2026 03:55
6a789ff to
a00e5fc
Compare
…ollbar contract The non-greedy regex only captured up to the first inner rule's closing brace, leaving the remaining ~30 lines of the @supports not block unchecked. Reuse blockBodyForSelectorAt for correct full-block extraction.
Add a ThemePreference (system|light|dark) persisted in AppConfig with a new set_theme_preference IPC command. The frontend theme-runtime hook resolves the preference against the OS prefers-color-scheme signal, applies data-theme + color-scheme before first paint, and syncs the Tauri native window theme. The startup ready gate now waits for the first native setTheme settlement (with a 750ms timeout guard) so the hidden window is never shown with the wrong theme. Semantic CSS tokens replace hardcoded colors across production components; the audience/fullscreen stage stays explicitly dark via a data-presentation-mode marker regardless of the primary preference. Settings gains an Appearance radio with en/zh-CN locale strings.
The scrollbar platform contract test asserts color-scheme contains 'dark', but the light-mode feature added a theme_preference field to AppSettings that the e2e mock did not return. Without it the settings store hydrated themePreference to undefined, so applyResolvedTheme never set color-scheme to dark and the assertion failed. Add theme_preference: 'dark' to the mock get_settings payload.
… audience dark mode, and contract doc - Fix dead conditional in LyricsFontSizeControl and LyricsOffsetControl: non-default values now show accent color instead of identical text color - Fix macOS light-mode scrollbar: use descendant combinator instead of compound selector so :root[data-theme="light"] matches the mac platform element correctly - Preserve audience dark color-scheme in applyResolvedTheme when data-presentation-mode="audience" marker is present - Correct settings contract doc: effective_theme_preference() returns raw preference, does not resolve System against OS appearance - Resolve CHANGELOG.md merge conflict with main
Fix QueueButton queue fixture typing, extract queue item state class resolution for drop-indicator token coverage, and assert volume mute hover tokens on both collapsed and expanded paths.
thedavidweng
force-pushed
the
feat/light-mode-96
branch
from
July 17, 2026 06:03
a00e5fc to
bac59a4
Compare
The font-size and offset controls used text-[var(--color-accent)] for the non-default branch, which was visually distinct from the default text-[var(--color-text-dim)] branch. However, the tests were checking for text-[var(--color-text)] which appears in every button element, making them pass vacuously. Switch the highlight token to text-[var(--color-control-primary)] and update the tests to assert on the actual non-default class.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…nguage switch failure When loadStartupSettings hydrates saved settings successfully but the subsequent language switch fails, the catch handler was overwriting every already-loaded preference with DEFAULT_APP_SETTINGS. Guard the default-settings patch with a hydrated check so only the genuinely un-hydrated case (missing/corrupt config) applies defaults, preserving user preferences when the language switch fails.
Owner
Author
Re-review — no LGTM yet: retarget/rebase and native acceptance requiredReviewed head #105 has merged, but this PR still targets the feature branch The code-level first-paint and rollback issues previously identified are addressed. Final sign-off still requires the issue's packaged native-WebView matrix:
Request re-review with the new head and attached platform evidence. |
7 tasks
Align light-mode surfaces and semantic foregrounds with the approved visual direction. Keep the native traffic-light controls available by routing the green control through AppKit zoom instead of titlebar-less native full screen.
Owner
Author
|
已补充本轮 #111 修复，CI run 938 全绿。 本轮变更
自动验证
仍需人工 UI 验收，暂不建议仅凭 CI 合并
|
4 tasks
thedavidweng
added a commit
that referenced
this pull request
Jul 19, 2026
Integrates the #111 → #112 → #118 stack onto current main: - Light/dark/system appearance with monochrome controls and selective accent - Unified right-side playback button chrome - Wider volume sliders with density-based px token table - Stem mixer popup via body portal (no clipping in settings) - Restored E2E sort fixtures after stack-merge conflict resolution Supersedes #111, #112, #118.
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ThemePreference(system|light|dark) persisted inAppConfigwith a newset_theme_preferenceIPC command andcore:window:allow-set-themecapabilitytheme-runtimehook resolves the preference against the OSprefers-color-schemesignal, appliesdata-theme+color-schemebefore first paint viauseLayoutEffect, and syncs the Tauri native window theme; the startup ready gate waits for the first nativesetThemesettlement (750ms timeout guard) so the hidden window is never shown with the wrong themetext-white/bg-white/rgba(255,255,255,…)across production components; the audience/fullscreen stage stays explicitly dark via adata-presentation-mode="audience"marker regardless of the primary preferencedocs/references/contracts/settings.mdTest plan
pnpm build(tsc + vite) passespnpm test— 1334 tests passpnpm lint— 0 warnings, 0 errorscargo test— 389 Rust tests passcargo fmtvia pre-commit hook