Skip to content

fix(dashboard): apply theme display font to headings + .font-expanded - #44

Merged
exiao merged 1 commit into
live-configfrom
wt/kanban-theme-route
Jun 26, 2026
Merged

fix(dashboard): apply theme display font to headings + .font-expanded#44
exiao merged 1 commit into
live-configfrom
wt/kanban-theme-route

Conversation

@exiao

@exiao exiao commented Jun 26, 2026

Copy link
Copy Markdown
Owner

What

Make a dashboard theme's typography.fontDisplay actually render — on content
headings and on brand chrome (page titles, the DS .font-expanded utility),
including on the /kanban plugin route. A user-YAML theme like cpe-research
(EB Garamond display) now shows its serif headings/titles with no per-theme
!important hack
.

Why (two rendered-DOM-verified bugs)

  1. Dead --theme-font-display. web/src/themes/context.tsx emits it from
    typography.fontDisplay, but nothing in index.css consumed it — every
    theme's fontDisplay was silently inert on headings.
  2. .font-expanded hardcodes the DS Rules Expanded face. The DS utility
    @utility font-expanded { font-family: var(--font-rules-expanded) } in
    @nous-research/ui wins on cascade over any theme variable, so page titles
    (host PageHeaderProvider h1) stayed on Rules Expanded regardless of theme —
    that's why the /kanban page title never picked up the active theme.

The plugin route was never the root cause: the kanban tab mounts inside the host
ThemeProvider and shares document.documentElement. The visible failure was
#1/#2. (Kanban lane names use the plugin's own monospace CSS, not
.font-expanded, and are intentionally left alone.)

How (default look preserved)

New CSS var --theme-font-chrome separates brand-chrome display from
content-heading display:

Default/preset themes (no fontDisplay) keep Rules Expanded; only themes that
ship an explicit fontDisplay override it.

Verification

  • Built web; confirmed both .font-expanded rules emit (DS in
    @layer utilities, ours un-layered) and the heading rule is present.
  • Loaded the real built index.css in headless Chrome, read
    getComputedStyle:
    • DEFAULT theme → .font-expanded = "Rules Expanded", sans-serif (no
      regression), h1 = sans.
    • CPE-RESEARCH theme → .font-expanded = "EB Garamond", Georgia, serif
      (page title serif, no hack); h1 = EB Garamond (dead var now consumed).
  • tsc -p web --noEmit passes. No theme unit tests exist; web vitest fails
    pre-existing/environmental (missing rolldown native binding) identically on
    untouched base, so it's not exercised here.

Follow-up

The personal ~/.hermes/dashboard-themes/cpe-research.yaml .font-expanded
!important block can be dropped once this lands.

Patch note: ~/.hermes/plans/hermes-patches/dashboard-display-font.md

Headings never consumed --theme-font-display, and the DS .font-expanded
utility (page titles, brand chrome) hardcoded Rules Expanded, winning on
cascade over any theme variable. So a user-YAML theme's fontDisplay was
inert on /kanban (and elsewhere), requiring a per-theme !important hack.

- index.css: consume --theme-font-display on h1-h6; re-point .font-expanded
  at a new --theme-font-chrome via an un-layered rule (beats the DS
  @layer utilities rule, no !important).
- context.tsx: emit --theme-font-chrome = fontDisplay ?? var(--font-rules-expanded)
  so default/preset themes keep Rules Expanded and themes with an explicit
  fontDisplay override it.

Verified in headless Chrome against the built CSS: default keeps Rules
Expanded; cpe-research renders EB Garamond on titles + headings.

Patch note: ~/.hermes/plans/hermes-patches/dashboard-display-font.md

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new --theme-font-chrome CSS variable to allow themes to customize the brand-chrome display font (used in page titles and the .font-expanded utility), and configures headings to honor the theme's display font. Feedback points out that applyFontOverride in web/src/themes/context.tsx should be updated to also override --theme-font-chrome when a custom font override is active to prevent visual mismatches.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +101 to +106
// Brand-chrome display font (page titles, the DS `.font-expanded` utility).
// Defaults to the DS Rules Expanded face so the default theme and presets
// that omit `fontDisplay` keep their canonical chrome; a theme that ships
// an explicit `fontDisplay` (e.g. an editorial serif) overrides it here so
// `.font-expanded` honors the theme without a per-theme `!important` hack.
"--theme-font-chrome": typo.fontDisplay ?? "var(--font-rules-expanded)",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While introducing --theme-font-chrome correctly allows themes to customize the brand-chrome display font, there is an issue when a user-selected font override is active.

Currently, applyFontOverride (defined around line 332) only overrides --theme-font-sans and --theme-font-display:

  root.style.setProperty("--theme-font-sans", choice.stack);
  root.style.setProperty("--theme-font-display", choice.stack);

Because it does not override --theme-font-chrome, any page titles or elements using .font-expanded will continue to render with the theme's display font (or the default Rules Expanded face) even when a custom font override is active. This leads to a visual mismatch where the brand chrome does not respect the user's chosen font override.

To fix this, please update applyFontOverride to also override --theme-font-chrome when a custom font is selected:

function applyFontOverride(fontId: string | undefined) {
  if (typeof document === "undefined") return;
  const root = document.documentElement;
  const choice: FontChoice | undefined = getFontChoice(fontId);
  if (!choice) {
    root.style.removeProperty("--theme-font-override-sans");
    return;
  }
  injectFontStylesheet(choice.fontUrl);
  root.style.setProperty("--theme-font-override-sans", choice.stack);
  root.style.setProperty("--theme-font-sans", choice.stack);
  root.style.setProperty("--theme-font-display", choice.stack);
  root.style.setProperty("--theme-font-chrome", choice.stack);
}

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 91e8e23cb4

ℹ️ 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".

Comment thread web/src/index.css
`@utility`/`@layer` rule), so it outranks the DS utility without
`!important`. */
.font-expanded {
font-family: var(--theme-font-chrome);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve user font overrides for chrome headings

When a user selects any dashboard font override from the theme switcher, applyTheme() writes --theme-font-chrome from the active theme and then applyFontOverride() reasserts only --theme-font-sans/--theme-font-display. Since .font-expanded now consumes --theme-font-chrome, page titles and other DS chrome ignore the active font override, and custom themes with fontDisplay will force their display face back over the user's chosen font; reassert or clear the chrome var alongside the existing display override.

Useful? React with 👍 / 👎.

Comment thread web/src/index.css
Without this rule the variable is computed but never consumed, and every
theme's `fontDisplay` is silently inert. */
h1, h2, h3, h4, h5, h6 {
font-family: var(--theme-font-display);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect explicit heading font utilities

Because this global heading rule is unlayered and loaded after the Tailwind/DS imports, it wins over layered font utilities on headings, not just unstyled headings. Existing dashboard section headers such as PluginsPage/AnalyticsPage use <h2>/<h3 className="font-mondwest ..."> for chrome labels; with the default theme --theme-font-display is the system sans stack, so those explicitly styled labels lose Mondwest instead of only headings without an explicit font honoring the display token.

Useful? React with 👍 / 👎.

@exiao
exiao merged commit 9683df2 into live-config Jun 26, 2026
@exiao
exiao deleted the wt/kanban-theme-route branch June 26, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant