Skip to content

Commit 83122ad

Browse files
fix(preview): adopt main's reveal-theme-on-slides on rebase (bd-y259zb57) + pin with test
Rebasing onto main surfaced the one semantic conflict in entry.tsx: our branch suppressed the compiled theme link on reveal decks (currentDocIsSlides/ setDocIsSlides), which main's bd-y259zb57 deliberately removed so the compiled Quarto reveal theme applies to slides too. Adopt main's behaviour: reconcileThemeLink() applies lastThemeCssUrl unconditionally; drop currentDocIsSlides + setDocIsSlides and the now-dead onDocIsSlides prop/effect in PreviewRoot. Pin it with a new fail-on-revert integration test that drives a slides UPDATE_AST + UPDATE_THEME and asserts the data-q2-theme link IS applied (RED under the old suppression, GREEN now).
1 parent 02283fd commit 83122ad

5 files changed

Lines changed: 119 additions & 56 deletions

File tree

crates/wasm-quarto-hub-client/Cargo.lock

Lines changed: 23 additions & 20 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

hub-client/changelog.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ be in reverse chronological order (latest first).
3535
- [`24df47d1`](https://github.com/quarto-dev/q2/commits/24df47d1): Revert the G8 marker-aware list resolution — it hijacked every tight list-item click/hover to the parent list (a tight item's text is a bare node, so text clicks also report `e.target===<li>`), breaking per-item editing. Tight list items hover/click as items again.
3636
- [`04440c47`](https://github.com/quarto-dev/q2/commits/04440c47): Add Playwright e2e spec: T13(c) crumb jump relands collapsed (proven RED-on-revert). (The G8 marker spec from this commit was removed in 24df47d1; the G6/G7 settle-gate e2e was omitted — that timing-race guard is bound deterministically at the jsdom tier, so a browser test of it would pass vacuously.)
3737
- [`5e20b96e`](https://github.com/quarto-dev/q2/commits/5e20b96e): G4/G11/G12: bare modifier keys no longer trigger expand-on-edit; a second click inside an open editor now expands it; overflow-y is now 'auto' only when content genuinely clips (no scrolljack on expanded/fitting editors).
38-
39-
### 2026-06-16
40-
4138
- [`63b463b6`](https://github.com/quarto-dev/q2/commits/63b463b6): Fix G3 ArrowDown on single-line tight list items: exclude paddingBottom from isOnLastVisualLine height comparison; add G1/G3 Playwright + jsdom tests (T18/T19/T21).
4239
- [`2efb0bde`](https://github.com/quarto-dev/q2/commits/2efb0bde): Fix dirty nest-in/out caret column to apply `prefixWidth` adjustment (Principle A), matching the clean path so the column lands at the same source position after a dirty round-trip.
4340

ts-packages/preview-renderer/src/q2-preview/PreviewRoot.tsx

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,6 @@ export interface PreviewRootProps {
169169
* `LOAD_CUSTOM_COMPONENTS`). Tests pass `{}` (the default).
170170
*/
171171
customRegistry?: Record<string, React.ComponentType<any>>;
172-
/**
173-
* Optional callback called when the document format switches to/from
174-
* slides. In production, `entry.tsx` passes `setDocIsSlides` to
175-
* reconcile the Bootstrap CSS link. Tests can ignore it (default no-op).
176-
*/
177-
onDocIsSlides?: (isSlides: boolean) => void;
178172
/**
179173
* Optional callback called when `scrollToAnchorInDocument` would be called.
180174
* In production, `entry.tsx` provides the real scroll logic.
@@ -1401,12 +1395,6 @@ export function PreviewRoot(props: PreviewRootProps) {
14011395
const previewFormat = parsed ? extractMetaString(parsed.meta?.format) : undefined;
14021396
const isSlides = previewFormat === 'q2-slides' || previewFormat === 'revealjs';
14031397

1404-
// Notify caller (entry.tsx) about format switch so it can reconcile the Bootstrap CSS link.
1405-
const onDocIsSlides = props.onDocIsSlides;
1406-
useEffect(() => {
1407-
onDocIsSlides?.(isSlides);
1408-
}, [isSlides, onDocIsSlides]);
1409-
14101398
// Build SourceInfo-value index from the untransformed AST (Plan 2a).
14111399
const sourceIndex = useMemo(
14121400
() => buildSourceIndex(props.untransformedAstJson),
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Iframe-side theme-on-slides behaviour (bd-y259zb57 parity).
3+
*
4+
* Unlike `entry.integration.test.tsx` (which mocks `createRoot` and only
5+
* exercises the module-top `UPDATE_THEME` → `<link data-q2-theme>` path for a
6+
* NON-slide doc), this test drives the REAL render path: a slides `UPDATE_AST`
7+
* mounts the real `PreviewRoot` (so the active doc genuinely IS a reveal deck),
8+
* then an `UPDATE_THEME` arrives. It pins the contract that the compiled theme
9+
* link IS applied on a reveal deck, matching `q2 render`.
10+
*
11+
* Fail-on-revert: re-introducing the old slide-theme suppression (a
12+
* `currentDocIsSlides` gate making `reconcileThemeLink()` call `applyTheme(null)`
13+
* on slides) yields no `<link data-q2-theme>` → this test goes RED.
14+
*/
15+
16+
// @vitest-environment jsdom
17+
18+
import { describe, it, expect, beforeAll, afterEach, vi } from 'vitest';
19+
import { act } from '@testing-library/react';
20+
21+
// RevealDeck (rendered when the AST is a slide deck) imports @revealjs/react
22+
// and the vendored reveal CSS — stub them so mounting is light in jsdom. Do
23+
// NOT mock react-dom/client: we need the real render so the active doc is a
24+
// genuine reveal deck when UPDATE_THEME arrives.
25+
vi.mock('@revealjs/react', () => ({ Deck: () => null, Slide: () => null }));
26+
vi.mock('../../../../resources/revealjs/reset.css', () => ({}));
27+
vi.mock('../../../../resources/revealjs/reveal.css', () => ({}));
28+
vi.mock('../../../../resources/revealjs/theme/white.css', () => ({}));
29+
vi.mock('../../../../resources/revealjs/quarto-reveal.css', () => ({}));
30+
vi.mock('katex/dist/katex.min.css', () => ({}));
31+
32+
beforeAll(async () => {
33+
// Side-effect import: registers the module-top message listener.
34+
await import('./entry');
35+
});
36+
37+
afterEach(() => {
38+
document.head
39+
.querySelectorAll('link[data-q2-theme]')
40+
.forEach((el) => el.remove());
41+
});
42+
43+
// A minimal slide-deck AST: meta.format = "revealjs" makes PreviewRoot's
44+
// `isSlides` true (extractMetaString reads MetaString.c), so the mounted doc
45+
// is a reveal deck.
46+
const SLIDES_AST = JSON.stringify({
47+
'pandoc-api-version': [1, 23, 0],
48+
meta: { format: { t: 'MetaString', c: 'revealjs' } },
49+
blocks: [],
50+
});
51+
52+
async function postMessageAndFlush(data: unknown) {
53+
await act(async () => {
54+
window.dispatchEvent(new MessageEvent('message', { data }));
55+
await Promise.resolve();
56+
});
57+
}
58+
59+
function themeLinks(): NodeListOf<HTMLLinkElement> {
60+
return document.head.querySelectorAll('link[data-q2-theme]');
61+
}
62+
63+
describe('q2-preview/entry theme link on a slide deck', () => {
64+
it('applies the theme link on a slide deck (no suppression)', async () => {
65+
// jsdom has no #root by default; updateAst needs it to mount.
66+
const root = document.createElement('div');
67+
root.id = 'root';
68+
document.body.appendChild(root);
69+
try {
70+
// 1. Slides AST → real PreviewRoot mounts → isSlides effect →
71+
// entry.setDocIsSlides(true).
72+
await postMessageAndFlush({ type: 'UPDATE_AST', payload: { astJson: SLIDES_AST, currentFilePath: 'deck.qmd' } });
73+
// 2. Theme arrives for the (slide) document.
74+
await postMessageAndFlush({ type: 'UPDATE_THEME', cssUrl: 'blob:slide-theme' });
75+
76+
// The compiled theme MUST reach the deck (bd-y259zb57): one link
77+
// carrying the url. (Pre-fix: suppressed on slides → zero links.)
78+
const links = themeLinks();
79+
expect(links).toHaveLength(1);
80+
expect(links[0].getAttribute('href')).toBe('blob:slide-theme');
81+
} finally {
82+
root.remove();
83+
}
84+
});
85+
});

ts-packages/preview-renderer/src/q2-preview/entry.tsx

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -246,29 +246,20 @@ function applyTheme(cssUrl: string | null): void {
246246
link.setAttribute('href', cssUrl);
247247
}
248248

249-
// bd-ibqkf9ry: the HTML-theme (Bootstrap) CSS must NOT reach a reveal deck.
250-
// A deck ships its own complete CSS (`resources/revealjs/…`, imported by
251-
// `RevealDeck`), exactly like `q2 render`'s standalone deck — so leaking the
252-
// HTML theme (e.g. Bootstrap's `h2 { border-bottom }`) onto slides is a render/
253-
// preview divergence. `UPDATE_THEME` (theme bytes) and `UPDATE_AST` (which
254-
// reveals the format) arrive on separate messages in either order, so we
255-
// remember the last theme URL + whether the active doc is a slide deck and
256-
// reconcile: attach the `data-q2-theme` link only for non-slide documents.
249+
// bd-y259zb57: the `UPDATE_THEME` channel carries the active document's
250+
// *compiled theme*. For an HTML page that's Bootstrap; for a `format: revealjs`
251+
// deck it's the compiled Quarto reveal theme, delivered through the SAME
252+
// `css:theme:<fp>` → styles.css transport. Both must be applied as the
253+
// `<link data-q2-theme>` so preview matches render.
254+
//
255+
// (Previously this suppressed the theme link on slides, because the preview
256+
// only ever produced Bootstrap CSS — never the reveal theme — and reveal decks
257+
// fell back to a hard-coded stock `white.css` import in `RevealDeck`. That was
258+
// the centered/uppercase render↔preview divergence this strand fixes.)
257259
let lastThemeCssUrl: string | null = null;
258-
let currentDocIsSlides = false;
259260

260261
function reconcileThemeLink(): void {
261-
applyTheme(currentDocIsSlides ? null : lastThemeCssUrl);
262-
}
263-
264-
/**
265-
* Record whether the active document is a reveal deck and re-reconcile the
266-
* HTML-theme link. Driven by the render component's `isSlides` effect, so a
267-
* format switch (html ⇄ revealjs) re-applies or removes the theme correctly.
268-
*/
269-
function setDocIsSlides(isSlides: boolean): void {
270-
currentDocIsSlides = isSlides;
271-
reconcileThemeLink();
262+
applyTheme(lastThemeCssUrl);
272263
}
273264

274265
/**
@@ -354,7 +345,6 @@ function updateAst(payload: UpdateAstPayload) {
354345
unlockNestingCursor={unlockNestingCursor}
355346
nestedEditBuffers={nestedEditBuffers}
356347
customRegistry={customRegistry}
357-
onDocIsSlides={setDocIsSlides}
358348
scrollToAnchor={scrollToAnchorInDocument}
359349
onNavigateToDocument={(path, anchor) => {
360350
window.parent.postMessage(

0 commit comments

Comments
 (0)