Repository navigation
Add an app theme construct with Classic as the default - #290
salman1993 wants to merge 4 commits into
Conversation
A realistic agent-drafted design doc for judging the reading experience in both themes: prose, headings, lists, tasks, quote, table, footnote, image, SQL/Ruby/YAML/diff code, a long scrolling line, and flowchart and sequence diagrams. Seeds open, agent-answered, resolved, diagram-node, table-cell, and general discussions so highlights are in place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A theme is a named set of CSS variable values applied with <html data-ui-theme>. Classic, the current look, sets no variables: every rule a theme can change reads var(--hook, <Classic value>), so Classic renders exactly as before. Hosts pick the site default with config.default_theme. Decks reset the hooks to keep their built-in look, and Mermaid can take a per-theme palette from CSS variables. See docs/THEMES.md for the hooks and how to add a theme. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Light, dark and system are now a "mode" in docs, comments and the contributor checklists, so "theme" means only the app theme. Slide looks are always a "deck theme". THEMES.md gains a short glossary. The persisted and host-visible mode names (data-theme, theme_preference, theme_controller.js, coplan:theme-changed) stay, with comments at their definitions. The new attribute is renamed from data-ui-theme to data-app-theme before it ships. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Themes.default now returns a Theme, and an unknown config.default_theme stops boot in development and test, naming the bad value and the valid keys. Other environments log a warning and fall back to Classic. Mermaid reads the palette from each diagram's container instead of <html>, so decks, which reset --diagram-bg, keep the built-in themes without a separate deck flag. The configuration cache keys on the palette's colors. An expanded deck diagram keeps the slide's background, so its built-in colors stay readable in dark mode. A static spec checks that every variable a theme file sets is reset in .deck or the deck's code panel, or is allowlisted with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
screenshots for "Classic" theme (same as it is today): 
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3db559eecd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the label size that survives (or doesn't) being scaled to fit. | ||
| fontFamily, | ||
| themeVariables: { fontSize: "16px", fontFamily, ...(dark && darkThemeVariables()) }, | ||
| themeVariables: { fontSize: "16px", fontFamily, ...(palette ? paletteThemeVariables(palette, dark) : dark && darkThemeVariables()) }, |
There was a problem hiding this comment.
When an app theme overrides --font-sans (explicitly supported by docs/THEMES.md), this configuration passes the root-derived appFontStack() to every Mermaid render, including diagrams inside .deck. The rest of each slide continues using its fixed --deck-font-* values, so diagram labels unexpectedly adopt the app theme and the supposedly theme-isolated deck changes appearance. Derive the font from the diagram's context—using the deck font inside slides—and include it in the Mermaid configuration cache key.
Useful? React with 👍 / 👎.
What changed and why
CoPlan has one look today: Lexend, glass panels and a document card. Some readers want a quieter reading view, but Lexend is there on purpose (it helps readers with dyslexia), so a new look has to be an opt-in theme rather than a change for everyone. This PR adds the theme system. Classic, today's look, is the only theme, and nothing else on screen changes apart from one bug fix (below).
<html data-app-theme="classic">.var(--hook, <Classic value>). Classic sets no hooks, so it renders exactly as before.docs/THEMES.mdlists about 70 hooks: page, glass, floating windows, panels, document card, plan title, reading type, code blocks, diagrams and tables.config.default_theme(default"classic"). The registry isCoPlan::Themes. An unknown name stops boot in development and test, naming the bad value and the valid keys. In production it logs a warning and falls back to Classic..deckresets the hooks to their built-in values and themes never restyle slides.spec/lib/deck_theme_isolation_spec.rbchecks that every variable a theme file sets is reset in the deck or allowlisted with a reason.main): an expanded deck diagram now keeps the slide's background. Before, in dark mode, acoplandeck diagram's light Mermaid colors sat on the dark expander and its dark edges nearly disappeared.data-theme,theme_preference,theme_controller.js,coplan:theme-changed) stay as they are, with comments where they are defined.reading-specimendemo document, which has every Markdown element, code in four languages and two diagrams. It is the test page for this stack.The next PR in the stack adds a second theme, Neutral, and a per-user Theme picker in Settings.
Evidence
Apart from the expanded deck diagram fix, no visible change. I took full-page screenshots before (
bd30add) and after this PR and diffed them pixel by pixel, in light and dark mode. The pages were the reading specimen (desktop and 390px phone), the editor, an open comment thread, home, Settings, the demo library, the 404 page, a page with embedded decks, a flowchart and the tables demo. All match. The only differences left are the random presence-avatar color and the sticky nav caught mid-fade.The diff caught one regression, which is fixed here: the new reading rule for paragraphs outranked the rule that pins Mermaid node labels to
line-height: 1.2, and labels moved down about 6px.How to try it
bin/rails db:seed, then sign in at http://localhost:3000/sign_in as hampton@example.com (no password)./hampton/demo-content/rich-text/reading-specimen-webhook-delivery-through-a-durable-outbox. It should look exactly likemain, and<html>should carrydata-app-theme="classic".config.default_theme = "nope"in the host's CoPlan config and runbin/rails runner 1. Boot stops withconfig.default_theme is "nope", which is not a theme. Valid themes: classic.Testing
bundle exec rspec: 3064 examples, 0 failures.The screenshot diff above.
Added or updated real JavaScript-enabled browser specs in
spec/system/for changed UI behavior, covering the user interaction and visible result; or explained why no browser UI is involved. (No visible change; the screenshot diff covers it.)Added model, service, or request specs where needed for domain behavior, authorization, or API cases.
Ran
bundle exec rspec, or explained why it could not run and listed the specs that did run.Tested affected UI behavior in both light and dark mode, or explained why mode testing does not apply.
Attached a video or images demonstrating the change, if this PR adds or changes a feature or fixes a UI bug. (The expanded deck diagram fix needs before/after images; to add.)
🤖 Generated with Claude Code