Repository navigation
GUI: add dark/light theme toggle (default dark) - #91
Merged
Anthony Sligar (sligara7) merged 2 commits intoOct 6, 2026
Merged
Conversation
Anthony Sligar (sligara7)
requested review from
Jakub Wlodek (jwlodek)
and
a balanced review from Copilot
October 5, 2026 21:14
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Multiple open windows can display theme controls inconsistent with the application-wide theme.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds persistent dark/light GUI theming with live switching.
Changes:
- Splits shared styling from theme-specific palettes.
- Adds a status-bar theme toggle backed by
QSettings. - Adds theme construction and persistence tests.
| File | Description |
|---|---|
src/hextools/gui/_theme.py |
Defines palettes, stylesheet generation, and persistence. |
src/hextools/gui/__main__.py |
Adds and initializes the theme toggle. |
tests/test_gui_theme.py |
Tests theme generation, application, and storage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Split the BNL stylesheet into a shared template plus dark and light surface palettes. A 'Dark mode' switch in the status bar restyles the app live; the choice is remembered per user via QSettings. Light mode uses teal instead of cerulean for accent text, for contrast on white. Assisted-by: copilot:claude-opus-4-8
Addresses PR 91 review. The stylesheet is application-wide but each window built its own checkbox, read the theme once, and heard nothing later, so with two windows the second showed the old theme (reproduced offscreen: theme=light, second switch still checked). QtThemeSwitch (gui/theme_switch.py) changes the theme through set_theme(), which emits one per-application signal every switch follows; Qt drops a closed window's connection. test_every_switch_follows_a_theme_change was observed failing before the fix. Assisted-by: copilot:claude-opus-4-8
Anthony Sligar (sligara7)
force-pushed
the
hex-gui-theme-toggle
branch
from
October 6, 2026 20:39
581e66e to
c1d4868
Compare
|
|
||
| @pytest.fixture | ||
| def qt_app(monkeypatch, tmp_path): | ||
| pytest.importorskip("qtpy") |
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
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.
Split the BNL stylesheet into a shared template plus dark and light surface palettes. A 'Dark mode' switch in the status bar restyles the app live; the choice is remembered per user via QSettings. Light mode uses teal instead of cerulean for accent text, for contrast on white. Every window's switch follows one application-wide theme-changed signal (
gui/theme_switch.py), so multiple windows stay in step.How it was verified: mock
pixi run -e gui-dev pytest tests/test_gui_theme.py: 6 passed, includingtest_every_switch_follows_a_theme_change(observed failing before the review fix).HEXTOOLS_RUNNING_IN_CI=YES): both themes captured, and a two-window check confirms both switches follow one change.CI
The red checks match
main's current baseline, measured with CI's own commands onmain(60acf66) and this branch in the same env:main, 77 here. Every added diagnostic is an unresolvedqtpyimport, the same as 50 ofmain's 71: CI type-checks in thedevenv, which has no GUI packages. For the same reason the Qt test intests/test_gui_theme.pyskips in CI.Assisted-by: copilot:claude-opus-5-5