[Improvement] Theme Swapping - #72
Conversation
- theme design vars - set default layout - set default theme - use design vars in todo app - update README - add theme & layout to appserver - retain appearance for legacy usage - add theme & layout to registry and session - 44 tests passing
[Improvement] Todo layout
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
97fdbe0 to
5b526c2
Compare
There was a problem hiding this comment.
Looks great overall Aaron. Having layouts and theme as independent variables is a great idea. A few asks:
-
can you make sure you the documentation reflects this new theme/layout approach? We want to make sure the README and hosted documentation 1) removed references to the old appearance, 2) includes example commands with corresponding images of the expected outputs, 3) a list of available themes/layouts in the documentation somewhere.
-
I'm seeing
INFO: ::1:55320 - "GET /.websim.json HTTP/1.1" 404 Not Foundwhen launching the apps viauv run launch.py apps/theme='mono'Do you know what this websim.json is the server is missing? -
the solarized theme doesn't seem to affect the calendar app or messenger or maps or code editor (see image below). Could we make sure at least part of the background is updated with the solarized theme?
Only todo renders from the design tokens in `open_apps.theme`; calendar, messenger, maps, code editor, and the start page each build a `:root` block of their own from `config/apps/<app>/appearance/`, so an `apps/theme=` selection was invisible to them. Add a bridge rather than rewrite five stylesheets: `legacy_theme_css()` emits the theme's token block, aliases the legacy custom properties onto the tokens they should follow, and adds the shared page chrome plus the few per-app selectors whose colors are hard-coded. It returns "" on the `default` theme, so a deployment that never selects one renders exactly as before. Each app emits it after its own stylesheet and per-request, so `reconfigure` theme swaps take effect. Also give every app a `theme: null` field (inherit the global selection, override with `apps.<app>.theme=`), and extend save_screenshots.py with a `theme_<stem>` variation per theme file. Test plan: uv run -m pytest tests/ -> 724 passed, 7 skipped.
The default theme's font-family token was "'Times New Roman', serif", so every app migrated to design tokens (todo, and the online shop) rendered in the browser's serif while the appearance-based apps kept the Arial they had always used. The two looked like different products side by side. Point the token at the same sans stack the appearance configs used. Varying the font deliberately is what the `mono` and `challenging_font` themes are for. Test plan: uv run -m pytest tests/ -> 724 passed, 7 skipped.
mkdocs renders every page it finds under docs/, not just the ones listed in nav. docs/internal-*.md are untracked by design (.gitignore line 201), but a plain `mkdocs build` was rendering them into site/ -- and inlining their full text into site/search/search_index.json and their URLs into site/sitemap.xml, both of which *are* tracked and both of which push to a public remote. The .gitignore rule covers the generated site/internal-* pages themselves but cannot help with the two index files, since those are single tracked blobs that mix internal and public content.
…p layout
Roll the theme/layout split out to every app and delete the `appearance`
group. Previously only todo rendered from design tokens; the other six
apps built their own `:root` block from `config/apps/<app>/appearance/`,
and `legacy_theme_css()` re-pointed those variables at the shared tokens
so a theme selection was at least partly visible. Both are now gone.
`appearance` was conflating three separate things, so each key moved to
where it belongs:
* look -> shared design tokens in `config/apps/theme/` (global)
* structure -> per-app `config/apps/<app>/layout/`
* behaviour -> plain keys in `config/apps/<app>/default.yaml`
(maps zoom/granularity/allow_planning, code editor
mode/highlight/sort_feature, shop enable/credit cards)
One `apps/theme=dark` now themes all seven apps; the dark variation in
`config_parallel_tasks.yaml` drops from six overrides to one.
Themes carry an app-agnostic `assets` block (`tone`, `icon_set`) for the
choices a CSS variable cannot reach -- the start page's raster icons, the
Leaflet tile layer, the CodeMirror stylesheet. Each app maps `tone` onto
its own concrete asset, so a shared theme file never has to grow a key
per app. Without it, `apps/theme=dark` would leave a bright OpenStreetMap
basemap and a light `eclipse` editor pane inside dark chrome.
Fixed along the way:
* `apps.code_editor.theme` silently clobbered CodeMirror's theme. Both
the per-app design-token override and CodeMirror's stylesheet were
named `theme`, and the override won, so the editor initialised with
`theme: 'None'`. CodeMirror's is now `editor_theme`.
* `dark` and `mono` were unusable as global themes -- literal ports of
todo's old local variant, `dark` had a white `color-bg` *and* a white
`color-fg`, `mono` was black on black. Invisible while only todo read
the tokens, broken the moment six more apps did.
* `layers['{{ default_layer }}'].addTo(map)` threw a TypeError on an
unknown layer name, aborting the rest of the script and leaving a
blank map with no landmarks or sidebar wiring. Now falls back.
* `font-size: "{{ base_font_size }}"` in map.html was quoted, so it was
invalid CSS and the setting never applied.
* A missing semicolon in map.html's `.search-btn` swallowed the next
declaration.
Also adds a `colorblind` theme (generalised from the code editor's
`colorblind_access` variant), a greyscale `CartoDB Positron` basemap for
`mono`, and a test that every theme declares the same token vocabulary --
an app uses `var(--token)` without knowing which theme is active, so a
missing token renders as an empty value rather than a loud failure.
Note the reference screenshots need regenerating: `save_screenshots.py`
now captures `theme_*` and `layout_*` variations instead of the old
appearance names.
Test plan: `uv run -m pytest tests/` -> 732 passed, 7 skipped (was 724/7;
+8 new theme/asset tests). ruff unchanged at 292 pre-existing errors with
an identical rule histogram. Served all six routes under `dark` and
`mono` and checked the emitted CSS: correct tokens everywhere, no stale
variables, no unrendered Jinja placeholders; `broken_logos` confirmed
detaching icons from their labels.
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared, design-token based theming system (plus theme “assets” for non-CSS choices) so theme selection can be applied globally or overridden per-app, while separating “layout” (structure) from “theme” (look). It updates several apps, tests, the MCP surface, and documentation to adopt the new apps/theme + per-app layout model.
Changes:
- Add
open_apps.themeutilities to resolve/load themes, render CSS tokens, and expose theme “assets” (tone/icon set) for apps. - Migrate apps/configs away from per-app
appearancegroups to globalapps/themeand per-applayout, updating MCPreconfigure+ variant listing accordingly. - Add/refresh tests and docs to cover precedence/fallback and to reflect the new variation axes.
Reviewed changes
Copilot reviewed 86 out of 87 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_theme.py | New unit tests for theme loading, precedence, assets, and safe token rendering. |
| tests/test_mcp.py | Update MCP variant listing expectations for layout and shared theme. |
| tests/test_apps.py | Update integration fixture overrides to use apps/theme and per-app theme keys. |
| tests/save_screenshots.py | Update screenshot variations to sweep shared themes and selected layouts. |
| src/open_apps/theme.py | New shared theming module (load/resolve/render tokens + theme assets helpers). |
| src/open_apps/mcp/session.py | Update MCP session reconfigure signature to theme/layout/content. |
| src/open_apps/mcp/server.py | Update MCP tools docstrings and params for theme/layout/content. |
| src/open_apps/mcp/registry.py | Teach list_variants that theme is a shared group under config/apps/theme. |
| src/open_apps/mcp/README.md | Document MCP API changes (reconfigure, list_variants) and semantics. |
| src/open_apps/mcp/appserver.py | Update live reconfigure override strings to apps/theme + per-app layout. |
| src/open_apps/apps/todo_app/main.py | Switch todo styling to token-based CSS + per-request theme_style(). |
| src/open_apps/apps/start_page/main.py | Add theme-aware tile colors/icons and emit per-request theme CSS via PageWrapper. |
| src/open_apps/apps/start_page/helper.py | Remove inline font styling and add theme CSS injection support in PageWrapper. |
| src/open_apps/apps/onlineshop_app/templates/html_generator.py | Replace config-driven colors/fonts with theme token CSS and Bootstrap surface overrides. |
| src/open_apps/apps/onlineshop_app/models/global_state.py | Store full apps config node to resolve shared theme from onlineshop templates. |
| src/open_apps/apps/onlineshop_app/main.py | Pass apps config into onlineshop global state for theme resolution. |
| src/open_apps/apps/messenger_app/main.py | Replace appearance-driven CSS vars with token-based component styles + per-request theme block. |
| src/open_apps/apps/map_app/templates/map.html | Convert template styling to design tokens; add themed tile layer fallback; inject theme CSS. |
| src/open_apps/apps/map_app/main.py | Select basemap by theme “tone” and pass rendered theme CSS to template. |
| src/open_apps/apps/codeeditor_app/main.py | Tokenize UI chrome; map shared theme tone to CodeMirror theme; persist in-page override. |
| src/open_apps/apps/calendar_app/main.py | Replace config-generated CSS with token-based styles + per-request layout vars. |
| site/tasks/index.html | Published docs update to include “Goal Variations” section. |
| site/sitemap.xml | Regenerate sitemap (timestamps/conflict cleanup). |
| site/search/search_index.json | Regenerate search index reflecting theme/layout docs changes. |
| site/index.md | Docs update: split appearance into Theme/Layout, update examples and parallel launch docs. |
| site/index.html | Generated HTML reflecting updated site/index.md. |
| site/agents/index.html | Generated HTML: new cluster eval instructions. |
| mkdocs.yml | Exclude internal-*.md from builds to avoid leaking untracked internal docs into site/. |
| docs/index.md | Source docs update mirroring site/index.md theme/layout guidance. |
| config/config.yaml | Add apps/theme: default to Hydra defaults. |
| config/config_parallel_tasks.yaml | Add apps/theme: default and simplify dark variation to a single global theme override. |
| config/apps/todo/layout/kanban_board.yaml | New per-app layout variant for todo. |
| config/apps/todo/layout/default.yaml | New default layout group for todo. |
| config/apps/todo/default.yaml | Replace appearance default with layout default; add per-app theme: null override field. |
| config/apps/todo/appearance/kanban_board.yaml | Remove legacy appearance variant. |
| config/apps/todo/appearance/default.yaml | Remove legacy appearance variant. |
| config/apps/todo/appearance/dark_theme.yaml | Remove legacy appearance variant. |
| config/apps/todo/appearance/challenging_font.yaml | Remove legacy appearance variant. |
| config/apps/todo/appearance/black_and_white.yaml | Remove legacy appearance variant. |
| config/apps/theme/default.yaml | New shared token vocabulary + default assets. |
| config/apps/theme/dark.yaml | New shared dark token palette + dark assets. |
| config/apps/theme/mono.yaml | New shared monochrome token palette + mono assets. |
| config/apps/theme/solarized.yaml | New shared solarized token palette + light assets. |
| config/apps/theme/material.yaml | New shared material-like token palette + light assets. |
| config/apps/theme/bootstrap.yaml | New shared bootstrap-like token palette + light assets. |
| config/apps/theme/challenging_font.yaml | New shared token palette with hard-to-read typography axis. |
| config/apps/theme/colorblind.yaml | New shared colorblind-safe palette generalized from prior editor variant. |
| config/apps/start_page/layout/default.yaml | Move typography/colors to shared theme; keep structure/geometry in layout; add icon sets. |
| config/apps/start_page/layout/clickable_logos.yaml | New/updated layout variant enabling clickable logo headers. |
| config/apps/start_page/layout/broken_logos.yaml | New layout variant to detach icons from tiles (shuffle). |
| config/apps/start_page/default.yaml | Replace appearance default with layout default; add per-app theme: null. |
| config/apps/start_page/appearance/default.yaml | Remove legacy appearance variant. |
| config/apps/start_page/appearance/dark_theme.yaml | Remove legacy appearance variant. |
| config/apps/start_page/appearance/challenging_font.yaml | Remove legacy appearance variant. |
| config/apps/start_page/appearance/black_and_white.yaml | Remove legacy appearance variant. |
| config/apps/onlineshop/layout/default.yaml | New default layout group for onlineshop. |
| config/apps/onlineshop/default.yaml | Replace appearance default with layout default; add per-app theme: null; keep behavior keys. |
| config/apps/onlineshop/appearance/default.yaml | Remove legacy appearance variant. |
| config/apps/onlineshop/appearance/dark_theme.yaml | Remove legacy appearance variant. |
| config/apps/onlineshop/appearance/challenging_font.yaml | Remove legacy appearance variant. |
| config/apps/onlineshop/appearance/black_and_white.yaml | Remove legacy appearance variant. |
| config/apps/messenger/layout/default.yaml | New default layout group for messenger. |
| config/apps/messenger/default.yaml | Replace appearance default with layout default; add per-app theme: null. |
| config/apps/messenger/appearance/default.yaml | Remove legacy appearance variant. |
| config/apps/messenger/appearance/dark_theme.yaml | Remove legacy appearance variant. |
| config/apps/messenger/appearance/challenging_font.yaml | Remove legacy appearance variant. |
| config/apps/messenger/appearance/black_and_white.yaml | Remove legacy appearance variant. |
| config/apps/maps/layout/default.yaml | New default layout group for maps. |
| config/apps/maps/default.yaml | Replace appearance default with layout default; add per-app theme: null; move behavior keys here; add layer_by_tone. |
| config/apps/maps/appearance/default.yaml | Remove legacy appearance variant. |
| config/apps/maps/appearance/dark_theme.yaml | Remove legacy appearance variant. |
| config/apps/maps/appearance/challenging_font.yaml | Remove legacy appearance variant. |
| config/apps/maps/appearance/black_and_white.yaml | Remove legacy appearance variant. |
| config/apps/code_editor/layout/default.yaml | New default layout group for code editor. |
| config/apps/code_editor/default.yaml | Replace appearance default with layout default; add per-app theme: null; define tone→CodeMirror theme mapping. |
| config/apps/code_editor/appearance/default.yaml | Remove legacy appearance variant. |
| config/apps/code_editor/appearance/dark_theme.yaml | Remove legacy appearance variant. |
| config/apps/code_editor/appearance/colorblind_access.yaml | Remove legacy appearance variant (folded into shared colorblind theme). |
| config/apps/code_editor/appearance/challenging_font.yaml | Remove legacy appearance variant. |
| config/apps/code_editor/appearance/black_and_white.yaml | Remove legacy appearance variant. |
| config/apps/calendar/layout/default.yaml | New default layout group for calendar (container width/spacing/button padding). |
| config/apps/calendar/default.yaml | Replace appearance default with layout default; add per-app theme: null. |
| config/apps/calendar/appearance/default.yaml | Remove legacy appearance variant. |
| config/apps/calendar/appearance/dark_theme.yaml | Remove legacy appearance variant. |
| config/apps/calendar/appearance/challenging_font.yaml | Remove legacy appearance variant. |
| config/apps/calendar/appearance/black_and_white.yaml | Remove legacy appearance variant. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| safe_lines: list[str] = [] | ||
| for key, value in tokens.items(): | ||
| key = str(key) | ||
| # Allow only simple custom-property names to avoid broken CSS/injection. | ||
| if (not key) or any(not (c.isalnum() or c in "-_") for c in key): | ||
| continue | ||
| val = str(value).replace("\n", " ").replace("\r", " ") | ||
| safe_lines.append(f" --{key}: {val};") |
| def available_themes() -> list[str]: | ||
| """Theme stems under ``config/apps/theme/``, default first.""" | ||
| stems = sorted(path.stem for path in THEME_DIR.glob("*.yaml")) | ||
| return ["default"] + [stem for stem in stems if stem != "default"] |
| {custom_css} | ||
|
|
||
| {theme_css} | ||
| </style> |
KarenUllrich
left a comment
There was a problem hiding this comment.
@marksibrahim This change will make the results in the paper not reproducible anymore, I am wondering if we should version these changes? Or are we ok with that?
Provides the ability to swap theme on a per-app basis, current implementation address To-Do App only with other apps still using legacy appearance.
Example theme swap:
(All apps)
uv run launch_agent.py apps/theme=solarized
(Single app)
apps/todo/theme=solarized
Themes available currently:
bootstrap
challenging_font
dark
material
mono
solarized