Skip to content

Commit 650cbdd

Browse files
cscheidclaude
andcommitted
fix(revealjs): apply the document's compiled reveal theme in q2 preview (Level 2)
`q2 preview` of a `format: revealjs` deck never applied the Quarto reveal theme: it showed reveal's stock `white.css` (centered text, uppercase headings) regardless of `theme:` / `_brand.yml`, diverging from `q2 render` (left-aligned, non-uppercase, document theme). bd-y259zb57 Bug 2 / Level 2. Root cause + fixes: - CompileThemeCssStage gated its reveal branch on `identifier == Revealjs`, but the preview pseudo-format `q2-slides` has `identifier == Html`, so the preview compiled Bootstrap, not the reveal theme. Gate is now `is_revealjs_target(target_format)` (true for both `revealjs` render and `q2-slides` preview; render output unchanged). Preview delivers the compiled theme through the existing `css:theme:<fp>` -> styles.css transport the SPA already reads (Q2PreviewIframe -> UPDATE_THEME); render still links the full `css:revealjs:*` asset set via site_libs. - MetadataMergeStage flattened `format.html.*` for `q2-slides`, burying `format.revealjs.{theme,brand,...}` — so *named* themes and brand silently fell back to the default theme in preview. It now maps the reveal preview to the `revealjs` base format, matching `q2 render`. - quarto-hub discovery never synced `_brand.yml` into the preview VFS (only `_quarto.yml`/`_metadata.yml` were recognized as config), so brand decks died with "Path not found: /project/_brand.yml" — for HTML brand decks too. Added `_brand.yml`/`_brand.yaml`. - Preview SPA: entry.tsx no longer suppresses the theme link for slides (the theme is now the reveal theme, not Bootstrap); RevealDeck drops its static `white.css` import so the per-document compiled theme applies instead. E2E verified (`q2 preview` + Chrome computed styles == `q2 render`) for default, named (`dark`), and `_brand.yml` decks. Tests: compile_theme_css (1), pipeline (3), quarto-hub discovery (1). `cargo xtask verify` green. Single-file `q2 preview deck.qmd` with a sibling `_brand.yml` still can't resolve brand (single-file mode excludes siblings by design) — tracked as bd-ggvq1j68. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6b553b9 commit 650cbdd

9 files changed

Lines changed: 405 additions & 60 deletions

File tree

claude-notes/plans/2026-06-16-revealjs-render-preview-theme-parity.md

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -66,24 +66,75 @@ async). So the preview can compile the reveal theme on WASM.
6666

6767
### Work (each TDD; verify through real `q2 preview` + Chrome)
6868

69-
- [ ] **L2.1 (Rust).** Make the preview pipeline compile the document's reveal
70-
theme. The reveal branch must fire for the reveal-preview pseudo-format.
71-
Decide the detection signal (target_format `q2-slides` /
72-
`is_revealjs_target`, vs `ast.meta` `format: revealjs`) since
73-
`format.identifier` is `Html` in preview. Register the compiled theme so
74-
the WASM extractor + VFS flush find it (either reuse `css:theme:<fp>` +
75-
the styles.css artifact path, or extend the extractor for the reveal key).
76-
- [ ] **L2.2 (WASM).** Ensure `render_page_for_preview` surfaces the reveal
77-
theme (fingerprint + bytes flushed to the VFS artifact path the SPA reads).
78-
- [ ] **L2.3 (hub-client).** The preview reveal renderer must consume the theme
79-
(UPDATE_THEME / prop), inject it as a stylesheet, and **drop the static
80-
`white.css` import** (keep reset + reveal core + quarto-reveal). Bundle the
81-
Source Sans Pro `@import` comes along via the compiled theme automatically.
82-
Files: `ts-packages/preview-renderer/src/q2-preview/RevealDeck.tsx`,
83-
`Q2PreviewIframe.tsx`, q2-preview-spa entry. **hub-client change → two-commit
84-
changelog; WASM/SPA rebuild required.**
85-
- [ ] **L2.4.** Tests + E2E parity check (`q2 render` vs `q2 preview` on a deck
86-
with default, a named theme, and `_brand.yml` — all should match).
69+
- [x] **L2.1 (Rust).** DONE. Gate changed from `identifier == Revealjs` to
70+
`is_revealjs_target(&ctx.format.target_format)` in `CompileThemeCssStage`
71+
(`compile_theme_css.rs`), so the reveal branch fires for the `q2-slides`
72+
preview pseudo-format (identifier `Html`) as well as `revealjs` render.
73+
**Delivery split**: render (`revealjs`) still calls
74+
`register_reveal_assets` (linkable `css:revealjs:*` set via site_libs);
75+
preview (`q2-slides`) calls `store_css` so the compiled reveal theme rides
76+
the *existing* `css:theme:<fp>` → styles.css transport the SPA already
77+
consumes. Compile-failure fallback → `stock_reveal_theme_css()` (vendored
78+
white). Tests: `reveal_preview_delivers_compiled_theme_via_css_theme_artifact`
79+
(compile_theme_css.rs), `q2_preview_pipeline_compiles_reveal_theme_for_slides`
80+
(pipeline.rs, exercises the full preview pipeline). Render output unchanged
81+
(all 2372 quarto-core tests pass).
82+
- [x] **L2.2 (WASM).** DONE — **no WASM code change required** for the single-doc
83+
path (the `q2 preview` target use case). Because L2.1 routes the reveal
84+
theme through `css:theme:<fp>`, `extract_theme_fingerprint` (looks for
85+
`css:theme:`) and `flush_artifacts_to_vfs` (writes styles.css) already
86+
surface it via `render_page_for_preview``RenderResponse.theme_fingerprint`.
87+
(Project-of-decks path uses the multi-doc `quarto/quarto-theme-<fp>.css`
88+
artifact path rather than styles.css — out of scope for the single-deck
89+
"done" criteria; tracked as a follow-up if needed.)
90+
- [x] **L2.3 (hub-client).** DONE. `entry.tsx`: removed the slides theme-link
91+
suppression (`currentDocIsSlides`/`setDocIsSlides`) — `reconcileThemeLink`
92+
now always applies `lastThemeCssUrl` (for slides that URL is the compiled
93+
reveal theme, not Bootstrap). `RevealDeck.tsx`: dropped the static
94+
`theme/white.css` import (kept reset + reveal + quarto-reveal); the
95+
per-document theme arrives via the `UPDATE_THEME``<link data-q2-theme>`
96+
transport. quarto-reveal.css is theme-independent/additive, so the runtime
97+
theme link landing after it does not change the cascade.
98+
**hub-client change → two-commit changelog; WASM/SPA rebuild required.**
99+
- [x] **L2.4.** DONE. Tests + E2E parity check across **default, named (`dark`),
100+
and `_brand.yml`** — all three match `q2 render`.
101+
- **Rust tests** (`cargo nextest`, all green; full workspace 10147 pass):
102+
- `compile_theme_css.rs`: `reveal_preview_delivers_compiled_theme_via_css_theme_artifact`.
103+
- `pipeline.rs`: `q2_preview_pipeline_compiles_reveal_theme_for_slides`,
104+
`…_compiles_named_reveal_theme_for_slides` (guards the metadata-flattening
105+
fix), `…_compiles_brand_reveal_theme_for_slides` (real tempdir `_brand.yml`).
106+
- `quarto-hub/discovery.rs`: `test_discover_brand_file`.
107+
- **Two extra fixes surfaced during E2E** (both were silent before because the
108+
default theme needs no `theme:`/brand lookup):
109+
1. **Metadata flattening** (`metadata_merge.rs`): `q2-slides` flattened
110+
`format.html.*`, burying `format.revealjs.{theme,brand,…}`. Now maps the
111+
reveal preview to the `revealjs` base format. Without this, *named* themes
112+
and brand silently fell back to the default theme in preview.
113+
2. **`_brand.yml` VFS sync** (`quarto-hub/discovery.rs`): the preview server
114+
never synced `_brand.yml` into the VFS (only `_quarto.yml`/`_metadata.yml`
115+
were recognized as config), so brand resolution died with "Path not found:
116+
/project/_brand.yml" — for HTML brand decks too, not just reveal. Added
117+
`_brand.yml`/`_brand.yaml` to config-file discovery.
118+
- **E2E evidence** (`cargo run --bin q2 -- preview <deck>`, computed styles read
119+
from the live iframe via Chrome DevTools; compared to `q2 render`'s compiled
120+
`theme-*.css`):
121+
- **default** (`.e2e-reveal/default.qmd`): heading `text-transform: none`,
122+
content `text-align: left`, `font-family: "Source Sans Pro"`, color `#222`
123+
matches render. `white.css` no longer statically imported; theme arrives via
124+
`<link data-q2-theme>`.
125+
- **dark** (`theme: dark`): `--r-background-color: #191919`, viewport bg
126+
`rgb(25,25,25)`, `--r-main-color: #fff` — matches render `#191919`.
127+
- **brand** (`_brand.yml`, run as a project): `--r-background-color: #fdf6ff`,
128+
`--r-main-color: #2a1a3a`, `--r-heading-color: #6f42c1`, `--r-main-font:
129+
Georgia`; h1 computed `rgb(111,66,193)` — all match render exactly.
130+
- **Known limitations (out of scope / follow-ups):**
131+
- **Single-file `q2 preview deck.qmd` + sibling `_brand.yml`**: the `bd-tnm3k`
132+
single-file watcher deliberately does NOT pull in siblings, so brand only
133+
resolves when the deck lives in a *project* (`_quarto.yml` present). Same
134+
for HTML brand decks. Tracked separately.
135+
- **`[Q-1-20] "Failed to parse metadata value as markdown"`** on `brand:
136+
_brand.yml` — pre-existing, appears identically in render *and* preview (so
137+
itself a parity success); unrelated to theming.
87138

88139
### Notes / gotchas
89140

crates/quarto-core/src/pipeline.rs

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2514,6 +2514,163 @@ mod tests {
25142514
);
25152515
}
25162516

2517+
/// bd-y259zb57 (L2.1): the q2-preview pipeline — the exact path
2518+
/// `q2 preview` drives in WASM — must compile the deck's reveal theme and
2519+
/// expose it through the standard `css:theme:<fp>` artifact, so the SPA's
2520+
/// existing `theme_fingerprint` + styles.css transport delivers it. Before
2521+
/// the fix the `q2-slides` pseudo-format (identifier `Html`) took the
2522+
/// Bootstrap branch, so the preview shipped Bootstrap and reveal decks fell
2523+
/// back to stock `white.css` (centered, uppercase) instead of the Quarto
2524+
/// reveal theme (left-aligned, non-uppercase).
2525+
#[test]
2526+
fn q2_preview_pipeline_compiles_reveal_theme_for_slides() {
2527+
let content = b"---\ntitle: Slides Test\nformat: revealjs\n---\n\n\
2528+
## First\n\n- a\n- b\n";
2529+
2530+
let project = make_test_project();
2531+
let doc = DocumentInfo::from_path("/project/test.qmd");
2532+
let format = Format::from_format_string("q2-slides").unwrap();
2533+
let binaries = BinaryDependencies::new();
2534+
let mut ctx = RenderContext::new(&project, &doc, &format, &binaries);
2535+
2536+
let runtime = make_test_runtime();
2537+
pollster::block_on(render_qmd_to_preview_ast(
2538+
content,
2539+
"test.qmd",
2540+
&mut ctx,
2541+
runtime,
2542+
None,
2543+
Vec::new(),
2544+
))
2545+
.expect("q2-slides render");
2546+
2547+
// The compiled reveal theme is delivered via `css:theme:<fp>` (the same
2548+
// artifact key + styles.css path the SPA already reads), NOT the
2549+
// linkable `css:revealjs:*` set (that's the render path's site_libs
2550+
// delivery — the SPA bundles reset/reveal/quarto-reveal itself).
2551+
let theme_entries = ctx.artifacts.get_by_prefix("css:theme:");
2552+
assert_eq!(
2553+
theme_entries.len(),
2554+
1,
2555+
"q2-slides preview should produce exactly one css:theme artifact"
2556+
);
2557+
let css = theme_entries[0].1.as_str().expect("theme CSS is UTF-8");
2558+
assert!(
2559+
css.contains(".reveal"),
2560+
"preview must compile the reveal theme (scoped under .reveal), not Bootstrap"
2561+
);
2562+
assert!(
2563+
!css.contains(".navbar"),
2564+
"preview reveal theme must not be Bootstrap (.navbar present)"
2565+
);
2566+
assert!(
2567+
ctx.artifacts.get_by_prefix("css:revealjs:").is_empty(),
2568+
"preview should not register linkable css:revealjs:* assets"
2569+
);
2570+
}
2571+
2572+
/// bd-y259zb57 (L2.1): a *named* reveal theme nested under
2573+
/// `format.revealjs.theme` must reach the preview's compiled theme. This
2574+
/// guards the metadata-flattening half of the fix: the `q2-slides`
2575+
/// pseudo-format has identifier `Html`, so `MetadataMergeStage` would
2576+
/// flatten `format.html.*` (burying `theme:`) unless it maps the reveal
2577+
/// preview back to the `revealjs` base format. Before that fix the preview
2578+
/// silently compiled the *default* theme for every named theme/brand.
2579+
#[test]
2580+
fn q2_preview_pipeline_compiles_named_reveal_theme_for_slides() {
2581+
// `theme: dark` lives under `format.revealjs.theme`, exactly as a real
2582+
// deck authors it.
2583+
let content = b"---\ntitle: Dark\nformat:\n revealjs:\n theme: dark\n---\n\n\
2584+
## First\n\nBody.\n";
2585+
2586+
let project = make_test_project();
2587+
let doc = DocumentInfo::from_path("/project/test.qmd");
2588+
let format = Format::from_format_string("q2-slides").unwrap();
2589+
let binaries = BinaryDependencies::new();
2590+
let mut ctx = RenderContext::new(&project, &doc, &format, &binaries);
2591+
2592+
let runtime = make_test_runtime();
2593+
pollster::block_on(render_qmd_to_preview_ast(
2594+
content,
2595+
"test.qmd",
2596+
&mut ctx,
2597+
runtime,
2598+
None,
2599+
Vec::new(),
2600+
))
2601+
.expect("q2-slides render");
2602+
2603+
let theme_entries = ctx.artifacts.get_by_prefix("css:theme:");
2604+
assert_eq!(theme_entries.len(), 1, "expected one css:theme artifact");
2605+
let css = theme_entries[0].1.as_str().expect("theme CSS is UTF-8");
2606+
// The reveal `dark` theme sets a dark background; the default theme is
2607+
// white (`#fff`). Asserting the dark background proves the named theme
2608+
// was resolved (not silently defaulted).
2609+
assert!(
2610+
css.contains("#191919"),
2611+
"named theme `dark` must reach the compiled preview theme \
2612+
(expected dark background #191919); got default theme?"
2613+
);
2614+
}
2615+
2616+
/// bd-y259zb57 (L2.1): a `_brand.yml` reveal deck previewed as `q2-slides`
2617+
/// must fold the brand into the compiled theme — same as `q2 render`. This
2618+
/// exercises the reveal branch's brand resolution (`resolve_brand_layers`
2619+
/// against `ctx.project.dir`) through the preview pipeline, plus the
2620+
/// `format.revealjs.brand` metadata flattening. (E2E `q2 preview` of a brand
2621+
/// deck is additionally gated on the preview server syncing `_brand.yml`
2622+
/// into the VFS — a separate, pre-existing infra gap that affects HTML brand
2623+
/// previews identically; tracked in its own strand.)
2624+
#[test]
2625+
fn q2_preview_pipeline_compiles_brand_reveal_theme_for_slides() {
2626+
// Real tempdir so the NativeRuntime can read `_brand.yml` from disk,
2627+
// exactly as the reveal branch resolves it against the project dir.
2628+
let dir = tempfile::TempDir::new().unwrap();
2629+
let root = dir.path().to_path_buf();
2630+
std::fs::write(
2631+
root.join("_brand.yml"),
2632+
"color:\n palette:\n purple: \"#6f42c1\"\n primary: purple\n \
2633+
background: \"#fdf6ff\"\n foreground: \"#2a1a3a\"\n",
2634+
)
2635+
.unwrap();
2636+
2637+
let content = b"---\ntitle: Brand\nformat:\n revealjs:\n brand: _brand.yml\n---\n\n\
2638+
## First\n\nBody.\n";
2639+
2640+
let project = ProjectContext {
2641+
dir: root.clone(),
2642+
config: crate::project::ProjectConfig::default(),
2643+
is_single_file: true,
2644+
files: vec![DocumentInfo::from_path(root.join("deck.qmd"))],
2645+
output_dir: root.clone(),
2646+
};
2647+
let doc = DocumentInfo::from_path(root.join("deck.qmd"));
2648+
let format = Format::from_format_string("q2-slides").unwrap();
2649+
let binaries = BinaryDependencies::new();
2650+
let mut ctx = RenderContext::new(&project, &doc, &format, &binaries);
2651+
2652+
let runtime = make_test_runtime();
2653+
pollster::block_on(render_qmd_to_preview_ast(
2654+
content,
2655+
root.join("deck.qmd").to_str().unwrap(),
2656+
&mut ctx,
2657+
runtime,
2658+
None,
2659+
Vec::new(),
2660+
))
2661+
.expect("q2-slides brand render");
2662+
2663+
let theme_entries = ctx.artifacts.get_by_prefix("css:theme:");
2664+
assert_eq!(theme_entries.len(), 1, "expected one css:theme artifact");
2665+
let css = theme_entries[0].1.as_str().expect("theme CSS is UTF-8");
2666+
// Brand background colour folded into the reveal theme proves the brand
2667+
// reached the compiled output (default reveal background is `#fff`).
2668+
assert!(
2669+
css.contains("#fdf6ff"),
2670+
"brand background must reach the compiled preview theme; got default?"
2671+
);
2672+
}
2673+
25172674
/// Phase 1 (target-incremental-writes): `render_qmd_to_preview_ast` must
25182675
/// return *both* the transformed AST (`ast_json`) and the untransformed
25192676
/// AST (`untransformed_ast_json`). An unchanged paragraph must have

crates/quarto-core/src/revealjs/assemble.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ const QUARTO_REVEAL_CSS: &str = include_str!("../../../../resources/revealjs/qua
3838
/// (`crate::revealjs::theme`) defaults to this; `white` aliases to it.
3939
pub const DEFAULT_THEME: &str = "default";
4040

41+
/// The vendored stock reveal theme CSS (`white.css`), used as the theme-slot
42+
/// fallback when Quarto's reveal-theme SCSS compilation fails so a deck still
43+
/// renders. Exposed so the `q2 preview` path can deliver the same fallback
44+
/// through its `css:theme:` transport (see `CompileThemeCssStage`'s reveal
45+
/// branch).
46+
pub fn stock_reveal_theme_css() -> &'static str {
47+
THEME_WHITE_CSS
48+
}
49+
4150
/// Kind of a vendored reveal asset (decides the `css:` / `js:` artifact key
4251
/// prefix and the emitted tag).
4352
#[derive(Debug, Clone, Copy, PartialEq, Eq)]

crates/quarto-core/src/revealjs/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ mod slides;
2626
mod theme;
2727
mod transform;
2828

29-
pub use assemble::{DEFAULT_THEME, register_reveal_assets, render_revealjs_document};
29+
pub use assemble::{
30+
DEFAULT_THEME, register_reveal_assets, render_revealjs_document, stock_reveal_theme_css,
31+
};
3032
pub use auto_stretch::RevealAutoStretchTransform;
3133
pub use columns::RevealColumnsTransform;
3234
pub use footer_logo::{RevealFooterAliasTransform, RevealFooterLogoTransform};

0 commit comments

Comments
 (0)