Skip to content

feat: add user-defined custom themes#1589

Open
Michaelyklam wants to merge 4 commits intonesquena:masterfrom
Michaelyklam:feat/custom-themes
Open

feat: add user-defined custom themes#1589
Michaelyklam wants to merge 4 commits intonesquena:masterfrom
Michaelyklam:feat/custom-themes

Conversation

@Michaelyklam
Copy link
Copy Markdown
Contributor

@Michaelyklam Michaelyklam commented May 4, 2026

Thinking Path

  • Hermes WebUI already centralizes most colors as CSS variables in static/style.css, but Appearance only exposes built-in theme/skin choices.
  • Users who want a calmer or personalized palette currently have to edit CSS or add one-off built-in themes, which does not scale.
  • This PR treats the CSS variable layer as the design-token contract and adds a server-persisted custom theme model around it.
  • The UI stays compact: custom theme creation starts from an Add theme card and only expands the editor while creating or editing.
  • The result is a first-class way to define, preview, save, edit, delete, and re-apply custom color themes without adding a new frontend framework or build step.

What Changed

  • Added server-side settings support for theme: "custom", custom_theme_id, and a bounded custom_themes list.
  • Added validation/sanitization for custom theme IDs, names, modes, token keys, and safe color values before persisting settings.
  • Added first-paint and runtime custom theme application through CSS variables, with localStorage caching to reduce reload flicker.
  • Added Appearance UI for custom themes:
    • Mode/Themes section labeling: built-in light/dark/system controls are labeled Mode, while skin/custom-theme cards live under Themes
    • Add theme card in the Themes picker
    • hidden/expandable editor
    • live preview
    • polished color picker rows using native color inputs behind Hermes-styled controls
    • custom theme cards with edit and delete controls
  • Updated /theme command validation to accept custom as a canonical theme value.
  • Added the Sienna server allowlist parity fix so the existing Sienna skin round-trips through /api/settings validation.
  • Documented custom themes as part of the design-token contract in DESIGN.md.
  • Added regression coverage in tests/test_custom_themes.py for settings persistence, unsafe token rejection, first-paint/runtime hooks, UI hooks, edit/delete affordances, and polished color picker controls.

Why It Matters

This turns theme customization from hardcoded one-off palettes into a user-facing design-system feature. People can tune backgrounds, surfaces, text, accents, borders, inputs, hover states, and semantic colors from the UI, keep those themes server-side, and get the same look after reloads without manually editing CSS.

Verification

node --check static/boot.js
node --check static/panels.js
node --check static/commands.js
/home/michael/.hermes/hermes-agent/venv/bin/python -m pytest   tests/test_custom_themes.py   tests/test_1003_appearance_autosave.py   tests/test_sienna_skin.py   tests/test_ui_tool_call_cleanup.py   tests/test_batch_fixes.py   tests/test_sprint26.py   tests/test_1003_preferences_autosave.py   tests/test_sprint27.py -q
env -u HERMES_CONFIG_PATH /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/ -q
git diff --check

Results after rebasing onto latest origin/master:

91 passed in 2.20s
4189 passed, 2 skipped, 3 xpassed, 1 warning, 8 subtests passed in 408.80s (0:06:48)

Manual verification:

  • Started a temporary local server from this branch on 127.0.0.1:18790.
  • Opened Settings → Appearance and verified built-in Light/Dark/System controls are labeled Mode, while skin cards and Add theme/custom theme cards are grouped under Themes.
  • Opened the Add theme flow and verified the editor expands below Themes with a live preview and styled color picker rows instead of raw base HTML inputs.
  • Captured desktop and 390px mobile screenshots from the PR branch and committed them under docs/pr-media/1589/.

UI media:

  • Desktop: Add theme card in the picker:

    Add theme card in Appearance picker

  • Desktop: Editor expanded with live preview and styled color token controls:

    Custom theme editor live preview

  • Desktop: Lower editor controls with create/cancel actions:

    Custom theme editor actions

  • Desktop: Saved custom theme card with edit/delete affordances:

    Saved custom theme card with edit and delete affordances

  • Desktop: Custom theme applied to the chat view:

    Custom theme applied to chat view

  • Mobile 390px: Appearance picker with Add theme card:

    Mobile 390px Appearance picker with Add theme card

  • Mobile 390px: Expanded Add theme editor with live preview and color rows:

    Mobile 390px custom theme editor

  • Mobile 390px: Saved custom theme card with edit/delete affordances:

    Mobile 390px saved custom theme card

  • Mobile 390px: Custom theme applied to chat view:

    Mobile 390px custom theme applied to chat

  • Desktop: Renamed Appearance sections with built-in modes separated from Themes:

    Mode and Themes sections in Appearance

  • Desktop: Add theme editor expanded below the renamed Themes section:

    Custom theme editor below Themes section

  • Desktop: Saved custom theme card under Themes with edit/delete affordances:

    Saved custom theme card under Themes

Risks / Follow-ups

  • Custom themes intentionally allow only a curated token whitelist and safe color formats; future work can expand the token set if reviewers want broader coverage.
  • This PR uses /api/settings for persistence because Appearance settings already live there. If multi-user theme sharing becomes a goal, a separate theme endpoint/model may be cleaner.

Model Used

AI assisted.

  • Provider: OpenAI Codex
  • Model: gpt-5.5
  • Notable tool use: repo inspection, git worktree isolation, patching, targeted and full pytest runs, Node syntax checks, local browser QA, desktop/mobile screenshot capture

@Michaelyklam Michaelyklam marked this pull request as ready for review May 4, 2026 02:59
@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Thanks @Michaelyklam — this is a substantial feature and the design choice to build it on top of the existing CSS-variable layer (rather than introducing a new design-system framework) is the right call for Hermes WebUI. Treating the variable layer as the design-token contract and keeping persistence inside /api/settings keeps scope manageable.

Pulled and ran the full target suite:

tests/test_custom_themes.py
tests/test_1003_appearance_autosave.py
tests/test_sienna_skin.py
tests/test_ui_tool_call_cleanup.py
tests/test_batch_fixes.py
tests/test_sprint26.py
tests/test_1003_preferences_autosave.py
tests/test_sprint27.py
→ 89 passed in 2.35s
node --check static/{boot,panels,commands}.js → ok

Wider sweep across theme / skin / appearance / settings-tagged tests: ~210 passed.

Things I like

  1. The token whitelist. _CUSTOM_THEME_TOKEN_KEYS is curated rather than open-ended — bg, accent, text, etc. but not arbitrary CSS variables. The required-subset check ({"bg", "surface", "text", "accent"}) ensures every saved custom theme has the four tokens needed to render anything sensible. This is the right shape: real surface area for users without making the theme system a generic CSS injection vector.

  2. Color value sanitization. _CUSTOM_THEME_COLOR_RE matches #rgb, #rrggbb, #rrggbbaa, rgb(…), rgba(…) and rejects anything else (no url(), no var(), no JS-injectable garbage). The follow-up numeric check on rgb/rgba clamps each channel to 0–255 and alpha to 0–1, closing off the rgb(999, -50, 0) rounding silliness even where the regex would let it through. Good defense-in-depth.

  3. The 20-theme cap and dedupe-on-id. themes[:20] and seen prevent both runaway storage and accidental duplicate-id collisions corrupting state. Reasonable cap.

  4. First-paint via boot.js. Reading the cached custom theme from localStorage on first paint is the right way to avoid a flash of the wrong palette during initial load. Worth confirming that the localStorage cache key is namespaced per profile (Hermes WebUI is multi-profile) — if it's a single global key, switching profiles could briefly paint the wrong custom theme before /api/settings returns. Worth a quick check; if it's not currently namespaced that's a small follow-up.

  5. Sienna allowlist fix is a stealth bug-fix. Sienna has CSS in static/style.css, an option in static/index.html, and a test file in master — but it was never added to _SETTINGS_SKIN_VALUES, meaning it currently round-trips as "invalid skin" through /api/settings validation. Adding "sienna" to the set here is correct. Worth pulling that one-liner into the PR description as a noted bonus, or splitting into a separate tiny PR if you want this one to stay strictly about custom themes (your call — both are reasonable).

Things worth confirming

  1. _CUSTOM_THEME_ID_RE = ^[a-z0-9][a-z0-9_-]{0,39}$. Lowercase, alphanumeric, hyphens/underscores, 1–40 chars, must start alphanumeric. Solid. Just confirm in panels.js that the UI lowercases user input before the round trip — otherwise a user typing "MyTheme" gets a confusing "Theme name invalid" from the server with no obvious cause.

  2. Mode coercion to dark on unknown values.

    mode = str(theme.get("mode") or "dark").strip().lower()
    if mode not in {"light", "dark"}:
        mode = "dark"

    Coercing to dark rather than rejecting is forgiving; that's fine. Just be sure the UI knows the value might come back coerced — if a user submits mode: "auto" they'll get mode: "dark" in the response and the picker should reflect that, not silently disagree.

  3. PR is large enough to want UI media before stage review. Per CONTRIBUTING.md, visible UI changes really want screenshots/GIFs attached before maintainer review. Your "Risks / Follow-ups" already calls this out — would you mind dropping a couple of screenshots showing:

    • the Add theme card in the picker
    • the editor expanded with live preview
    • a saved custom theme card with edit/delete affordances
    • a custom theme actually applied to the chat view

    That'd unblock review without requiring a re-pull.

Smaller things

  • _sanitize_custom_theme_token_value does the regex match before the rgb/rgba numeric check. The regex already validates structure, so by the time the numeric check runs you know it's syntactically rgb/rgba. Code reads correctly; a one-line comment "regex validates structure, numeric check validates ranges" would help future readers.

  • theme_id = str(theme.get("id") or "").strip().lower() then regex-checks against _CUSTOM_THEME_ID_RE which is already lowercase-only. So the .lower() is defensive — fine. The empty-string case passes through str().strip().lower() → empty string → regex rejects → returns None. ✅ correct.

  • raw_name[:60] or theme_id — if the trimmed name is empty, fall back to theme_id for display. Sensible.

  • _CUSTOM_THEME_COLOR_RE allows uppercase hex digits but the rgb/rgba branch uses value.lower().startswith(...). The .lower() is harmless on already-lowercase output, but the call shape made me double-check there isn't a "value gets returned uppercase" path. There isn't — value is unchanged through the rgb numeric validation. ✅.

Verdict

Strong feature work and the security model (curated token set + strict color regex + numeric range check + 20-cap + id regex) is well thought through. The main thing blocking stage review is UI media — once you attach screenshots of the new picker and editor, this should be a strong candidate for the next stage queue. Sienna allowlist fix is a nice bonus.

@Michaelyklam
Copy link
Copy Markdown
Contributor Author

Added the requested UI media in follow-up commit bc8e0ec and updated the PR body with screenshots for each requested state:

  • Add theme card in the picker
  • Editor expanded with live preview
  • Lower editor with token controls plus Create theme / Cancel actions
  • Saved custom theme card with Edit/Delete affordances
  • Custom theme applied to the chat view

Raw image links are committed under docs/pr-media/1589/ so they stay attached to the branch instead of depending on local screenshots.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Thanks for the substantial work on this @Michaelyklam — first-class user-defined themes is a real feature win and the design instinct (CSS variable layer as the contract, server-side validation, localStorage cache for first paint) is right.

This PR is large enough (+627/-7, 12 files, new feature surface) and changes a top-bar UI surface visible at all viewport widths, so it needs a UX gate before I can route it into a batch.

What's needed before merge

1. Mobile (390px) screenshots in the PR body

The desktop screenshots are great. We also need 390px iPhone-reference views of:

  • Appearance section with the Add theme card in the theme picker grid (closed/empty state)
  • Add theme flow with the editor expanded — color picker rows, live preview pane
  • Saved custom theme card with edit/delete affordances visible
  • Custom theme applied to chat view — to verify the runtime token application doesn't break responsive layout

The custom theme grid card layout, color picker rows, and the editor panel all need to work at 390px without horizontal overflow or truncated controls.

2. Run the full pytest suite

You noted in "Risks / Follow-ups":

Full pytest tests/ was not run on this rebased PR branch; targeted UI/settings suites passed locally.

Before merge we want a clean full-suite run. There are 4094 tests in current master, and this PR touches api/config.py settings normalization which has cross-cutting test coverage. Please run:

/path/to/.hermes/hermes-agent/venv/bin/python -m pytest tests/ -q

…and post the result count in a comment. If anything fails, we'd want to address before stage.

3. UX maintainer review

Once mobile screenshots land, our UX reviewer will look at the visual results and confirm responsive behavior at the breakpoints. cc @aronprins — flagging this for your queue once @Michaelyklam posts the mobile screenshots.

Things that look right on a structural read

  • Server-side custom theme storage as custom_themes list + custom_theme_id active pointer — good shape, doesn't conflict with the existing theme/skin pair semantics.
  • Color sanitization regex _CUSTOM_THEME_COLOR_RE accepting #hex (3/6/8 chars) and rgb()/rgba() with bounded numeric ranges — defends against url(...) / var(...) / arbitrary CSS injection. Good defense-in-depth.
  • _CUSTOM_THEME_TOKEN_KEYS allowlist scopes which CSS vars a custom theme can touch (no per-component overrides). Good — keeps the design-token contract clean as the PR description argues.
  • Adding sienna to _SETTINGS_SKIN_VALUES — clean addition. Aligns with the project's preference for paint/material naming for accent skins.
  • DESIGN.md update documenting the new contract — appreciated.

What I want to look at more closely once UX gate clears

  • The _sanitize_custom_theme_token_value regex + the rgb/rgba numeric range guards.
  • The localStorage first-paint hook — ordering relative to settings load + sw cache invalidation.
  • The 201-LOC test file tests/test_custom_themes.py — coverage of malicious input rejection (CSS injection attempts), edit/delete idempotency, the multi-theme-active edge cases.
  • Interaction with the existing skin system — what happens when theme=custom and the legacy theme dropdown is opened?

Adding ux label, holding for now. Once mobile screenshots + full-suite test result are posted, I'll do a careful line-level review and Aron will weigh in on the visual side.

Thanks again — this is a meaningful upgrade to the customization surface.

@nesquena-hermes nesquena-hermes added ux User experience / visual polish hold labels May 4, 2026
@Michaelyklam Michaelyklam force-pushed the feat/custom-themes branch from 2b691c0 to 60a489c Compare May 4, 2026 16:35
@Michaelyklam
Copy link
Copy Markdown
Contributor Author

Thanks — I’ve finished the requested follow-up items and updated the PR body.

What changed since the last review comment:

  • Added the requested 390px mobile screenshots to the PR body:
    • Appearance picker with Add theme card
    • Expanded Add theme editor with live preview and color rows
    • Saved custom theme card with Edit/Delete affordances
    • Custom theme applied to the chat view
  • Rebasing/merge-conflict cleanup is done; the previous api/config.py conflict against latest master is resolved and the branch has been force-pushed with lease.
  • Updated Verification to remove the stale “full suite not run” note.

Verification after rebasing onto latest origin/master:

node --check static/boot.js
node --check static/panels.js
node --check static/commands.js
90 passed in 2.22s
4150 passed, 2 skipped, 3 xpassed, 1 warning, 8 subtests passed in 404.47s (0:06:44)

Full-suite command:

env -u HERMES_CONFIG_PATH /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/ -q

The existing raw image links under docs/pr-media/1589/ are committed on the branch, and the PR body now embeds both desktop and mobile evidence. This should be ready for the UX gate / stage review path from my side.

Persist custom theme token bundles through settings, apply them before first paint, and add Appearance UI for creating, editing, deleting, and previewing custom themes.
@Michaelyklam Michaelyklam force-pushed the feat/custom-themes branch from 60a489c to 2e9f392 Compare May 4, 2026 17:21
@Michaelyklam
Copy link
Copy Markdown
Contributor Author

Rebased feat/custom-themes onto latest origin/master and resolved the merge conflict in static/panels.js by preserving upstream's split WebUI/Agent version badges while keeping the custom-theme hydration/cache/render hooks.

New head SHA: 2e9f392f92cb75897de84cedc154502682d969e5

Verification:

  • node --check static/boot.js
  • node --check static/panels.js
  • node --check static/commands.js
  • targeted custom theme/appearance suite: 90 passed in 2.23s
  • full suite: 4188 passed, 2 skipped, 3 xpassed, 1 warning, 8 subtests passed in 404.58s (0:06:44)
  • git diff --check
  • isolated browser smoke test on 127.0.0.1:18793: Appearance shows Add theme with the editor collapsed by default; Add theme expands the editor with preview/color rows; saving creates a My theme card with edit/delete affordances.

Readback after push: GitHub reports mergeable_state: clean; checks test (3.11), test (3.12), and test (3.13) are all successful. Existing PR media raw URLs return HTTP 200. Temporary server stopped and disposable worktree removed.

@nesquena
Copy link
Copy Markdown
Owner

nesquena commented May 4, 2026

Cool idea, thanks for putting this together!

@aronprins Has the concern that this might lead to people creating suboptimal themes, and that curated themes system may be better for our purposes. I'm still thinking on if this is worth including... but wanted to post an update.

@Michaelyklam
Copy link
Copy Markdown
Contributor Author

Imo, since these themes are local, it shouldn't matter if they are suboptimal. they can always swap back to the ones included by default.

@heagandev
Copy link
Copy Markdown

Themes section should be renamed to 'Mode'.
Skins section renamed to 'Themes', add custom themes here <--

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

Labels

hold ux User experience / visual polish

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants