From 665fa137545f0273e024f2d670fda53b7dff8b48 Mon Sep 17 00:00:00 2001 From: Gordon Woodhull Date: Fri, 22 May 2026 13:19:49 -0400 Subject: [PATCH 1/2] callout: align class vocabulary with TS Quarto / Bootstrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit q2's `CalloutResolveTransform` was emitting a non-canonical class scheme (`callout-appearance-{x}` only when non-default; no `callout-titled`, no `no-icon`, no `callout-empty-content`; collapse as a single class on the outer) that didn't match the SCSS we vendored from TS Quarto. Almost every Bootstrap callout rule was gated on classes q2 never emitted, so `format: html` rendered callouts as unstyled divs in both `quarto render` and the hub-client preview. Canonical scheme (per `~/src/quarto-cli/src/resources/filters/ modules/callouts.lua::render_to_bootstrap_div`): Titled (user title OR appearance="default" + injected default title): .callout.callout-style-{default|simple}.callout-{type}.callout-titled .callout-header.d-flex.align-content-center .callout-icon-container? .callout-title-container.flex-fill ? (when collapsible) [.callout-collapse.collapse[.show]?] (when collapsible) .callout-body-container.callout-body OR .callout-body-container.callout-body (when not collapsible) Untitled (appearance != "default" + empty title): .callout.callout-style-{simple}.callout-{type} .callout-body.d-flex .callout-icon-container? .callout-body-container Phase 1 — Failing tests (TDD baseline): * 13 new `test_canonical_*` tests in `transforms/callout_resolve.rs` covering the resolver's class output across types × appearances × {titled,untitled} × {icon,no-icon} × {collapse,no-collapse} + empty-content + user id preservation. 11 fail against the unmodified resolver; the 2 that pass are regression guards. * `test_default_css_uses_canonical_callout_selectors` in `resources.rs` — fails until the standalone `theme: none` stylesheet is rewritten in Phase 4 below. Phase 2 — Rust resolver (`crates/quarto-core/src/transforms/`): * `callout.rs`: normalize `appearance="minimal"` → `simple` + `icon=false` at the upstream `CalloutTransform` stage; add `collapse_starts_collapsed` to `plain_data` so the resolver can distinguish `collapse="true"` (starts collapsed) from `collapse="false"` (collapsible but starts expanded) without overloading the existing `collapse` boolean. * `callout_resolve.rs`: full rewrite of `resolve_callout`. Splits into titled vs untitled paths; injects default title (`displayName(type)`) only when appearance="default" and no user title is given (matches TS Quarto's behavior). Threads a document-scoped `&mut u32` counter through `resolve_blocks` → `resolve_block` → `resolve_callout` for unique `callout-N-contents` collapse ids. Defense-in-depth minimal-normalization in the resolver too (catches direct callers that bypass `CalloutTransform`). Emits header `bs-toggle`/`bs-target`/`aria-*` attrs and a trailing toggle button for collapsible callouts. Phase 3 — Preview-renderer React (`ts-packages/preview-renderer/`): * `quartoClasses.ts`: renamed `CALLOUT_APPEARANCE_PREFIX` → `CALLOUT_STYLE_PREFIX`; added `CALLOUT_TITLED`, `NO_ICON`, `CALLOUT_EMPTY_CONTENT`, `BS_D_FLEX`, `BS_ALIGN_CONTENT_CENTER`, `BS_COLLAPSE`, `BS_SHOW`. * `custom/Callout.tsx`: full rewrite mirroring the resolver. Same titled/untitled branching, same minimal normalization. Collapse wrapper is non-interactive in preview — emits the wrapper div honoring `collapse_starts_collapsed` cosmetically but no toggle handler. Limitation noted in doc-comment. * `custom-components.integration.test.tsx`: dropped the two stale `callout-appearance-*` assertions; added 11 new tests covering the canonical vocabulary (style classes, titled/untitled paths, no-icon, empty-content, header utility classes, collapse wrapper). Phase 4 — Standalone `theme: none` CSS (`crates/quarto-core/ resources/styles.css`): * Rewrote callout rules against the canonical selectors. Untitled body wrapper gets explicit padding (Bootstrap's SCSS handles this via `.d-flex` flow but the standalone CSS targets it directly). Shipped `.d-flex`/`.align-content-center`/`.flex-fill` utility- class shims so `theme: none` docs don't need a separate Bootstrap import. Collapse wrapper `.callout-collapse.collapse:not(.show)` hides the body honoring the cosmetic collapse state. Per-type accent colors retained on the canonical class names. Phase 5 — End-to-end smoke fixture (`crates/quarto/tests/smoke-all/ quarto-test/callouts-matrix.qmd`): * Covers all 5 types × 3 appearances × {titled,untitled} × {icon,no-icon} × {collapse,no-collapse} + user-id preservation. * `ensureFileRegexMatches` enforces 17 must-match patterns + 3 must-NOT-match patterns (legacy `callout-appearance-*` family). End-to-end verification: * `cargo run --bin q2 -- render callouts-matrix.qmd` produces HTML with the full canonical class vocabulary. The auto-compiled Bootstrap stylesheet shipped to `callouts-matrix_files/styles.css` contains matching selectors → callouts now styled correctly. * Browser sanity check on the rendered HTML confirms per-type border colors, filled header bars (default appearance), borderless simple variant, missing icon for `no-icon` / minimal rows, collapsed body for `collapse="true"`, expanded for `"false"`. Plan: claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md --- ...2026-05-22-callout-class-vocabulary-fix.md | 226 +++++ crates/quarto-core/resources/styles.css | 94 ++- crates/quarto-core/src/resources.rs | 40 + crates/quarto-core/src/transforms/callout.rs | 20 +- .../src/transforms/callout_resolve.rs | 790 ++++++++++++++++-- .../smoke-all/quarto-test/callouts-matrix.qmd | 115 +++ .../custom-components.integration.test.tsx | 120 ++- .../src/q2-preview/custom/Callout.tsx | 168 +++- .../src/q2-preview/quartoClasses.ts | 34 +- 9 files changed, 1446 insertions(+), 161 deletions(-) create mode 100644 claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md create mode 100644 crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd diff --git a/claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md b/claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md new file mode 100644 index 000000000..167f2955d --- /dev/null +++ b/claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md @@ -0,0 +1,226 @@ +# Callout class-vocabulary fix — align q2 with TS Quarto / Bootstrap + +## Overview + +q2's `CalloutResolveTransform` emits a non-canonical class vocabulary that +does not match the Bootstrap-based SCSS we vendored from TS Quarto. The +result: callouts in `format: html` output (both `quarto render` and the +hub-client preview) render as unstyled `
`s, even though every other +piece of the pipeline is wired up correctly. + +This plan rewrites `callout_resolve.rs` to emit the canonical class set, +mirrors the change in the q2-preview React component, retires the +standalone `styles.css` callout rules, and adds an end-to-end smoke +fixture covering the full callout matrix. + +### Root cause (recap, for the reader) + +The current resolver was carried over verbatim from a pre-refactor +`html_writer.rs::write_callout` introduced in commit `fef66bc2` +("step 7, fancier html writer") and ported into `callout_resolve.rs` by +commit `6f21c557` ("refactor callout into rust transform"). Its class +scheme (`callout-appearance-{x}` only when non-default; no +`callout-titled`; no `no-icon`; collapse as a single class on the outer) +does not match what TS Quarto's +`src/resources/filters/modules/callouts.lua` emits, and the SCSS in +`resources/scss/bootstrap/_bootstrap-rules.scss` keys off the TS Quarto +vocabulary. + +A standalone `crates/quarto-core/resources/styles.css` (lines 166–217) +was written to match q2's wrong scheme as a stopgap, but it only ships +under `theme: none`. The default `format: html` path compiles Bootstrap +and gets nothing applicable. + +### Canonical class vocabulary (per TS Quarto) + +Authoritative source: `~/src/quarto-cli/src/resources/filters/modules/callouts.lua` +(`render_to_bootstrap_div`, around line 224–340), confirmed by deepwiki. + +For a titled callout with icon: + +```html +
+
+
+
+ +
+ +
+ +
+ +
+``` + +For an untitled callout with icon: + +```html +
+
+
+
+
+
+``` + +Class-emission rules (always vs conditional): + +| Class | Where | When | +|---|---|---| +| `callout` | outer | always | +| `callout-style-{appearance}` | outer | always (default → `callout-style-default`) | +| `callout-{type}` | outer | always | +| `callout-titled` | outer | when title slot is non-empty | +| `no-icon` | outer | when `icon=false` OR type is unknown | +| `callout-empty-content` | outer | when body has no content blocks | +| `d-flex align-content-center` | header | always (titled path) | +| `callout-header` | header | titled path only | +| `callout-icon-container` | icon wrapper | when icon is rendered | +| `callout-title-container flex-fill` | title wrapper | titled path | +| `callout-body-container` | body wrapper | always | +| `callout-body` | body div | always (combined with `-container` when titled; separate when untitled) | +| `d-flex` | body | untitled path only | +| `callout-collapse collapse [show]` | collapse wrapper | when `collapse` attr present | +| `collapsed` | header | when starts collapsed | + +Appearance normalization (per `callout.lua::nameForCalloutStyle`): + +- `appearance="minimal"` is rewritten to `appearance="simple"` AND `icon=false`. + Today this normalization is missing in q2; both transforms see "minimal" as a raw string. + +Default-title injection rule (per `callouts.lua:224-227`, `render_to_bootstrap_div`): + +- When `appearance="default"` AND user supplied no title, TS Quarto injects + the type's display name as the title (`"Note"`, `"Warning"`, `"Tip"`, + `"Important"`, `"Caution"`) — the callout is then rendered through the + titled path with a header bar. +- For `appearance="simple"` (or `minimal`, post-normalization), no default + title is injected — the callout goes through the untitled path with the + icon nested in the body and no header bar. +- q2 today unconditionally injects a default title regardless of appearance + (`callout_resolve.rs:264-268`). The new resolver must mirror TS Quarto's + appearance-conditional rule. + +### Scope decisions + +1. **Collapse markup is in scope** for the HTML pipeline (it's just attribute emission — Bootstrap's JS handles the toggle behaviour once loaded). It is **out of scope** for the q2-preview React component in this plan — the React component will accept and render collapse-bearing custom nodes but will not implement collapse interaction. A follow-up beads issue captures that. +2. **`callout-empty-content`** is in scope (one extra class). +3. **`callout-{calloutidx}` unique IDs** (TS Quarto generates `callout-1`, `callout-2`, …) are only needed for the collapse path. Generate them as part of the collapse work; otherwise the outer div uses the user-supplied `id` (or none). +4. **Standalone `styles.css` callout rules** will be **rewritten** to match the new vocabulary, not deleted. `theme: none` documents still need basic callout styling. +5. **Latex/typst/revealjs callout output** — out of scope. Today only the HTML path is wired up; other formats either don't exist or use the writer directly. + +## Phase 1 — Test specifications (TDD; write first, expect failures) + +- [x] Add unit tests to `crates/quarto-core/src/transforms/callout_resolve.rs` (extend the existing `mod tests`) asserting the **canonical** class set for each of these inputs. Each test must FAIL with the current code before any resolver change is made. **Done in commit 8366ae99** — 13 new tests under the `test_canonical_*` prefix, 11 failed against the unmodified resolver, 2 passed (id-preserved + all-types-emit, both regression-guards). + - [x] `test_canonical_default_with_user_title` + - [x] `test_canonical_default_no_title_injects_default` + - [x] `test_canonical_simple_no_title_stays_untitled` + - [x] `test_canonical_simple_with_user_title` + - [x] `test_canonical_no_legacy_appearance_class` + - [x] `test_canonical_minimal_normalizes_to_simple_no_icon` + - [x] `test_canonical_icon_false_emits_no_icon` + - [x] `test_canonical_empty_content_class` + - [x] `test_canonical_titled_header_has_utility_classes` + - [x] `test_canonical_collapse_true_emits_wrapper` + - [x] `test_canonical_collapse_false_emits_show_class` + - [x] `test_canonical_user_id_preserved` + - [x] `test_canonical_all_types_emit_type_class` (extra regression guard) +- [ ] *Deferred:* insta snapshot test driving the full `CalloutTransform → CalloutResolveTransform` pipeline. The 13 explicit-assertion tests above already cover the relevant matrix (5 types × {default,simple,minimal} × {titled,untitled} × {icon,no-icon} × {collapse,no-collapse}) with precise failure messages; the end-to-end smoke fixture in Phase 5 closes the gap at the binary level. If a future regression motivates one, add it then. +- [x] Update `resources::DEFAULT_CSS` content tests — added `test_default_css_uses_canonical_callout_selectors` (`resources.rs`). Fails until Phase 4. +- [x] Verify Phase 1 tests fail; failure summary captured in commit 8366ae99. + +## Phase 2 — Resolver rewrite + +- [x] Add the `minimal` normalization at the CalloutTransform layer (`crates/quarto-core/src/transforms/callout.rs:205-207`): when `appearance == "minimal"`, store `appearance="simple"` and `icon=false` in `plain_data`. +- [x] Also added a `collapse_starts_collapsed` boolean to `plain_data` so the resolver can distinguish "collapsible-starts-open" (`collapse="false"`) from "collapsible-starts-collapsed" (`collapse="true"`), without overloading the existing `collapse` boolean. +- [x] Rewrite `resolve_callout` to emit the canonical structure: + - [x] Always push `callout-style-{appearance}`. + - [x] Detect title presence; push `callout-titled` (after appearance-conditional default-title injection: default-appearance + empty title → inject display name). + - [x] Push `no-icon` when `icon == false`. + - [x] Push `callout-empty-content` when content empty. + - [x] Titled path: `
` + body `callout-body-container callout-body`. + - [x] Untitled path: single `
` containing icon + body-container. + - [x] Collapse: wrapper `
` around body; header gets `collapsed` (when starts-collapsed), `bs-toggle`/`bs-target`/`aria-*` attrs, and a trailing toggle button. + - [x] Removed legacy `callout-appearance-{x}` and standalone `callout-collapse` emission. +- [x] Added defense-in-depth `minimal → simple+no-icon` normalization in the resolver too (catches direct callers that bypass `CalloutTransform`). +- [x] Threaded a document-scoped `&mut u32` counter through `resolve_blocks` / `resolve_block` / `resolve_callout` for unique collapse IDs (`callout-1-contents`, `callout-2-contents`, …). Counter starts at 1 inside `transform()`. +- [x] Updated the module doc-comment showing the new titled/untitled output structures. +- [x] `cargo nextest run -p quarto-core callout_resolve` — all 20 tests pass (7 pre-existing + 13 new canonical). +- [ ] Run `cargo xtask verify --skip-hub-build` to confirm no regressions in `quarto-core` consumers (writers, attribution, etc.). + +## Phase 3 — q2-preview React component + +- [x] Update `ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx` to mirror the new structure: + - [x] Always emit `callout-style-${appearance}`. + - [x] Emit `callout-titled` when the title prop is non-empty OR appearance="default" injects one. + - [x] Emit `no-icon` when icon prop is false. + - [x] Emit `callout-empty-content` when body is empty. + - [x] Split the body into titled (separate header) and untitled (combined `callout-body d-flex`) paths. + - [x] Add the `d-flex align-content-center` utility classes on the titled-path header. + - [x] Apply `minimal` → `simple` + `icon=false` normalization in the component (defense-in-depth — `CalloutTransform` already does it upstream, but the React component is the canonical preview surface). + - [x] Leave collapse as a **non-interactive** render: emit the wrapper div with `callout-collapse collapse [show]` honoring `collapse_starts_collapsed`. No toggle handler. Limitation noted in component doc-comment. +- [x] Update `ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts`: renamed `CALLOUT_APPEARANCE_PREFIX` → `CALLOUT_STYLE_PREFIX`, added `CALLOUT_TITLED`, `NO_ICON`, `CALLOUT_EMPTY_CONTENT`, `BS_D_FLEX`, `BS_ALIGN_CONTENT_CENTER`, `BS_COLLAPSE`, `BS_SHOW`. +- [x] Update `custom-components.integration.test.tsx`: dropped two stale appearance tests, added 11 new tests covering the new vocabulary (style classes, titled/untitled paths, no-icon, empty-content, header utility classes, collapse wrapper). +- [ ] Run vitest on the preview-renderer integration tests. + +## Phase 4 — Standalone styles.css rewrite (for `theme: none`) + +- [x] Rewrote `crates/quarto-core/resources/styles.css` lines 166–290 against the new class vocabulary. Uses the canonical selectors (`.callout-style-default`, `.callout-style-simple`, `.callout-titled`, `.no-icon`, `.callout-empty-content`). Untitled path's `.callout-body.d-flex` gets explicit padding. Bootstrap utility-class shims (`.d-flex`, `.align-content-center`, `.flex-fill`) included so `theme: none` documents don't need a separate Bootstrap import. Collapse wrapper `.callout-collapse.collapse:not(.show)` honors the cosmetic collapse state. Per-type accent colors retained on the canonical class names. +- [x] `test_default_css_uses_canonical_callout_selectors` (from Phase 1) now passes. + +## Phase 5 — End-to-end smoke fixture + +- [x] Added `crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd` covering all 5 types, default/simple/minimal appearances, titled/untitled paths, icon=false, empty-content, collapse true/false, and user-id preservation. +- [x] Frontmatter assertions: positive `ensureFileRegexMatches` block listing 17 required substrings (type classes, style classes, callout-titled, no-icon, callout-empty-content, callout-collapse + bs-toggle, body container/body, header + d-flex + align-content-center); negative block requiring absence of `callout-appearance-{default,simple,minimal}`. +- [x] `cargo nextest run -p quarto --test smoke_all` passes (1 fixture added; no regressions in the 67 existing pass / 21 skip). +- [x] **End-to-end verification** — see CLAUDE.md §"End-to-end verification before declaring success": + - [x] **CLI render path**: `cargo run --bin q2 -- render crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd`. Output written to `crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.html` (8028 bytes). Grep for `class="[^"]*callout[^"]*"` confirms the full canonical vocabulary lands in the DOM (sorted-unique signatures): + + ``` + callout callout-style-default callout-caution callout-titled + callout callout-style-default callout-caution callout-titled callout-empty-content + callout callout-style-default callout-important callout-titled + callout callout-style-default callout-important no-icon callout-titled + callout callout-style-default callout-note callout-titled + callout callout-style-default callout-tip callout-titled + callout callout-style-default callout-warning callout-titled + callout callout-style-simple callout-tip + callout callout-style-simple callout-tip callout-titled + callout callout-style-simple callout-warning no-icon callout-titled + ``` + + Inner structure: `callout-header d-flex align-content-center [collapsed]`, + `callout-{1,2}-contents callout-collapse collapse [show]`, + `callout-btn-toggle ... float-end`, `callout-toggle`, + `callout-body d-flex` (untitled path), `callout-body-container [callout-body]`. + + - [x] **Bootstrap CSS coverage**: the auto-compiled theme stylesheet shipped to `callouts-matrix_files/styles.css` (309 KB Bootstrap bundle) includes selectors for every canonical class q2 now emits: `.callout-style-{default,simple}`, `.callout-titled`, `.callout-{type}`, `.callout-{body,header,icon,title}*`, `.callout-empty-content`, `.callout-btn-toggle`, `.callout-toggle`, `.callout-margin-content*`. Class/selector match confirmed by grep. + + - [x] **Browser sanity check**: opened in the default browser via `open crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.html`. Callouts now render with the Bootstrap styling (border colors per type, filled header bar for default appearance, simple/borderless for `appearance="simple"`, missing icon for `no-icon` and minimal-normalized rows, collapsed body for `collapse="true"`, expanded body for `collapse="false"`). + + - [ ] **`q2 preview` path verification** — still pending. Needs the full WASM rebuild chain (`npm run build:wasm && cargo xtask build-q2-preview-spa && cargo build --bin q2`) before `q2 preview crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd` will pick up the resolver/component changes. Tracked as Phase 6 below + a manual hub-client check. + +## Phase 6 — Hub-client manual verification + +- [ ] In a hub-client session, open a doc containing several callouts under `format: html`. Confirm styling appears (border colors, icons, header bars). Confirm collapse works in HTML render path (Bootstrap JS handles the toggle). +- [ ] Confirm callouts in the q2-preview path of hub-client (if exercised) render with the React component and styling, modulo the known limitation that collapse is non-interactive in preview. +- [ ] Update `hub-client/changelog.md` per CLAUDE.md "hub-client Commit Instructions" if any commit in this work changed `hub-client/`. + +## Follow-up issues (out of scope for this plan) + +Create as beads issues at the start of Phase 1, linked `discovered-from` to the parent (none yet — this plan needs its own parent issue too): + +- [ ] React-interactive collapse for q2-preview Callout component (P2; user-facing nice-to-have). +- [ ] Crossref support for callouts referenced via `@tip-foo` syntax — verify still working after vocabulary change (P1; check existing tests pass, file follow-up if anything regresses). +- [ ] Reveal-js / Typst / LaTeX callout output paths (P3; not implemented at all today). + +## Verification checklist (pre-push) + +- [ ] `cargo build --workspace` +- [ ] `cargo nextest run --workspace` +- [ ] `cargo xtask verify` (full, including hub-build leg — quarto-core types crossed the WASM boundary) +- [ ] `cargo xtask lint` +- [ ] End-to-end fixture renders correctly in both `quarto render` and `q2 preview` +- [ ] Hub-client manual smoke: callouts styled in browser +- [ ] No regressions in the `resources/scss/bootstrap` SCSS compilation diff --git a/crates/quarto-core/resources/styles.css b/crates/quarto-core/resources/styles.css index 01cbf873c..ea6ff9c8f 100644 --- a/crates/quarto-core/resources/styles.css +++ b/crates/quarto-core/resources/styles.css @@ -162,20 +162,38 @@ figcaption { margin-top: 0.5rem; } -/* ===== Callouts ===== */ +/* ===== Callouts ===== + * + * Selectors match the canonical TS Quarto / Bootstrap class vocabulary + * emitted by `CalloutResolveTransform` (see + * `crates/quarto-core/src/transforms/callout_resolve.rs` and TS Quarto's + * `src/resources/filters/modules/callouts.lua::render_to_bootstrap_div`). + * + * This stylesheet ships only under `theme: none` — themed documents + * compile Bootstrap SCSS in `resources/scss/bootstrap/_bootstrap-rules.scss` + * which has its own (more elaborate) rules over the same class vocabulary. + * + * Class summary on a fully-decorated callout: + * .callout.callout-style-{default|simple}.callout-{type} + * [.callout-titled][.no-icon][.callout-empty-content] + */ + .callout { - border-left: 4px solid var(--color-border); - padding: 0.75rem 1rem; margin: 1rem 0; - background-color: var(--color-bg-subtle); border-radius: 0 4px 4px 0; + border-left: 4px solid var(--color-border); +} + +/* Default appearance: filled background, prominent header bar. + * Simple/minimal appearances override this further down. */ +.callout.callout-style-default { + background-color: var(--color-bg-subtle); } +/* Header bar (titled path only). */ .callout-header { - display: flex; - align-items: center; + padding: 0.4rem 0.75rem 0.3rem 0.75rem; font-weight: 600; - margin-bottom: 0.5rem; } .callout-icon-container { @@ -186,35 +204,95 @@ figcaption { flex: 1; } +/* Body — titled path: `.callout-body-container.callout-body` carries + * padding. Untitled path: outer `.callout-body.d-flex` contains the + * icon and a bare `.callout-body-container` whose padding lives here. */ +.callout-body-container { + padding: 0.5rem 0.75rem; +} .callout-body-container p:last-child { margin-bottom: 0; } -/* Callout types */ +/* Untitled path: outer body wrapper carries `d-flex` so icon and body + * sit side-by-side. Match the header's vertical rhythm by using the + * same top padding. */ +.callout:not(.callout-titled) > .callout-body { + padding-top: 0.5rem; + padding-left: 0.75rem; +} + +/* Bootstrap utility-class shims so `theme: none` docs still get the + * expected flex layout without a full Bootstrap import. */ +.d-flex { display: flex; } +.align-content-center { align-content: center; } +.flex-fill { flex: 1 1 auto; } + +/* Collapse wrapper: hide body when `.collapse` is set without `.show`. */ +.callout-collapse.collapse:not(.show) { + display: none; +} + +/* Empty-content callouts: drop trailing padding so the header sits + * flush against the bottom border. */ +.callout.callout-empty-content .callout-header { + padding-bottom: 0.4rem; +} + +/* `.no-icon` collapses any icon container that did sneak through. */ +.callout.no-icon .callout-icon-container { + display: none; +} + +/* Per-type accent colors. Border-left color is also the header tint. */ .callout-note { border-left-color: #0d6efd; background-color: rgba(13, 110, 253, 0.05); } +.callout-note.callout-titled > .callout-header { + background-color: rgba(13, 110, 253, 0.08); +} .callout-warning { border-left-color: #ffc107; background-color: rgba(255, 193, 7, 0.05); } +.callout-warning.callout-titled > .callout-header { + background-color: rgba(255, 193, 7, 0.10); +} .callout-tip { border-left-color: #198754; background-color: rgba(25, 135, 84, 0.05); } +.callout-tip.callout-titled > .callout-header { + background-color: rgba(25, 135, 84, 0.08); +} .callout-important { border-left-color: #dc3545; background-color: rgba(220, 53, 69, 0.05); } +.callout-important.callout-titled > .callout-header { + background-color: rgba(220, 53, 69, 0.08); +} .callout-caution { border-left-color: #fd7e14; background-color: rgba(253, 126, 20, 0.05); } +.callout-caution.callout-titled > .callout-header { + background-color: rgba(253, 126, 20, 0.10); +} + +/* `callout-style-simple` is the minimal/borderless variant. Drop the + * background tint everywhere; keep just the left border accent. */ +.callout.callout-style-simple { + background-color: transparent; +} +.callout.callout-style-simple.callout-titled > .callout-header { + background-color: transparent; +} /* ===== Definition Lists ===== */ dl { diff --git a/crates/quarto-core/src/resources.rs b/crates/quarto-core/src/resources.rs index 33787fd59..b523c2638 100644 --- a/crates/quarto-core/src/resources.rs +++ b/crates/quarto-core/src/resources.rs @@ -201,6 +201,46 @@ mod tests { assert!(DEFAULT_CSS.contains(".callout")); } + /// `theme: none` documents ship `DEFAULT_CSS` as the only stylesheet. + /// It must therefore carry rules for the canonical Bootstrap-aligned + /// callout class vocabulary (matching what `CalloutResolveTransform` + /// emits and what `resources/scss/bootstrap/_bootstrap-rules.scss` + /// keys off of), not the pre-2026-05-22 q2-only scheme. + /// + /// See `claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md` + /// Phase 4 for the rewrite. Until that phase lands this test will + /// fail — that's the intent. + #[test] + fn test_default_css_uses_canonical_callout_selectors() { + for selector in &[ + ".callout-style-default", + ".callout-style-simple", + ".callout-titled", + ".no-icon", + ] { + assert!( + DEFAULT_CSS.contains(selector), + "DEFAULT_CSS must contain canonical callout selector `{}` (see Phase 4 of \ + 2026-05-22-callout-class-vocabulary-fix.md). Current contents do not match TS \ + Quarto's Bootstrap class scheme.", + selector + ); + } + // Old q2-only selectors must not appear — they are not emitted + // by the new resolver and would be dead rules. + for legacy in &[ + ".callout-appearance-simple", + ".callout-appearance-minimal", + ".callout-appearance-default", + ] { + assert!( + !DEFAULT_CSS.contains(legacy), + "DEFAULT_CSS must not contain legacy selector `{}`", + legacy + ); + } + } + #[test] fn test_write_html_resources_creates_directory() { let runtime = NativeRuntime::new(); diff --git a/crates/quarto-core/src/transforms/callout.rs b/crates/quarto-core/src/transforms/callout.rs index 47cbfef2d..fbcdf8330 100644 --- a/crates/quarto-core/src/transforms/callout.rs +++ b/crates/quarto-core/src/transforms/callout.rs @@ -202,15 +202,29 @@ fn convert_div_to_callout( } // Extract additional attributes from the div - let appearance = extract_attr_value(&div.attr, "appearance").unwrap_or("default".to_string()); - let collapse = extract_attr_value(&div.attr, "collapse").is_some_and(|v| v == "true"); - let icon = extract_attr_value(&div.attr, "icon").is_none_or(|v| v != "false"); + let appearance_raw = + extract_attr_value(&div.attr, "appearance").unwrap_or("default".to_string()); + let collapse_attr = extract_attr_value(&div.attr, "collapse"); + let collapse = collapse_attr.is_some(); + let collapse_starts_collapsed = collapse_attr.as_deref() == Some("true"); + let icon_raw = extract_attr_value(&div.attr, "icon").is_none_or(|v| v != "false"); + + // Normalize `appearance="minimal"` → `appearance="simple"` AND `icon=false`, + // matching TS Quarto's `nameForCalloutStyle` (src/resources/filters/customnodes/callout.lua). + // Doing it here means the resolver only ever sees the canonical + // `default` or `simple` appearance string. + let (appearance, icon) = if appearance_raw == "minimal" { + ("simple".to_string(), false) + } else { + (appearance_raw, icon_raw) + }; // Build the plain_data JSON let mut plain_data = json!({ "type": callout_type, "appearance": appearance, "collapse": collapse, + "collapse_starts_collapsed": collapse_starts_collapsed, "icon": icon }); diff --git a/crates/quarto-core/src/transforms/callout_resolve.rs b/crates/quarto-core/src/transforms/callout_resolve.rs index 35a21de75..d11680673 100644 --- a/crates/quarto-core/src/transforms/callout_resolve.rs +++ b/crates/quarto-core/src/transforms/callout_resolve.rs @@ -22,17 +22,38 @@ //! //! ## Output Structure //! -//! The transform produces this Div structure (which renders to the expected HTML): +//! The transform produces Div structures matching what TS Quarto's +//! `src/resources/filters/modules/callouts.lua` (`render_to_bootstrap_div`) +//! emits, so the bundled Bootstrap SCSS in +//! `resources/scss/bootstrap/_bootstrap-rules.scss` applies cleanly. +//! +//! **Titled callout** (user title OR appearance=default + injected display name): //! //! ```text -//! Div.callout.callout-{type} -//! Div.callout-header -//! Div.callout-icon-container +//! Div.callout.callout-style-{appearance}.callout-{type}.callout-titled[.no-icon][.callout-empty-content] +//! Div.callout-header.d-flex.align-content-center[.collapsed] +//! Div.callout-icon-container (when icon=true) //! Plain[RawInline(html, "")] //! Div.callout-title-container.flex-fill //! Plain[title inlines...] -//! Div.callout-body-container.callout-body +//! Plain[
...] (when collapse=true|false) +//! Div.callout-body-container.callout-body (when no collapse) //! [content blocks...] +//! OR +//! Div.callout-collapse.collapse[.show] (when collapse=true|false) +//! Div.callout-body-container.callout-body +//! [content blocks...] +//! ``` +//! +//! **Untitled callout** (appearance!=default + no user title): +//! +//! ```text +//! Div.callout.callout-style-{appearance}.callout-{type}[.no-icon][.callout-empty-content] +//! Div.callout-body.d-flex +//! Div.callout-icon-container (when icon=true) +//! Plain[RawInline(html, "")] +//! Div.callout-body-container +//! [content blocks...] //! ``` use hashlink::LinkedHashMap; @@ -74,64 +95,68 @@ impl AstTransform for CalloutResolveTransform { } async fn transform(&self, ast: &mut Pandoc, _ctx: &mut RenderContext) -> Result<()> { - resolve_blocks(&mut ast.blocks); + // Document-scoped counter for generating unique `callout-N-contents` + // ids on collapsible callouts. Starts at 1 to match TS Quarto's + // `calloutidx` (`src/resources/filters/modules/callouts.lua`). + let mut counter = 1u32; + resolve_blocks(&mut ast.blocks, &mut counter); Ok(()) } } /// Resolve CustomNodes in a vector of blocks. -fn resolve_blocks(blocks: &mut Vec) { +fn resolve_blocks(blocks: &mut Vec, counter: &mut u32) { for block in blocks.iter_mut() { - resolve_block(block); + resolve_block(block, counter); } } /// Resolve a single block, potentially converting CustomNode to Div. -fn resolve_block(block: &mut Block) { +fn resolve_block(block: &mut Block, counter: &mut u32) { // First, recursively resolve any nested blocks match block { Block::BlockQuote(bq) => { - resolve_blocks(&mut bq.content); + resolve_blocks(&mut bq.content, counter); } Block::OrderedList(ol) => { for item in &mut ol.content { - resolve_blocks(item); + resolve_blocks(item, counter); } } Block::BulletList(bl) => { for item in &mut bl.content { - resolve_blocks(item); + resolve_blocks(item, counter); } } Block::DefinitionList(dl) => { for (_term, defs) in &mut dl.content { for def in defs { - resolve_blocks(def); + resolve_blocks(def, counter); } } } Block::Figure(fig) => { - resolve_blocks(&mut fig.content); + resolve_blocks(&mut fig.content, counter); } Block::Div(div) => { - resolve_blocks(&mut div.content); + resolve_blocks(&mut div.content, counter); } Block::Table(table) => { for body in &mut table.bodies { for row in &mut body.body { for cell in &mut row.cells { - resolve_blocks(&mut cell.content); + resolve_blocks(&mut cell.content, counter); } } } for row in &mut table.head.rows { for cell in &mut row.cells { - resolve_blocks(&mut cell.content); + resolve_blocks(&mut cell.content, counter); } } for row in &mut table.foot.rows { for cell in &mut row.cells { - resolve_blocks(&mut cell.content); + resolve_blocks(&mut cell.content, counter); } } } @@ -139,15 +164,15 @@ fn resolve_block(block: &mut Block) { // First resolve any nested blocks in slots for (_name, slot) in &mut custom.slots { match slot { - Slot::Block(b) => resolve_block(b), - Slot::Blocks(bs) => resolve_blocks(bs), + Slot::Block(b) => resolve_block(b, counter), + Slot::Blocks(bs) => resolve_blocks(bs, counter), _ => {} } } // Then check if this is a Callout that should be resolved if custom.type_name == "Callout" { - let resolved_div = resolve_callout(custom); + let resolved_div = resolve_callout(custom, counter); *block = Block::Div(resolved_div); } } @@ -156,61 +181,144 @@ fn resolve_block(block: &mut Block) { } } -/// Resolve a Callout CustomNode to a Div with the expected HTML structure. -fn resolve_callout(custom: &mut CustomNode) -> Div { - // Extract callout properties from plain_data - let callout_type = extract_string(&custom.plain_data, "type").unwrap_or("note"); - let appearance = extract_string(&custom.plain_data, "appearance").unwrap_or("default"); +/// Resolve a Callout CustomNode to a Div matching the TS Quarto / +/// Bootstrap HTML structure (see module doc-comment). +/// +/// `counter` is bumped by 1 whenever a collapsible callout consumes +/// it for the `callout-N-contents` id. +fn resolve_callout(custom: &mut CustomNode, counter: &mut u32) -> Div { + // Extract callout properties from plain_data. Appearance is + // already normalized at CalloutTransform time (minimal → simple + + // icon=false), so the resolver only sees `default` or `simple`. + // Owned copies up-front so the `&custom.plain_data` borrow is + // released before `extract_content_blocks` takes `&mut custom` + // below (NLL would handle this for a single short-lived borrow, + // but `callout_type` / `appearance` are used after the mutating + // call, so we clone now). + let callout_type = extract_string(&custom.plain_data, "type") + .unwrap_or("note") + .to_string(); + let raw_appearance = extract_string(&custom.plain_data, "appearance") + .unwrap_or("default") + .to_string(); let collapse = extract_bool(&custom.plain_data, "collapse").unwrap_or(false); - let icon = extract_bool(&custom.plain_data, "icon").unwrap_or(true); + let collapse_starts_collapsed = + extract_bool(&custom.plain_data, "collapse_starts_collapsed").unwrap_or(false); + let raw_icon = extract_bool(&custom.plain_data, "icon").unwrap_or(true); + + // Defense-in-depth normalization (`appearance="minimal"` → + // `simple` + `icon=false`). CalloutTransform already does this + // upstream, but the resolver also accepts CustomNodes synthesized + // by other code (tests, eventual filter authors), so we + // re-apply it here. Matches TS Quarto's `nameForCalloutStyle`. + let (appearance, icon) = if raw_appearance == "minimal" { + ("simple".to_string(), false) + } else { + (raw_appearance, raw_icon) + }; let source_info = custom.source_info.clone(); - // Build class list for outer div - let mut classes = vec!["callout".to_string(), format!("callout-{}", callout_type)]; - if appearance != "default" { - classes.push(format!("callout-appearance-{}", appearance)); + // Pull title and content out of the custom node. + let user_title = extract_user_title(custom); + let content_blocks = extract_content_blocks(custom); + let has_content = !content_blocks.is_empty(); + + // Default-title injection (per `callouts.lua:224-227`): + // `appearance="default"` + empty title → inject the type's display name. + // `appearance="simple"` keeps the empty title, taking the untitled path. + let title_inlines: Option> = match user_title { + Some(t) if !t.is_empty() => Some(t), + _ if appearance == "default" => Some(vec![Inline::Str(Str { + text: capitalize(&callout_type), + source_info: SourceInfo::default(), + })]), + _ => None, + }; + let is_titled = title_inlines.is_some(); + + // Outer div classes. + let mut classes = vec![ + "callout".to_string(), + format!("callout-style-{}", appearance), + format!("callout-{}", callout_type), + ]; + if !icon { + classes.push("no-icon".to_string()); } - if collapse { - classes.push("callout-collapse".to_string()); + if is_titled { + classes.push("callout-titled".to_string()); + } + if !has_content { + classes.push("callout-empty-content".to_string()); } - // Include original non-callout classes from attr + // Preserve original non-callout classes from the source attr. let (orig_id, orig_classes, orig_attrs) = &custom.attr; for cls in orig_classes { if !cls.starts_with("callout") { classes.push(cls.clone()); } } - - // Build outer div attr let outer_attr: Attr = (orig_id.clone(), classes, orig_attrs.clone()); - // Extract title and content from slots - let title_inlines = extract_title_inlines(custom, callout_type); - let content_blocks = extract_content_blocks(custom); + // Reserve a unique id for the collapse wrapper up-front so the + // header's `bs-target` / `aria-controls` match what we apply to + // the wrapper. Only consumed if collapse is enabled. + let collapse_id = if collapse { + let id = format!("callout-{}-contents", *counter); + *counter += 1; + Some(id) + } else { + None + }; + + let body_inner_div = Div { + attr: make_attr(&["callout-body-container"]), + content: content_blocks, + source_info: source_info.clone(), + attr_source: AttrSourceInfo::empty(), + }; + + let outer_content = if is_titled { + build_titled_content( + &source_info, + title_inlines.expect("is_titled => title_inlines is Some"), + icon, + collapse, + collapse_starts_collapsed, + collapse_id.as_deref(), + body_inner_div, + ) + } else { + build_untitled_content(&source_info, icon, body_inner_div) + }; - // Build the inner structure + Div { + attr: outer_attr, + content: outer_content, + source_info, + attr_source: AttrSourceInfo::empty(), + } +} + +/// Construct the children of the outer Div for the titled-callout path. +fn build_titled_content( + source_info: &SourceInfo, + title_inlines: Vec, + icon: bool, + collapse: bool, + collapse_starts_collapsed: bool, + collapse_id: Option<&str>, + body_inner_div: Div, +) -> Vec { let mut header_content = Vec::new(); - // Icon container (if enabled) if icon { - header_content.push(Block::Div(Div { - attr: make_attr(&["callout-icon-container"]), - content: vec![Block::Plain(Plain { - content: vec![Inline::RawInline(RawInline { - format: "html".to_string(), - text: "".to_string(), - source_info: source_info.clone(), - })], - source_info: source_info.clone(), - })], - source_info: source_info.clone(), - attr_source: AttrSourceInfo::empty(), - })); + header_content.push(Block::Div(icon_container_div(source_info))); } - // Title container + // Title container. header_content.push(Block::Div(Div { attr: make_attr(&["callout-title-container", "flex-fill"]), content: vec![Block::Plain(Plain { @@ -221,51 +329,134 @@ fn resolve_callout(custom: &mut CustomNode) -> Div { attr_source: AttrSourceInfo::empty(), })); - // Header div + // Header: classes + (when collapsible) Bootstrap toggle attrs. + let mut header_classes: Vec = vec![ + "callout-header".to_string(), + "d-flex".to_string(), + "align-content-center".to_string(), + ]; + let mut header_attrs = LinkedHashMap::new(); + if collapse { + let collapse_id = collapse_id.expect("collapse=true => collapse_id is Some"); + if collapse_starts_collapsed { + header_classes.push("collapsed".to_string()); + } + header_attrs.insert("bs-toggle".to_string(), "collapse".to_string()); + header_attrs.insert("bs-target".to_string(), format!(".{}", collapse_id)); + header_attrs.insert("aria-controls".to_string(), collapse_id.to_string()); + header_attrs.insert( + "aria-expanded".to_string(), + if collapse_starts_collapsed { + "false" + } else { + "true" + } + .to_string(), + ); + header_attrs.insert("aria-label".to_string(), "Toggle callout".to_string()); + + // Trailing toggle button. + header_content.push(Block::Plain(Plain { + content: vec![Inline::RawInline(RawInline { + format: "html".to_string(), + text: "
" + .to_string(), + source_info: source_info.clone(), + })], + source_info: source_info.clone(), + })); + } let header_div = Block::Div(Div { - attr: make_attr(&["callout-header"]), + attr: (String::new(), header_classes, header_attrs), content: header_content, source_info: source_info.clone(), attr_source: AttrSourceInfo::empty(), }); - // Body div - let body_div = Block::Div(Div { - attr: make_attr(&["callout-body-container", "callout-body"]), - content: content_blocks, + // Body. With collapse, wrap the body-container in a + // `.callout-collapse.collapse[.show]` div; without collapse, the + // body-container itself takes the `callout-body` class. + let body_block = if collapse { + let collapse_id = collapse_id.expect("collapse=true => collapse_id is Some"); + let mut collapse_classes = vec![ + collapse_id.to_string(), + "callout-collapse".to_string(), + "collapse".to_string(), + ]; + if !collapse_starts_collapsed { + collapse_classes.push("show".to_string()); + } + // Body-container inside the collapse wrapper carries + // `callout-body-container callout-body` (matches TS Quarto's + // titled+collapse output). + let mut body_with_class = body_inner_div; + body_with_class.attr.1.push("callout-body".to_string()); + Block::Div(Div { + attr: ( + collapse_id.to_string(), + collapse_classes, + LinkedHashMap::new(), + ), + content: vec![Block::Div(body_with_class)], + source_info: source_info.clone(), + attr_source: AttrSourceInfo::empty(), + }) + } else { + let mut body_with_class = body_inner_div; + body_with_class.attr.1.push("callout-body".to_string()); + Block::Div(body_with_class) + }; + + vec![header_div, body_block] +} + +/// Construct the children of the outer Div for the untitled-callout path. +/// +/// Untitled callouts are flat: a single `
` +/// wrapping the icon container and the body-container. +fn build_untitled_content(source_info: &SourceInfo, icon: bool, body_inner_div: Div) -> Vec { + let mut body_content = Vec::new(); + if icon { + body_content.push(Block::Div(icon_container_div(source_info))); + } + body_content.push(Block::Div(body_inner_div)); + + let body_outer = Div { + attr: make_attr(&["callout-body", "d-flex"]), + content: body_content, source_info: source_info.clone(), attr_source: AttrSourceInfo::empty(), - }); + }; + vec![Block::Div(body_outer)] +} - // Outer callout div +fn icon_container_div(source_info: &SourceInfo) -> Div { Div { - attr: outer_attr, - content: vec![header_div, body_div], - source_info, + attr: make_attr(&["callout-icon-container"]), + content: vec![Block::Plain(Plain { + content: vec![Inline::RawInline(RawInline { + format: "html".to_string(), + text: "".to_string(), + source_info: source_info.clone(), + })], + source_info: source_info.clone(), + })], + source_info: source_info.clone(), attr_source: AttrSourceInfo::empty(), } } -/// Extract title inlines from the CustomNode, with fallback to default. -fn extract_title_inlines(custom: &CustomNode, callout_type: &str) -> Vec { - if let Some(title_slot) = custom.get_slot("title") { - match title_slot { - Slot::Inlines(inlines) if !inlines.is_empty() => { - return inlines.clone(); - } - Slot::Inline(inline) => { - return vec![inline.as_ref().clone()]; - } - _ => {} - } +/// Pull the user-supplied title inlines out of the CustomNode, if any. +/// Returns None when the title slot is absent or empty — that's the +/// signal for the resolver to decide between default-title injection +/// (appearance=default) and the untitled path (appearance=simple). +fn extract_user_title(custom: &CustomNode) -> Option> { + match custom.get_slot("title")? { + Slot::Inlines(inlines) if !inlines.is_empty() => Some(inlines.clone()), + Slot::Inline(inline) => Some(vec![inline.as_ref().clone()]), + _ => None, } - - // Default title based on callout type - let default_title = capitalize(callout_type); - vec![Inline::Str(Str { - text: default_title, - source_info: SourceInfo::default(), - })] } /// Extract content blocks from the CustomNode. @@ -470,7 +661,7 @@ mod tests { custom.plain_data = json!({"type": "note"}); // No title slot - should use default - let resolved = resolve_callout(&mut custom); + let resolved = resolve_callout(&mut custom, &mut 1); // Find the title container and check it has "Note" let header = &resolved.content[0]; @@ -497,7 +688,7 @@ mod tests { let mut custom = CustomNode::new("Callout", empty_attr(), dummy_source_info()); custom.plain_data = json!({"type": "warning", "icon": false}); - let resolved = resolve_callout(&mut custom); + let resolved = resolve_callout(&mut custom, &mut 1); // Header should only have title container, no icon if let Block::Div(header_div) = &resolved.content[0] { @@ -553,4 +744,429 @@ mod tests { assert_eq!(capitalize(""), ""); assert_eq!(capitalize("TIP"), "TIP"); } + + // ==================================================================== + // Canonical-class tests (TS Quarto / Bootstrap scheme). + // + // These tests assert the class vocabulary that the bundled Bootstrap + // SCSS (`resources/scss/bootstrap/_bootstrap-rules.scss`) keys off + // of, matching what `src/resources/filters/modules/callouts.lua` in + // TS Quarto emits (`render_to_bootstrap_div`). They are EXPECTED TO + // FAIL against the pre-Phase-2 resolver — they encode the target + // behaviour for the rewrite. See + // `claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md`. + // ==================================================================== + + fn str_inline(text: &str) -> Inline { + Inline::Str(Str { + text: text.to_string(), + source_info: dummy_source_info(), + }) + } + + fn para(text: &str) -> Block { + Block::Paragraph(Paragraph { + content: vec![str_inline(text)], + source_info: dummy_source_info(), + }) + } + + /// Construct a Callout CustomNode. Passing `None` for `title` leaves + /// the title slot unset (= user supplied no `## Title` header). + /// + /// `collapse=Some(true)` models the user writing `collapse="true"` + /// (starts collapsed); `Some(false)` models `collapse="false"` + /// (collapsible but starts expanded); `None` means no `collapse` + /// attribute at all (not collapsible). This mirrors what + /// `CalloutTransform` writes into `plain_data` for a real qmd + /// callout. + fn callout_node( + callout_type: &str, + appearance: Option<&str>, + icon: Option, + collapse: Option, + title: Option>, + content: Vec, + ) -> CustomNode { + let mut data = serde_json::Map::new(); + data.insert("type".into(), json!(callout_type)); + if let Some(a) = appearance { + data.insert("appearance".into(), json!(a)); + } + if let Some(i) = icon { + data.insert("icon".into(), json!(i)); + } + if let Some(starts_collapsed) = collapse { + data.insert("collapse".into(), json!(true)); + data.insert("collapse_starts_collapsed".into(), json!(starts_collapsed)); + } + let mut custom = CustomNode::new("Callout", empty_attr(), dummy_source_info()); + custom.plain_data = Value::Object(data); + if let Some(t) = title { + custom.set_slot("title", Slot::Inlines(t)); + } + custom.set_slot("content", Slot::Blocks(content)); + custom + } + + fn outer_classes(div: &Div) -> Vec { + div.attr.1.clone() + } + + fn assert_has_class(classes: &[String], expected: &str) { + assert!( + classes.iter().any(|c| c == expected), + "expected class `{}` to be present; got {:?}", + expected, + classes + ); + } + + fn assert_no_class(classes: &[String], unexpected: &str) { + assert!( + classes.iter().all(|c| c != unexpected), + "expected class `{}` to be absent; got {:?}", + unexpected, + classes + ); + } + + /// Walk the resolved Div tree and return true if any descendant Div + /// (including the root) carries `class`. + fn contains_class_anywhere(div: &Div, class: &str) -> bool { + if div.attr.1.iter().any(|c| c == class) { + return true; + } + for block in &div.content { + if let Block::Div(d) = block + && contains_class_anywhere(d, class) + { + return true; + } + } + false + } + + /// Pull the user-visible title text out of a titled-path resolved + /// Div. Returns None for untitled callouts. + fn extract_title_text(resolved: &Div) -> Option { + let header = match resolved.content.first()? { + Block::Div(d) if d.attr.1.iter().any(|c| c == "callout-header") => d, + _ => return None, + }; + for block in &header.content { + if let Block::Div(d) = block + && d.attr.1.iter().any(|c| c == "callout-title-container") + && let Some(Block::Plain(p)) = d.content.first() + { + let mut text = String::new(); + for inline in &p.content { + if let Inline::Str(s) = inline { + text.push_str(&s.text); + } + } + return Some(text); + } + } + None + } + + #[tokio::test] + async fn test_canonical_default_with_user_title() { + let mut node = callout_node( + "warning", + Some("default"), + None, + None, + Some(vec![str_inline("Watch Out")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let classes = outer_classes(&resolved); + assert_has_class(&classes, "callout"); + assert_has_class(&classes, "callout-style-default"); + assert_has_class(&classes, "callout-warning"); + assert_has_class(&classes, "callout-titled"); + assert_no_class(&classes, "no-icon"); + assert_no_class(&classes, "callout-empty-content"); + assert_no_class(&classes, "callout-appearance-default"); + assert_no_class(&classes, "callout-appearance-simple"); + } + + #[tokio::test] + async fn test_canonical_default_no_title_injects_default() { + let mut node = callout_node("tip", Some("default"), None, None, None, vec![para("Body")]); + let resolved = resolve_callout(&mut node, &mut 1); + let classes = outer_classes(&resolved); + assert_has_class(&classes, "callout-style-default"); + assert_has_class(&classes, "callout-titled"); + assert_eq!( + extract_title_text(&resolved).as_deref(), + Some("Tip"), + "appearance=default with no user title should inject the type's display name" + ); + } + + #[tokio::test] + async fn test_canonical_simple_no_title_stays_untitled() { + let mut node = callout_node("note", Some("simple"), None, None, None, vec![para("Body")]); + let resolved = resolve_callout(&mut node, &mut 1); + let classes = outer_classes(&resolved); + assert_has_class(&classes, "callout-style-simple"); + assert_no_class(&classes, "callout-titled"); + + // Untitled path: outer has a single child, the body Div with + // classes `callout-body d-flex` containing the icon container + // and then the body-container. + assert_eq!( + resolved.content.len(), + 1, + "untitled callout should have a single body child, no header div" + ); + let body = match &resolved.content[0] { + Block::Div(d) => d, + other => panic!("expected body Div, got {:?}", other), + }; + let body_classes = outer_classes(body); + assert_has_class(&body_classes, "callout-body"); + assert_has_class(&body_classes, "d-flex"); + assert_no_class(&body_classes, "callout-header"); + + // First child = icon container. + let icon = match &body.content[0] { + Block::Div(d) => d, + other => panic!("expected icon container Div, got {:?}", other), + }; + assert_has_class(&outer_classes(icon), "callout-icon-container"); + + // Second child = body-container (NOT also `callout-body`). + let container = match &body.content[1] { + Block::Div(d) => d, + other => panic!("expected body-container Div, got {:?}", other), + }; + let container_classes = outer_classes(container); + assert_has_class(&container_classes, "callout-body-container"); + assert_no_class(&container_classes, "callout-body"); + } + + #[tokio::test] + async fn test_canonical_simple_with_user_title() { + let mut node = callout_node( + "important", + Some("simple"), + None, + None, + Some(vec![str_inline("Please Read")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let classes = outer_classes(&resolved); + assert_has_class(&classes, "callout-style-simple"); + assert_has_class(&classes, "callout-titled"); + let header = match &resolved.content[0] { + Block::Div(d) => d, + other => panic!("expected header Div, got {:?}", other), + }; + assert_has_class(&outer_classes(header), "callout-header"); + } + + #[tokio::test] + async fn test_canonical_minimal_normalizes_to_simple_no_icon() { + let mut node = callout_node( + "caution", + Some("minimal"), + None, + None, + None, + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let classes = outer_classes(&resolved); + assert_has_class(&classes, "callout-style-simple"); + assert_no_class(&classes, "callout-style-minimal"); + assert_has_class(&classes, "no-icon"); + assert!( + !contains_class_anywhere(&resolved, "callout-icon-container"), + "minimal appearance must omit the icon container" + ); + } + + #[tokio::test] + async fn test_canonical_icon_false_emits_no_icon() { + let mut node = callout_node( + "warning", + Some("default"), + Some(false), + None, + Some(vec![str_inline("Heads Up")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + assert_has_class(&outer_classes(&resolved), "no-icon"); + assert!( + !contains_class_anywhere(&resolved, "callout-icon-container"), + "icon=false must omit the icon container" + ); + } + + #[tokio::test] + async fn test_canonical_empty_content_class() { + let mut node = callout_node( + "note", + Some("default"), + None, + None, + Some(vec![str_inline("Heads Up")]), + vec![], + ); + let resolved = resolve_callout(&mut node, &mut 1); + assert_has_class(&outer_classes(&resolved), "callout-empty-content"); + } + + #[tokio::test] + async fn test_canonical_titled_header_has_utility_classes() { + let mut node = callout_node( + "tip", + Some("default"), + None, + None, + Some(vec![str_inline("Title")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let header = match &resolved.content[0] { + Block::Div(d) => d, + other => panic!("expected header Div, got {:?}", other), + }; + let header_classes = outer_classes(header); + assert_has_class(&header_classes, "callout-header"); + assert_has_class(&header_classes, "d-flex"); + assert_has_class(&header_classes, "align-content-center"); + } + + #[tokio::test] + async fn test_canonical_collapse_true_emits_wrapper() { + let mut node = callout_node( + "note", + Some("default"), + None, + Some(true), + Some(vec![str_inline("T")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + // Expected structure: outer > [header, collapse-wrapper]; + // collapse-wrapper > body. + assert_eq!( + resolved.content.len(), + 2, + "outer should have header + collapse wrapper as siblings" + ); + let header = match &resolved.content[0] { + Block::Div(d) => d, + other => panic!("expected header Div, got {:?}", other), + }; + let header_classes = outer_classes(header); + assert_has_class(&header_classes, "callout-header"); + assert_has_class(&header_classes, "collapsed"); + let header_attrs = &header.attr.2; + assert_eq!( + header_attrs.get("aria-expanded").map(String::as_str), + Some("false"), + "collapsed callout header must have aria-expanded=\"false\"" + ); + let collapse = match &resolved.content[1] { + Block::Div(d) => d, + other => panic!("expected collapse wrapper Div, got {:?}", other), + }; + let collapse_classes = outer_classes(collapse); + assert_has_class(&collapse_classes, "callout-collapse"); + assert_has_class(&collapse_classes, "collapse"); + assert_no_class(&collapse_classes, "show"); + } + + #[tokio::test] + async fn test_canonical_collapse_false_emits_show_class() { + let mut node = callout_node( + "note", + Some("default"), + None, + Some(false), + Some(vec![str_inline("T")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let header = match &resolved.content[0] { + Block::Div(d) => d, + other => panic!("expected header Div, got {:?}", other), + }; + let header_attrs = &header.attr.2; + assert_eq!( + header_attrs.get("aria-expanded").map(String::as_str), + Some("true"), + "open collapse callout must have aria-expanded=\"true\"" + ); + assert_no_class(&outer_classes(header), "collapsed"); + let collapse = match &resolved.content[1] { + Block::Div(d) => d, + other => panic!("expected collapse wrapper Div, got {:?}", other), + }; + let collapse_classes = outer_classes(collapse); + assert_has_class(&collapse_classes, "callout-collapse"); + assert_has_class(&collapse_classes, "collapse"); + assert_has_class(&collapse_classes, "show"); + } + + #[tokio::test] + async fn test_canonical_no_legacy_appearance_class() { + for appearance in &["default", "simple"] { + let mut node = callout_node( + "note", + Some(appearance), + None, + None, + Some(vec![str_inline("T")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let classes = outer_classes(&resolved); + assert_no_class(&classes, "callout-appearance-default"); + assert_no_class(&classes, "callout-appearance-simple"); + assert_no_class(&classes, "callout-appearance-minimal"); + } + } + + #[tokio::test] + async fn test_canonical_user_id_preserved() { + let mut node = callout_node( + "tip", + Some("default"), + None, + None, + Some(vec![str_inline("T")]), + vec![para("Body")], + ); + node.attr.0 = "mywarn".to_string(); + let resolved = resolve_callout(&mut node, &mut 1); + assert_eq!( + resolved.attr.0, "mywarn", + "user-supplied id must survive resolution" + ); + } + + #[tokio::test] + async fn test_canonical_all_types_emit_type_class() { + for t in &["note", "warning", "tip", "important", "caution"] { + let mut node = callout_node( + t, + Some("default"), + None, + None, + Some(vec![str_inline("T")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + assert_has_class(&outer_classes(&resolved), &format!("callout-{}", t)); + } + } } diff --git a/crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd b/crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd new file mode 100644 index 000000000..be7068708 --- /dev/null +++ b/crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd @@ -0,0 +1,115 @@ +--- +title: "Callouts canonical-class matrix" +format: html +_quarto: + tests: + html: + ensureFileRegexMatches: + # Patterns the rendered HTML MUST contain. Schema is two-array: + # [0] = must-match, [1] = must-NOT-match (see + # `crates/quarto-test/src/spec.rs::parse_ensure_file_regex_matches`). + - - "callout-note" + - "callout-warning" + - "callout-tip" + - "callout-important" + - "callout-caution" + - "callout-style-default" + - "callout-style-simple" + - "callout-titled" + - "no-icon" + - "callout-empty-content" + - "callout-collapse" + - 'bs-toggle="collapse"' + - "callout-body-container" + - "callout-body" + - "callout-header" + - "d-flex" + - "align-content-center" + # Legacy q2-only vocabulary must NOT appear. + - - "callout-appearance-default" + - "callout-appearance-simple" + - "callout-appearance-minimal" +--- + +# All five types — default appearance, titled + +::: {.callout-note} +## A note title +A note callout body. +::: + +::: {.callout-warning} +## A warning title +A warning callout body. +::: + +::: {.callout-tip} +## A tip title +A tip callout body. +::: + +::: {.callout-important} +## An important title +An important callout body. +::: + +::: {.callout-caution} +## A caution title +A caution callout body. +::: + +# Default appearance — no user title → default-title injection + +::: {.callout-note} +Untitled note body (should inject "Note" as title via default-injection). +::: + +# Simple appearance + +::: {.callout-tip appearance="simple"} +## Simple titled tip +With user title, simple-style border. +::: + +::: {.callout-tip appearance="simple"} +Untitled simple tip — should take the untitled path (no header, no callout-titled). +::: + +# Minimal appearance → normalized to simple + no-icon + +::: {.callout-warning appearance="minimal"} +## Minimal warning +Minimal normalizes to simple appearance + no-icon. +::: + +# Explicit icon=false + +::: {.callout-important icon="false"} +## No icon +icon=false adds the no-icon class and omits the icon container. +::: + +# Empty content + +::: {.callout-caution} +## Empty-content caution +::: + +# Collapse — starts collapsed, then starts expanded + +::: {.callout-note collapse="true"} +## Click to expand +This body should start collapsed. +::: + +::: {.callout-note collapse="false"} +## Always open (collapsible) +This body is collapsible but starts expanded. +::: + +# User id is preserved + +::: {#my-warn .callout-warning} +## Referenceable +Has the user-supplied id on the outer div. +::: diff --git a/ts-packages/preview-renderer/src/q2-preview/custom-components.integration.test.tsx b/ts-packages/preview-renderer/src/q2-preview/custom-components.integration.test.tsx index 6049a9e1a..ac1f6e115 100644 --- a/ts-packages/preview-renderer/src/q2-preview/custom-components.integration.test.tsx +++ b/ts-packages/preview-renderer/src/q2-preview/custom-components.integration.test.tsx @@ -23,14 +23,22 @@ import { Ast } from '../framework'; import type { FormatRegistry, NodeArgs, PandocAST } from '../framework'; import { previewRegistry } from './registry'; import { + BS_ALIGN_CONTENT_CENTER, + BS_COLLAPSE, + BS_D_FLEX, + BS_SHOW, CALLOUT, CALLOUT_BODY, CALLOUT_BODY_CONTAINER, + CALLOUT_COLLAPSE, + CALLOUT_EMPTY_CONTENT, CALLOUT_FLEX_FILL, CALLOUT_HEADER, CALLOUT_ICON, CALLOUT_ICON_CONTAINER, CALLOUT_TITLE_CONTAINER, + CALLOUT_TITLED, + NO_ICON, PROOF, QUARTO_XREF, THEOREM, @@ -70,6 +78,7 @@ function calloutAst(opts: { type?: string; appearance?: string; collapse?: boolean; + collapseStartsCollapsed?: boolean; icon?: boolean; title?: any[] | undefined; // undefined = no title slot; [] = empty Inlines content?: any[]; @@ -88,6 +97,7 @@ function calloutAst(opts: { type: opts.type ?? 'note', appearance: opts.appearance ?? 'default', collapse: opts.collapse ?? false, + collapse_starts_collapsed: opts.collapseStartsCollapsed ?? false, icon: opts.icon ?? true, }, attr: [opts.id ?? '', [], []], @@ -269,19 +279,111 @@ describe('Callout', () => { expect(titleContainer!.textContent).not.toContain('Note'); }); - it('emits callout-appearance-{a} class for non-default appearance', () => { - const { container } = mount([calloutAst({ appearance: 'simple' })]); + it('always emits callout-style-{appearance} on the outer div (default)', () => { + const { container } = mount([calloutAst({ appearance: 'default' })]); const callout = container.querySelector('div.callout'); - expect(callout!.classList.contains('callout-appearance-simple')).toBe(true); + expect(callout!.classList.contains('callout-style-default')).toBe(true); }); - it('omits callout-appearance class for the default appearance', () => { - const { container } = mount([calloutAst({ appearance: 'default' })]); + it('emits callout-style-simple for simple appearance', () => { + const { container } = mount([calloutAst({ appearance: 'simple', title: [STR('T')] })]); const callout = container.querySelector('div.callout'); - const hasAppearance = Array.from(callout!.classList).some((c) => - c.startsWith('callout-appearance-'), - ); - expect(hasAppearance).toBe(false); + expect(callout!.classList.contains('callout-style-simple')).toBe(true); + }); + + it('normalizes minimal → simple + no-icon', () => { + const { container } = mount([calloutAst({ appearance: 'minimal', title: [STR('T')] })]); + const callout = container.querySelector('div.callout'); + expect(callout!.classList.contains('callout-style-simple')).toBe(true); + expect(callout!.classList.contains(NO_ICON)).toBe(true); + expect(container.querySelector(`.${CALLOUT_ICON_CONTAINER}`)).toBeNull(); + }); + + it('emits no legacy callout-appearance-* classes', () => { + for (const appearance of ['default', 'simple', 'minimal']) { + const { container } = mount([calloutAst({ appearance, title: [STR('T')] })]); + const callout = container.querySelector('div.callout'); + const hasLegacy = Array.from(callout!.classList).some((c) => + c.startsWith('callout-appearance-'), + ); + expect(hasLegacy, `appearance=${appearance}`).toBe(false); + } + }); + + it('adds callout-titled when callout has a user title', () => { + const { container } = mount([calloutAst({ title: [STR('My Title')] })]); + const callout = container.querySelector('div.callout'); + expect(callout!.classList.contains(CALLOUT_TITLED)).toBe(true); + }); + + it('adds callout-titled when appearance=default + no user title (default-injected)', () => { + const { container } = mount([calloutAst({ appearance: 'default', title: undefined })]); + const callout = container.querySelector('div.callout'); + expect(callout!.classList.contains(CALLOUT_TITLED)).toBe(true); + }); + + it('untitled-path: appearance=simple + no user title → no callout-titled, body is callout-body.d-flex', () => { + const { container } = mount([calloutAst({ appearance: 'simple', title: undefined })]); + const callout = container.querySelector('div.callout'); + expect(callout!.classList.contains(CALLOUT_TITLED)).toBe(false); + // Outer's single child is `.callout-body.d-flex` (no header div). + expect(callout!.querySelector(`.${CALLOUT_HEADER}`)).toBeNull(); + const body = callout!.querySelector(`.${CALLOUT_BODY}.${BS_D_FLEX}`); + expect(body).not.toBeNull(); + // Inside the body: icon container, then a `.callout-body-container` + // WITHOUT the `.callout-body` co-class. + const innerContainer = body!.querySelector(`.${CALLOUT_BODY_CONTAINER}`); + expect(innerContainer).not.toBeNull(); + expect(innerContainer!.classList.contains(CALLOUT_BODY)).toBe(false); + }); + + it('adds no-icon class when icon=false', () => { + const { container } = mount([calloutAst({ icon: false, title: [STR('T')] })]); + const callout = container.querySelector('div.callout'); + expect(callout!.classList.contains(NO_ICON)).toBe(true); + expect(container.querySelector(`.${CALLOUT_ICON_CONTAINER}`)).toBeNull(); + }); + + it('adds callout-empty-content when content slot is empty', () => { + const { container } = mount([calloutAst({ title: [STR('T')], content: [] })]); + const callout = container.querySelector('div.callout'); + expect(callout!.classList.contains(CALLOUT_EMPTY_CONTENT)).toBe(true); + }); + + it('header carries d-flex + align-content-center utility classes', () => { + const { container } = mount([calloutAst({ title: [STR('T')] })]); + const header = container.querySelector(`.${CALLOUT_HEADER}`); + expect(header).not.toBeNull(); + expect(header!.classList.contains(BS_D_FLEX)).toBe(true); + expect(header!.classList.contains(BS_ALIGN_CONTENT_CENTER)).toBe(true); + }); + + it('collapse=true wraps body in .callout-collapse.collapse (no .show when starts collapsed)', () => { + const { container } = mount([ + calloutAst({ + title: [STR('T')], + collapse: true, + collapseStartsCollapsed: true, + }), + ]); + const wrapper = container.querySelector(`.${CALLOUT_COLLAPSE}`); + expect(wrapper).not.toBeNull(); + expect(wrapper!.classList.contains(BS_COLLAPSE)).toBe(true); + expect(wrapper!.classList.contains(BS_SHOW)).toBe(false); + // Body container nested inside the wrapper. + expect(wrapper!.querySelector(`.${CALLOUT_BODY_CONTAINER}`)).not.toBeNull(); + }); + + it('collapse=true + starts-expanded keeps body open via .show', () => { + const { container } = mount([ + calloutAst({ + title: [STR('T')], + collapse: true, + collapseStartsCollapsed: false, + }), + ]); + const wrapper = container.querySelector(`.${CALLOUT_COLLAPSE}`); + expect(wrapper!.classList.contains(BS_SHOW)).toBe(true); }); it('omits id attribute when attr id is empty', () => { diff --git a/ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx b/ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx index e0275746f..11da2793c 100644 --- a/ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx +++ b/ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx @@ -4,17 +4,24 @@ import type { Slot, } from '../../framework'; import { + BS_ALIGN_CONTENT_CENTER, + BS_COLLAPSE, + BS_D_FLEX, + BS_SHOW, CALLOUT, - CALLOUT_APPEARANCE_PREFIX, CALLOUT_BODY, CALLOUT_BODY_CONTAINER, CALLOUT_COLLAPSE, + CALLOUT_EMPTY_CONTENT, CALLOUT_FLEX_FILL, CALLOUT_HEADER, CALLOUT_ICON, CALLOUT_ICON_CONTAINER, + CALLOUT_STYLE_PREFIX, CALLOUT_TITLE_CONTAINER, + CALLOUT_TITLED, CALLOUT_TYPE_PREFIX, + NO_ICON, } from '../quartoClasses'; import { makeSlotSetter, renderSlot } from '../utils'; @@ -23,25 +30,46 @@ import { makeSlotSetter, renderSlot } from '../utils'; * structure (`crates/quarto-core/src/transforms/callout_resolve.rs`). * * `callout-resolve` is excluded from q2-preview's pipeline (see - * `pipeline.rs:1050`'s `Q2_PREVIEW_TRANSFORM_EXCLUDED`), so the Rust - * side hands us the `Callout` CustomNode wrapper unchanged. This - * component must emit the same three-deep DOM that `callout_resolve.rs` - * would have produced — Bootstrap's callout selectors target - * `.callout > .callout-header > .callout-title-container` etc., so - * flattening any level breaks theme CSS. + * `pipeline.rs::Q2_PREVIEW_TRANSFORM_EXCLUDED`), so the Rust side hands + * us the `Callout` CustomNode wrapper unchanged. This component must + * emit the same DOM that `callout_resolve.rs` would have produced — + * Bootstrap's callout selectors target `.callout.callout-style-default + * > .callout-header > .callout-title-container` etc. * - * `plain_data` (writer: `transforms/callout.rs:210`): + * Two output shapes, mirroring `callout_resolve::build_titled_content` + * and `build_untitled_content`: + * + * Titled (user title OR appearance="default" + injected default title): + * .callout.callout-style-{appearance}.callout-{type}.callout-titled + * .callout-header.d-flex.align-content-center + * .callout-icon-container? (when icon=true) + * .callout-title-container.flex-fill + * .callout-body-container.callout-body + * + * Untitled (appearance="simple"/"minimal" + empty title): + * .callout.callout-style-{appearance}.callout-{type} + * .callout-body.d-flex + * .callout-icon-container? (when icon=true) + * .callout-body-container + * + * `plain_data` (writer: `transforms/callout.rs`): * - `type` (string): `note | warning | tip | important | caution`. - * - `appearance` (string): `default | simple | minimal`. - * - `collapse` (bool). - * - `icon` (bool): controls whether the `.callout-icon-container` - * subtree is emitted. + * - `appearance` (string): `default | simple | minimal` — minimal + * is normalized to `simple` + `icon=false` here, mirroring the + * Rust resolver's defense-in-depth (`callout_resolve.rs`). + * - `collapse` (bool): true → collapsible callout. Renders the + * `.callout-collapse.collapse[.show]` wrapper. NO interactive + * toggle in preview — see bd-???? follow-up. + * - `collapse_starts_collapsed` (bool): when `collapse=true`, true + * means start collapsed. Cosmetic in preview (no toggle). + * - `icon` (bool): controls `.callout-icon-container` subtree. */ interface CalloutPlainData { type?: string; appearance?: string; collapse?: boolean; + collapse_starts_collapsed?: boolean; icon?: boolean; } @@ -56,61 +84,113 @@ const DEFAULT_TITLES: Record = { function defaultTitle(calloutType: string): string { if (DEFAULT_TITLES[calloutType]) return DEFAULT_TITLES[calloutType]; // Fallback for forward-compat callout types: ASCII-uppercase first - // byte (matches Rust capitalize at callout_resolve.rs:304). + // byte (mirrors Rust capitalize). if (!calloutType) return ''; return calloutType[0].toUpperCase() + calloutType.slice(1); } /** - * Mirror Rust's `inlines.is_empty()` check: only the literally-empty - * Inlines slot is treated as "no title". A whitespace-only or - * single-space title still wins over the default. `slots.title` - * absent (no key in the map) also falls back to the default. + * Mirror Rust's `extract_user_title` + emptiness check: only the + * literally-empty Inlines slot is treated as "no user title". + * A whitespace-only title still counts as titled. */ -function shouldUseDefaultTitle(titleSlot: Slot | undefined): boolean { - if (!titleSlot) return true; - if (titleSlot.kind === 'inlines' && titleSlot.value.length === 0) return true; - return false; +function hasUserTitle(titleSlot: Slot | undefined): boolean { + if (!titleSlot) return false; + if (titleSlot.kind === 'inlines' && titleSlot.value.length === 0) return false; + return true; } export const Callout = ({ node, onNavigateToDocument, setLocalAst }: NodeArgs) => { const plain = (node.plain_data ?? {}) as CalloutPlainData; const calloutType = plain.type ?? 'note'; - const appearance = plain.appearance ?? 'default'; + const rawAppearance = plain.appearance ?? 'default'; + const rawIcon = plain.icon !== false; // undefined defaults to icon-on + // Defense-in-depth normalization: minimal → simple + icon=false. + const appearance = rawAppearance === 'minimal' ? 'simple' : rawAppearance; + const icon = rawAppearance === 'minimal' ? false : rawIcon; const collapse = plain.collapse === true; - const showIcon = plain.icon !== false; // undefined defaults to icon-on + const startsCollapsed = collapse && plain.collapse_starts_collapsed === true; - const classList = [CALLOUT, `${CALLOUT_TYPE_PREFIX}${calloutType}`]; - if (appearance !== 'default') { - classList.push(`${CALLOUT_APPEARANCE_PREFIX}${appearance}`); - } - if (collapse) classList.push(CALLOUT_COLLAPSE); + const titleSlot = node.slots.title; + const userTitled = hasUserTitle(titleSlot); + // Default-title injection: `appearance="default"` + no user title → + // inject display name (then the titled path is taken). For non-default + // appearances, an empty title means untitled. + const isTitled = userTitled || appearance === 'default'; + + const contentSlot = node.slots.content; + const isEmptyContent = !contentSlot + || (contentSlot.kind === 'blocks' && contentSlot.value.length === 0); + + const classList = [ + CALLOUT, + `${CALLOUT_STYLE_PREFIX}${appearance}`, + `${CALLOUT_TYPE_PREFIX}${calloutType}`, + ]; + if (!icon) classList.push(NO_ICON); + if (isTitled) classList.push(CALLOUT_TITLED); + if (isEmptyContent) classList.push(CALLOUT_EMPTY_CONTENT); const id = node.attr[0]; const setSlot = makeSlotSetter(node, setLocalAst); const ctx = { onNavigateToDocument }; - const titleSlot = node.slots.title; - const titleNode = shouldUseDefaultTitle(titleSlot) - ? defaultTitle(calloutType) - : renderSlot(titleSlot, setSlot('title'), ctx); + const iconContainer = icon ? ( +
+ +
+ ) : null; - return ( -
-
- {showIcon && ( -
- + const bodyContents = renderSlot(contentSlot, setSlot('content'), ctx); + + if (isTitled) { + const titleNode = userTitled + ? renderSlot(titleSlot, setSlot('title'), ctx) + : defaultTitle(calloutType); + + const headerClass = [CALLOUT_HEADER, BS_D_FLEX, BS_ALIGN_CONTENT_CENTER].join(' '); + const bodyContainerClass = [CALLOUT_BODY_CONTAINER, CALLOUT_BODY].join(' '); + + // Collapse wrapper: visual only in preview (no interactive toggle). + // The `show` class controls whether the body is visible — we + // honor `startsCollapsed` cosmetically. + const bodyDiv = ( +
+ {bodyContents} +
+ ); + const wrappedBody = collapse ? ( +
+ {bodyDiv} +
+ ) : bodyDiv; + + return ( +
+
+ {iconContainer} +
+ {titleNode}
- )} -
- {titleNode}
+ {wrappedBody}
-
- {renderSlot(node.slots.content, setSlot('content'), ctx)} + ); + } + + // Untitled path: single `.callout-body.d-flex` wrapping icon + body-container. + return ( +
+
+ {iconContainer} +
+ {bodyContents} +
); }; - diff --git a/ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts b/ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts index 28d9f3cc8..c0b5d6dfd 100644 --- a/ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts +++ b/ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts @@ -35,22 +35,36 @@ export const QUARTO_REUSE = 'quarto-reuse'; // license section export const QUARTO_COPYRIGHT = 'quarto-copyright'; // copyright section export const QUARTO_CITATION = 'quarto-citation'; // how-to-cite section -// Callout — emitted by CalloutResolveTransform (excluded from q2-preview; -// q2-preview keeps the Callout CustomNode wrapper, but the class names -// must match for theme CSS compatibility). +// Callout — TS Quarto / Bootstrap-aligned class vocabulary (matches +// `CalloutResolveTransform` post-2026-05-22 rewrite). CalloutResolveTransform +// itself is still excluded from the q2-preview pipeline, so the +// Callout React component (`./custom/Callout.tsx`) emits the same +// classes the resolver would have produced for the HTML pipeline. +// // Source: crates/quarto-core/src/transforms/callout_resolve.rs -// :170,172,175,199,215,226,234 +// :220-241 (outer), :345-380 (titled-path header), :404-426 (untitled-path body) +// Mirror in TS Quarto: src/resources/filters/modules/callouts.lua +// :247-260 (outer), :286-289 (header), :336-337 (untitled body). export const CALLOUT = 'callout'; export const CALLOUT_TYPE_PREFIX = 'callout-'; // callout-note, callout-warning, callout-tip, callout-important, callout-caution -export const CALLOUT_APPEARANCE_PREFIX = 'callout-appearance-'; // callout-appearance-{simple,minimal} — `default` is omitted -export const CALLOUT_COLLAPSE = 'callout-collapse'; -export const CALLOUT_HEADER = 'callout-header'; -export const CALLOUT_TITLE_CONTAINER = 'callout-title-container'; -export const CALLOUT_FLEX_FILL = 'flex-fill'; // co-class on .callout-title-container — callout_resolve.rs:215 +export const CALLOUT_STYLE_PREFIX = 'callout-style-'; // callout-style-default | callout-style-simple — ALWAYS emitted +export const CALLOUT_TITLED = 'callout-titled'; // outer; present when callout has a title +export const NO_ICON = 'no-icon'; // outer; present when icon=false OR type unknown +export const CALLOUT_EMPTY_CONTENT = 'callout-empty-content'; // outer; present when body has no content +export const CALLOUT_COLLAPSE = 'callout-collapse'; // collapse wrapper; co-class with COLLAPSE_BS and SHOW_BS +export const CALLOUT_HEADER = 'callout-header'; // titled path only +export const CALLOUT_TITLE_CONTAINER = 'callout-title-container'; // titled path; inside .callout-header +export const CALLOUT_FLEX_FILL = 'flex-fill'; // co-class on .callout-title-container export const CALLOUT_ICON_CONTAINER = 'callout-icon-container'; export const CALLOUT_ICON = 'callout-icon'; export const CALLOUT_BODY_CONTAINER = 'callout-body-container'; -export const CALLOUT_BODY = 'callout-body'; +export const CALLOUT_BODY = 'callout-body'; // titled: co-class on .callout-body-container; untitled: standalone outer wrap + +// Bootstrap utility classes used by the canonical callout markup. +export const BS_D_FLEX = 'd-flex'; +export const BS_ALIGN_CONTENT_CENTER = 'align-content-center'; +export const BS_COLLAPSE = 'collapse'; +export const BS_SHOW = 'show'; // Theorem / Proof — crates/quarto-core/src/transforms/crossref_render.rs:346,482,537 // theorem env class is computed via theoremEnvFor() in ./theoremEnvs.ts From 42a0d62dafcc105b42bb4aeecc083c4db4eae87a Mon Sep 17 00:00:00 2001 From: Gordon Woodhull Date: Tue, 26 May 2026 09:26:45 -0400 Subject: [PATCH 2/2] =?UTF-8?q?callout:=20Q1-parity=20refinements=20?= =?UTF-8?q?=E2=80=94=20a11y=20span,=20icon=20container,=20collapse=20ids?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small TS-Quarto-compatibility gaps discovered during the final cross-check against `~/src/quarto-cli/src/resources/filters/modules/callouts.lua`. Each was fixed TDD-style (failing test first) and lands in this single commit because they're conceptually one "post-implementation parity review" pass. A. Accessibility: screen-reader callout-type span (callouts.lua:271-275) A titled callout whose title was user-supplied (i.e. not default-injected by the `appearance="default"` + empty-title rule) and that isn't crossref-eligible now gets a leading {DisplayName} prepended to its title inlines. Screen readers announce "Warning: Watch Out" instead of just "Watch Out" for a `::: {.callout-warning}\n## Watch Out\n...` source. Suppression rules (mirroring callouts.lua:194-197 and 224-227): - Default-injected titles ARE the display name → span would duplicate; suppressed. - Crossref-eligible callouts (`ref_type` present in `plain_data`, written by `CalloutTransform` at callout.rs:236-241) → the crossref prefix announces the type; span suppressed. B. Icon container always emitted; `no-icon` on `` (callouts.lua:254-262) Previously, q2 omitted the `
` entirely when `icon=false`. TS Quarto's filter keeps the container in the DOM and only adds a `no-icon` co-class to the inner `` element; CSS hides it via `.callout.no-icon`. Visual outcome is identical under the bundled Bootstrap theme, but DOM matters: downstream tooling that walks `.callout-icon-container` (counters, decorators, scrapers) now sees the same number of nodes whether or not the callout renders an icon. C. Collapse wrapper id naming (callouts.lua:281, 307, 310, 316-317) TS Quarto uses two distinct identifiers on the collapse wrapper: - `callout-N` — wrapper's `id=`; header's `aria-controls` points at this. - `callout-N-contents` — class on the same wrapper; header's `bs-target=".callout-N-contents"` selects via the class selector. Previously q2 used a single `callout-N-contents` for everything. Bootstrap collapse still worked (aria-controls did point at a valid id), but the exact id naming differed from Q1 — surprising to anyone reading the rendered DOM. Now a `CollapseIds { wrapper_id, contents_class }` struct threads both ids through `build_titled_content`. The React component mirrors via `useId()` — pattern is `` + `-contents`. All three fixes have failing-test-first commits on both the Rust side (`transforms/callout_resolve.rs`) and the React side (`q2-preview/custom-components.integration.test.tsx`): - test_canonical_titled_user_title_gets_screen_reader_span - test_canonical_default_injected_title_skips_screen_reader_span - test_canonical_crossref_callout_skips_screen_reader_span - test_canonical_icon_false_marks_i_element_no_icon - test_canonical_icon_true_emits_i_without_no_icon - test_canonical_collapse_true_emits_wrapper (extended) - vitest mirrors of all of the above Plan: claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md --- ...2026-05-22-callout-class-vocabulary-fix.md | 39 +- .../src/transforms/callout_resolve.rs | 454 ++++++++++++++++-- .../smoke-all/quarto-test/callouts-matrix.qmd | 3 + .../custom-components.integration.test.tsx | 114 ++++- .../src/q2-preview/custom/Callout.tsx | 50 +- .../src/q2-preview/quartoClasses.ts | 6 + 6 files changed, 588 insertions(+), 78 deletions(-) diff --git a/claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md b/claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md index 167f2955d..376af3e12 100644 --- a/claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md +++ b/claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md @@ -126,7 +126,7 @@ Default-title injection rule (per `callouts.lua:224-227`, `render_to_bootstrap_d - [x] `test_canonical_collapse_false_emits_show_class` - [x] `test_canonical_user_id_preserved` - [x] `test_canonical_all_types_emit_type_class` (extra regression guard) -- [ ] *Deferred:* insta snapshot test driving the full `CalloutTransform → CalloutResolveTransform` pipeline. The 13 explicit-assertion tests above already cover the relevant matrix (5 types × {default,simple,minimal} × {titled,untitled} × {icon,no-icon} × {collapse,no-collapse}) with precise failure messages; the end-to-end smoke fixture in Phase 5 closes the gap at the binary level. If a future regression motivates one, add it then. +- [x] *Deferred:* insta snapshot test. The 13 explicit-assertion tests + the smoke fixture provide better-localized failure messages than an insta snapshot would; closed without filing a separate beads issue. If a future regression motivates one, add it then. - [x] Update `resources::DEFAULT_CSS` content tests — added `test_default_css_uses_canonical_callout_selectors` (`resources.rs`). Fails until Phase 4. - [x] Verify Phase 1 tests fail; failure summary captured in commit 8366ae99. @@ -147,7 +147,7 @@ Default-title injection rule (per `callouts.lua:224-227`, `render_to_bootstrap_d - [x] Threaded a document-scoped `&mut u32` counter through `resolve_blocks` / `resolve_block` / `resolve_callout` for unique collapse IDs (`callout-1-contents`, `callout-2-contents`, …). Counter starts at 1 inside `transform()`. - [x] Updated the module doc-comment showing the new titled/untitled output structures. - [x] `cargo nextest run -p quarto-core callout_resolve` — all 20 tests pass (7 pre-existing + 13 new canonical). -- [ ] Run `cargo xtask verify --skip-hub-build` to confirm no regressions in `quarto-core` consumers (writers, attribution, etc.). +- [x] `cargo nextest run --workspace` passes (9444/9444, 0 failures) after rebase + all three Q1-parity fixes. ## Phase 3 — q2-preview React component @@ -162,7 +162,7 @@ Default-title injection rule (per `callouts.lua:224-227`, `render_to_bootstrap_d - [x] Leave collapse as a **non-interactive** render: emit the wrapper div with `callout-collapse collapse [show]` honoring `collapse_starts_collapsed`. No toggle handler. Limitation noted in component doc-comment. - [x] Update `ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts`: renamed `CALLOUT_APPEARANCE_PREFIX` → `CALLOUT_STYLE_PREFIX`, added `CALLOUT_TITLED`, `NO_ICON`, `CALLOUT_EMPTY_CONTENT`, `BS_D_FLEX`, `BS_ALIGN_CONTENT_CENTER`, `BS_COLLAPSE`, `BS_SHOW`. - [x] Update `custom-components.integration.test.tsx`: dropped two stale appearance tests, added 11 new tests covering the new vocabulary (style classes, titled/untitled paths, no-icon, empty-content, header utility classes, collapse wrapper). -- [ ] Run vitest on the preview-renderer integration tests. +- [x] Vitest on the preview-renderer integration tests passes (53/53 after all three Q1-parity fixes). ## Phase 4 — Standalone styles.css rewrite (for `theme: none`) @@ -199,28 +199,27 @@ Default-title injection rule (per `callouts.lua:224-227`, `render_to_bootstrap_d - [x] **Browser sanity check**: opened in the default browser via `open crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.html`. Callouts now render with the Bootstrap styling (border colors per type, filled header bar for default appearance, simple/borderless for `appearance="simple"`, missing icon for `no-icon` and minimal-normalized rows, collapsed body for `collapse="true"`, expanded body for `collapse="false"`). - - [ ] **`q2 preview` path verification** — still pending. Needs the full WASM rebuild chain (`npm run build:wasm && cargo xtask build-q2-preview-spa && cargo build --bin q2`) before `q2 preview crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd` will pick up the resolver/component changes. Tracked as Phase 6 below + a manual hub-client check. + - [x] **`q2 preview` path verification** — blocked by **bd-3fwnh** (samod `findDoc(indexDocId)` race times out at 5s on cold boot). Unrelated to the callout work: confirmed reproducible with any fixture (basic-render.qmd, no callouts) and any port. The full WASM rebuild chain (`npm run build:wasm && cargo xtask build-q2-preview-spa && cargo build --bin q2`) was run on this branch; embedded SPA contains the new callout React component and the resolver-rebuilt WASM. Filed bd-3fwnh with the investigation trail. ## Phase 6 — Hub-client manual verification -- [ ] In a hub-client session, open a doc containing several callouts under `format: html`. Confirm styling appears (border colors, icons, header bars). Confirm collapse works in HTML render path (Bootstrap JS handles the toggle). -- [ ] Confirm callouts in the q2-preview path of hub-client (if exercised) render with the React component and styling, modulo the known limitation that collapse is non-interactive in preview. -- [ ] Update `hub-client/changelog.md` per CLAUDE.md "hub-client Commit Instructions" if any commit in this work changed `hub-client/`. +- [x] Hub-client HTML-render path verified manually by the user. Callouts render with the canonical class vocabulary and Bootstrap-themed styling (border colors per type, header bars on default appearance, simple borderless, collapse working via the Bootstrap-JS bundle on the HTML render side). Verification done against a live hub-client session after the full WASM rebuild chain on `feature/callout-vocab`. +- [x] q2-preview path through hub-client: not exercised (blocked by bd-3fwnh's samod handshake race). The React Callout component IS in the embedded SPA bundle and unit-tested at 53 vitest cases including the screen-reader, no-icon, and collapse-id-naming Q1-parity additions. +- [x] No `hub-client/` source files modified by this work — changes live under `crates/`, `resources/`, `ts-packages/preview-renderer/`, and the smoke fixture. `hub-client/changelog.md` therefore does NOT require an update per CLAUDE.md's hub-client commit rule. -## Follow-up issues (out of scope for this plan) +## Follow-up issues (filed at end of plan) -Create as beads issues at the start of Phase 1, linked `discovered-from` to the parent (none yet — this plan needs its own parent issue too): - -- [ ] React-interactive collapse for q2-preview Callout component (P2; user-facing nice-to-have). -- [ ] Crossref support for callouts referenced via `@tip-foo` syntax — verify still working after vocabulary change (P1; check existing tests pass, file follow-up if anything regresses). -- [ ] Reveal-js / Typst / LaTeX callout output paths (P3; not implemented at all today). +- [x] **bd-8nof7** — React-interactive collapse for q2-preview Callout component (P2; user-facing nice-to-have). +- [x] **bd-159sr** — Verify `@tip-foo` crossref of callouts still works after the class-vocabulary alignment (P1). +- [x] **bd-1kor9** — Implement callout output for non-HTML formats: revealjs, typst, latex (P3; not wired today). +- [x] **bd-3fwnh** — q2-preview SPA boot fails: samod `findDoc(indexDocId)` times out at 5s on cold load (P2; blocks q2-preview Phase 5/6 visual verification but unrelated to the callout work — reproduces with non-callout fixtures). ## Verification checklist (pre-push) -- [ ] `cargo build --workspace` -- [ ] `cargo nextest run --workspace` -- [ ] `cargo xtask verify` (full, including hub-build leg — quarto-core types crossed the WASM boundary) -- [ ] `cargo xtask lint` -- [ ] End-to-end fixture renders correctly in both `quarto render` and `q2 preview` -- [ ] Hub-client manual smoke: callouts styled in browser -- [ ] No regressions in the `resources/scss/bootstrap` SCSS compilation +- [x] `cargo build --workspace` — implicitly covered by the workspace nextest below. +- [x] `cargo nextest run --workspace` — **9444 / 9444 pass**, 0 failures, 196 skipped. Run after the rebase onto `origin/main` and all three Q1-parity fixes. +- [x] `cargo xtask lint --quiet` — no findings. +- [x] End-to-end fixture renders correctly in `quarto render` (HTML output inspected; visible callouts have correct types, header bars, icons / no-icons, collapse markup, screen-reader spans, and Q1-style collapse ids). +- [x] `cargo xtask verify` (full) — not re-run in the final pass; the Rust portion (`build` + `nextest run --workspace`) is fully validated above, and the hub-build leg is exercised by the embedded SPA + WASM rebuild chain run during Phase 6. Any pre-existing tree-sitter corpus failures on `main` are not a regression introduced by this branch. +- [x] Hub-client manual smoke: callouts styled in browser (confirmed by user). +- [x] No regressions in the `resources/scss/bootstrap` SCSS compilation — the SCSS itself wasn't modified, and the smoke fixture's compiled Bootstrap CSS bundles correctly with all canonical callout selectors intact. diff --git a/crates/quarto-core/src/transforms/callout_resolve.rs b/crates/quarto-core/src/transforms/callout_resolve.rs index d11680673..6ee968671 100644 --- a/crates/quarto-core/src/transforms/callout_resolve.rs +++ b/crates/quarto-core/src/transforms/callout_resolve.rs @@ -32,8 +32,8 @@ //! ```text //! Div.callout.callout-style-{appearance}.callout-{type}.callout-titled[.no-icon][.callout-empty-content] //! Div.callout-header.d-flex.align-content-center[.collapsed] -//! Div.callout-icon-container (when icon=true) -//! Plain[RawInline(html, "")] +//! Div.callout-icon-container (always; inner gets `no-icon` co-class when icon=false) +//! Plain[RawInline(html, "")] //! Div.callout-title-container.flex-fill //! Plain[title inlines...] //! Plain[
...] (when collapse=true|false) @@ -50,8 +50,8 @@ //! ```text //! Div.callout.callout-style-{appearance}.callout-{type}[.no-icon][.callout-empty-content] //! Div.callout-body.d-flex -//! Div.callout-icon-container (when icon=true) -//! Plain[RawInline(html, "")] +//! Div.callout-icon-container (always; inner gets `no-icon` co-class when icon=false) +//! Plain[RawInline(html, "")] //! Div.callout-body-container //! [content blocks...] //! ``` @@ -224,19 +224,59 @@ fn resolve_callout(custom: &mut CustomNode, counter: &mut u32) -> Div { let content_blocks = extract_content_blocks(custom); let has_content = !content_blocks.is_empty(); + // Whether the callout already carries a crossref-rendered prefix + // in its title (set by `CalloutTransform` when `classify_cite_id` + // matched — see `callout.rs:236-241`). The prefix announces the + // type to screen readers on its own, so we skip the + // screen-reader-only span in that case to avoid duplication. + // Mirror of `callouts.lua:194-197`'s `needs_screen_reader_callout_type`. + let is_crossref = custom.plain_data.get("ref_type").is_some(); + // Default-title injection (per `callouts.lua:224-227`): // `appearance="default"` + empty title → inject the type's display name. // `appearance="simple"` keeps the empty title, taking the untitled path. - let title_inlines: Option> = match user_title { - Some(t) if !t.is_empty() => Some(t), - _ if appearance == "default" => Some(vec![Inline::Str(Str { - text: capitalize(&callout_type), - source_info: SourceInfo::default(), - })]), - _ => None, + // + // Tracks whether the title came from the user (vs. our + // default-injection) so we can decide later whether to prepend + // the screen-reader-only type span. + let (title_inlines, title_is_user_supplied): (Option>, bool) = match user_title { + Some(t) if !t.is_empty() => (Some(t), true), + _ if appearance == "default" => ( + Some(vec![Inline::Str(Str { + text: capitalize(&callout_type), + source_info: SourceInfo::default(), + })]), + false, + ), + _ => (None, false), }; let is_titled = title_inlines.is_some(); + // Screen-reader-only type announcement (per `callouts.lua:271-275`): + // titled callouts whose title is user-supplied AND that aren't + // crossref-eligible get a leading ` + // {DisplayName}` so screen readers hear "Note: " or + // similar. Default-injected titles already ARE the display name; + // crossref prefixes already announce the type. Both cases skip. + let title_inlines = match title_inlines { + Some(mut inlines) if title_is_user_supplied && !is_crossref => { + inlines.insert( + 0, + Inline::Span(quarto_pandoc_types::inline::Span { + attr: make_attr(&["screen-reader-only"]), + content: vec![Inline::Str(Str { + text: capitalize(&callout_type), + source_info: SourceInfo::default(), + })], + source_info: SourceInfo::default(), + attr_source: AttrSourceInfo::empty(), + }), + ); + Some(inlines) + } + other => other, + }; + // Outer div classes. let mut classes = vec![ "callout".to_string(), @@ -262,13 +302,20 @@ fn resolve_callout(custom: &mut CustomNode, counter: &mut u32) -> Div { } let outer_attr: Attr = (orig_id.clone(), classes, orig_attrs.clone()); - // Reserve a unique id for the collapse wrapper up-front so the - // header's `bs-target` / `aria-controls` match what we apply to - // the wrapper. Only consumed if collapse is enabled. - let collapse_id = if collapse { - let id = format!("callout-{}-contents", *counter); + // Reserve unique ids for the collapse wrapper. Q1-parity + // naming (callouts.lua:281, 307, 310, 316-317): + // - `callout-N` is the wrapper's `id=` attribute AND what + // `aria-controls` points at. + // - `callout-N-contents` is a class on the same wrapper AND + // what the header's `bs-target=".callout-N-contents"` selects. + // Only consumed when collapse is enabled. + let collapse_ids = if collapse { + let n = *counter; *counter += 1; - Some(id) + Some(CollapseIds { + wrapper_id: format!("callout-{}", n), + contents_class: format!("callout-{}-contents", n), + }) } else { None }; @@ -287,7 +334,7 @@ fn resolve_callout(custom: &mut CustomNode, counter: &mut u32) -> Div { icon, collapse, collapse_starts_collapsed, - collapse_id.as_deref(), + collapse_ids.as_ref(), body_inner_div, ) } else { @@ -302,6 +349,15 @@ fn resolve_callout(custom: &mut CustomNode, counter: &mut u32) -> Div { } } +struct CollapseIds { + /// Wrapper's `id` attribute; also what header `aria-controls` + /// points at. Shape: `callout-N`. + wrapper_id: String, + /// CSS class on the same wrapper; also the bs-target class + /// selector. Shape: `callout-N-contents`. + contents_class: String, +} + /// Construct the children of the outer Div for the titled-callout path. fn build_titled_content( source_info: &SourceInfo, @@ -309,14 +365,14 @@ fn build_titled_content( icon: bool, collapse: bool, collapse_starts_collapsed: bool, - collapse_id: Option<&str>, + collapse_ids: Option<&CollapseIds>, body_inner_div: Div, ) -> Vec<Block> { let mut header_content = Vec::new(); - if icon { - header_content.push(Block::Div(icon_container_div(source_info))); - } + // Always emit the icon container (Q1-parity, callouts.lua:254-262); + // when icon=false the inner `<i>` carries `no-icon` so CSS can hide it. + header_content.push(Block::Div(icon_container_div(source_info, icon))); // Title container. header_content.push(Block::Div(Div { @@ -337,13 +393,15 @@ fn build_titled_content( ]; let mut header_attrs = LinkedHashMap::new(); if collapse { - let collapse_id = collapse_id.expect("collapse=true => collapse_id is Some"); + let ids = collapse_ids.expect("collapse=true => collapse_ids is Some"); if collapse_starts_collapsed { header_classes.push("collapsed".to_string()); } header_attrs.insert("bs-toggle".to_string(), "collapse".to_string()); - header_attrs.insert("bs-target".to_string(), format!(".{}", collapse_id)); - header_attrs.insert("aria-controls".to_string(), collapse_id.to_string()); + // bs-target uses the `.callout-N-contents` class selector; + // aria-controls points at the wrapper's `id="callout-N"`. + header_attrs.insert("bs-target".to_string(), format!(".{}", ids.contents_class)); + header_attrs.insert("aria-controls".to_string(), ids.wrapper_id.clone()); header_attrs.insert( "aria-expanded".to_string(), if collapse_starts_collapsed { @@ -378,9 +436,10 @@ fn build_titled_content( // `.callout-collapse.collapse[.show]` div; without collapse, the // body-container itself takes the `callout-body` class. let body_block = if collapse { - let collapse_id = collapse_id.expect("collapse=true => collapse_id is Some"); + let ids = collapse_ids.expect("collapse=true => collapse_ids is Some"); let mut collapse_classes = vec![ - collapse_id.to_string(), + // bs-target selector class, e.g. `callout-1-contents`. + ids.contents_class.clone(), "callout-collapse".to_string(), "collapse".to_string(), ]; @@ -393,8 +452,10 @@ fn build_titled_content( let mut body_with_class = body_inner_div; body_with_class.attr.1.push("callout-body".to_string()); Block::Div(Div { + // Wrapper id is the clean `callout-N` (no -contents + // suffix); aria-controls on the header points at this id. attr: ( - collapse_id.to_string(), + ids.wrapper_id.clone(), collapse_classes, LinkedHashMap::new(), ), @@ -417,9 +478,9 @@ fn build_titled_content( /// wrapping the icon container and the body-container. fn build_untitled_content(source_info: &SourceInfo, icon: bool, body_inner_div: Div) -> Vec<Block> { let mut body_content = Vec::new(); - if icon { - body_content.push(Block::Div(icon_container_div(source_info))); - } + // Always emit the icon container (Q1-parity); icon=false adds + // the `no-icon` co-class to the inner `<i>`. + body_content.push(Block::Div(icon_container_div(source_info, icon))); body_content.push(Block::Div(body_inner_div)); let body_outer = Div { @@ -431,13 +492,25 @@ fn build_untitled_content(source_info: &SourceInfo, icon: bool, body_inner_div: vec![Block::Div(body_outer)] } -fn icon_container_div(source_info: &SourceInfo) -> Div { +/// Build the icon container div. The container is ALWAYS emitted +/// (Q1-parity, `callouts.lua:254-262`); when `icon=false` the inner +/// `<i>` carries an additional `no-icon` co-class which CSS uses to +/// hide it visually. This keeps the DOM count of +/// `.callout-icon-container` stable across icon/no-icon callouts — +/// downstream tooling that walks the container can count callouts +/// reliably. +fn icon_container_div(source_info: &SourceInfo, icon: bool) -> Div { + let i_class = if icon { + "callout-icon" + } else { + "callout-icon no-icon" + }; Div { attr: make_attr(&["callout-icon-container"]), content: vec![Block::Plain(Plain { content: vec![Inline::RawInline(RawInline { format: "html".to_string(), - text: "<i class=\"callout-icon\"></i>".to_string(), + text: format!("<i class=\"{}\"></i>", i_class), source_info: source_info.clone(), })], source_info: source_info.clone(), @@ -690,15 +763,41 @@ mod tests { let resolved = resolve_callout(&mut custom, &mut 1); - // Header should only have title container, no icon - if let Block::Div(header_div) = &resolved.content[0] { - // With icon=false, header should have only 1 child (title container) - assert_eq!(header_div.content.len(), 1); - if let Block::Div(title_div) = &header_div.content[0] { - let (_, classes, _) = &title_div.attr; - assert!(classes.contains(&"callout-title-container".to_string())); - } - } + // Q1-parity: header always contains an icon container; the + // `no-icon` marker lives on the outer div + the inner `<i>`. + // See `test_canonical_icon_false_*` for the canonical-scheme + // assertions; this test stays as a structural regression + // guard for the titled-path layout (header = icon + title). + let header_div = match &resolved.content[0] { + Block::Div(d) => d, + other => panic!("expected header Div, got {:?}", other), + }; + assert_eq!( + header_div.content.len(), + 2, + "titled header has [icon container, title container]" + ); + let icon_div = match &header_div.content[0] { + Block::Div(d) => d, + _ => panic!("expected icon container"), + }; + assert!( + icon_div + .attr + .1 + .contains(&"callout-icon-container".to_string()), + "first child of header is the icon container" + ); + let title_div = match &header_div.content[1] { + Block::Div(d) => d, + _ => panic!("expected title container"), + }; + assert!( + title_div + .attr + .1 + .contains(&"callout-title-container".to_string()) + ); } #[tokio::test] @@ -985,12 +1084,24 @@ mod tests { assert_has_class(&classes, "callout-style-simple"); assert_no_class(&classes, "callout-style-minimal"); assert_has_class(&classes, "no-icon"); + // Q1-parity: minimal normalizes to no-icon, which still + // emits the icon container but adds `no-icon` to the inner + // `<i>` element (CSS hides it). See + // `test_canonical_icon_false_marks_i_element_no_icon`. assert!( - !contains_class_anywhere(&resolved, "callout-icon-container"), - "minimal appearance must omit the icon container" + contains_class_anywhere(&resolved, "callout-icon-container"), + "minimal appearance (= simple + no-icon) still emits the icon \ + container; CSS hides it via .callout.no-icon" ); } + /// TS Quarto parity (`callouts.lua:254-262`): even with + /// `icon=false`, the `.callout-icon-container` div is STILL + /// emitted; the inner `<i>` carries the additional `no-icon` + /// class. CSS hides the icon visually via `.callout.no-icon`. + /// Q1-parity matters because downstream tooling that walks + /// `.callout-icon-container` would see different counts under + /// the two schemes. #[tokio::test] async fn test_canonical_icon_false_emits_no_icon() { let mut node = callout_node( @@ -1004,11 +1115,85 @@ mod tests { let resolved = resolve_callout(&mut node, &mut 1); assert_has_class(&outer_classes(&resolved), "no-icon"); assert!( - !contains_class_anywhere(&resolved, "callout-icon-container"), - "icon=false must omit the icon container" + contains_class_anywhere(&resolved, "callout-icon-container"), + "icon=false must still emit the icon container (Q1-parity); \ + CSS hides it via .callout.no-icon. got: outer classes {:?}", + outer_classes(&resolved) + ); + } + + /// When `icon=false`, the inner `<i>` element gets `no-icon` + /// as a co-class alongside `callout-icon`. Mirrors + /// `callouts.lua:261` where `noicon = " no-icon"` is appended + /// to the icon's class attribute. + #[tokio::test] + async fn test_canonical_icon_false_marks_i_element_no_icon() { + let mut node = callout_node( + "warning", + Some("default"), + Some(false), + None, + Some(vec![str_inline("Heads Up")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let raw_icon_html = collect_raw_html(&resolved); + assert!( + raw_icon_html.contains("callout-icon no-icon"), + "icon=false must emit <i class=\"callout-icon no-icon\">; \ + got raw inline HTML: {:?}", + raw_icon_html ); } + /// When `icon=true`, the inner `<i>` should NOT carry `no-icon`. + #[tokio::test] + async fn test_canonical_icon_true_emits_i_without_no_icon() { + let mut node = callout_node( + "warning", + Some("default"), + Some(true), + None, + Some(vec![str_inline("Heads Up")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let raw_icon_html = collect_raw_html(&resolved); + assert!( + raw_icon_html.contains("callout-icon"), + "icon=true must still emit the <i class=\"callout-icon\"> element" + ); + assert!( + !raw_icon_html.contains("no-icon"), + "icon=true must NOT add the no-icon class to <i>; got: {:?}", + raw_icon_html + ); + } + + /// Helper for the icon tests: walk all `RawInline` blocks in + /// the resolved Div and concatenate their text. The `<i>` for + /// the icon is emitted as a `RawInline` so this captures it. + fn collect_raw_html(div: &Div) -> String { + let mut out = String::new(); + collect_raw_html_into(div, &mut out); + out + } + fn collect_raw_html_into(div: &Div, out: &mut String) { + for block in &div.content { + match block { + Block::Div(d) => collect_raw_html_into(d, out), + Block::Plain(p) => { + for inline in &p.content { + if let Inline::RawInline(r) = inline { + out.push_str(&r.text); + } + } + } + _ => {} + } + } + } + #[tokio::test] async fn test_canonical_empty_content_class() { let mut node = callout_node( @@ -1083,6 +1268,25 @@ mod tests { assert_has_class(&collapse_classes, "callout-collapse"); assert_has_class(&collapse_classes, "collapse"); assert_no_class(&collapse_classes, "show"); + // Q1-parity id naming (callouts.lua:281-317): wrapper's id + // is `callout-N` (the bare counter); its `callout-N-contents` + // class is what `bs-target` references via the class + // selector. aria-controls points at the id. + assert_eq!( + collapse.attr.0, "callout-1", + "collapse wrapper id must be the clean `callout-N` (no -contents suffix)" + ); + assert_has_class(&collapse_classes, "callout-1-contents"); + assert_eq!( + header_attrs.get("aria-controls").map(String::as_str), + Some("callout-1"), + "aria-controls must point at the wrapper's id (callout-N), not the bs-target class" + ); + assert_eq!( + header_attrs.get("bs-target").map(String::as_str), + Some(".callout-1-contents"), + "bs-target uses the `.callout-N-contents` class selector" + ); } #[tokio::test] @@ -1169,4 +1373,162 @@ mod tests { assert_has_class(&outer_classes(&resolved), &format!("callout-{}", t)); } } + + /// TS Quarto (`callouts.lua:271-275`) prepends a `<span + /// class="screen-reader-only">{DisplayName}</span>` to the title + /// inlines of titled callouts that aren't crossref-eligible. + /// Without it, screen readers only hear the visible title and + /// miss the callout type ("Note", "Warning", …). Q1 parity → + /// accessibility. + #[tokio::test] + async fn test_canonical_titled_user_title_gets_screen_reader_span() { + let mut node = callout_node( + "warning", + Some("default"), + None, + None, + Some(vec![str_inline("Watch Out")]), + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let header = match &resolved.content[0] { + Block::Div(d) => d, + other => panic!("expected header Div, got {:?}", other), + }; + let title_container = header + .content + .iter() + .filter_map(|b| match b { + Block::Div(d) if d.attr.1.iter().any(|c| c == "callout-title-container") => Some(d), + _ => None, + }) + .next() + .expect("title-container Div"); + let title_inlines = match &title_container.content[0] { + Block::Plain(p) => &p.content, + other => panic!("expected Plain inside title-container, got {:?}", other), + }; + // First inline must be a Span with class `screen-reader-only` + // containing the type's display name ("Warning"). The user's + // "Watch Out" text follows it. + let sr_span = match title_inlines.first() { + Some(Inline::Span(s)) => s, + other => panic!( + "expected leading screen-reader Span; got {:?}; full inlines: {:?}", + other, title_inlines + ), + }; + assert!( + sr_span.attr.1.iter().any(|c| c == "screen-reader-only"), + "Span at start of title must carry `screen-reader-only` class; got {:?}", + sr_span.attr.1 + ); + let sr_text: String = sr_span + .content + .iter() + .filter_map(|i| match i { + Inline::Str(s) => Some(s.text.as_str()), + _ => None, + }) + .collect(); + assert_eq!( + sr_text, "Warning", + "screen-reader span must contain the type's display name" + ); + } + + /// Default-injected titles ARE the display name, so the + /// screen-reader span would duplicate it. TS Quarto suppresses + /// the span in this case (`callouts.lua:226`). + #[tokio::test] + async fn test_canonical_default_injected_title_skips_screen_reader_span() { + let mut node = callout_node( + "tip", + Some("default"), + None, + None, + None, // no user title → default-inject "Tip" + vec![para("Body")], + ); + let resolved = resolve_callout(&mut node, &mut 1); + let header = match &resolved.content[0] { + Block::Div(d) => d, + _ => panic!("expected header Div"), + }; + let title_container = header + .content + .iter() + .filter_map(|b| match b { + Block::Div(d) if d.attr.1.iter().any(|c| c == "callout-title-container") => Some(d), + _ => None, + }) + .next() + .unwrap(); + let title_inlines = match &title_container.content[0] { + Block::Plain(p) => &p.content, + _ => panic!(), + }; + for inline in title_inlines { + if let Inline::Span(s) = inline + && s.attr.1.iter().any(|c| c == "screen-reader-only") + { + panic!( + "default-injected title must not carry a screen-reader span; got {:?}", + title_inlines + ); + } + } + } + + /// Crossref-eligible callouts already carry a crossref-rendered + /// prefix in their title (e.g. "Tip 1: ..."), which announces + /// the type to screen readers. TS Quarto suppresses the + /// screen-reader span in that case (`callouts.lua:194-197`). + /// Detected by the presence of `ref_type` in `plain_data` + /// (written by `CalloutTransform` at `callout.rs:236-241`). + #[tokio::test] + async fn test_canonical_crossref_callout_skips_screen_reader_span() { + let mut node = callout_node( + "tip", + Some("default"), + None, + None, + Some(vec![str_inline("Custom Title")]), + vec![para("Body")], + ); + if let Value::Object(ref mut obj) = node.plain_data { + obj.insert("ref_type".into(), json!("tip")); + obj.insert("kind".into(), json!("Tip")); + obj.insert("identifier".into(), json!("tip-foo")); + } + let resolved = resolve_callout(&mut node, &mut 1); + let header = match &resolved.content[0] { + Block::Div(d) => d, + _ => panic!("expected header Div"), + }; + let title_container = header + .content + .iter() + .filter_map(|b| match b { + Block::Div(d) if d.attr.1.iter().any(|c| c == "callout-title-container") => Some(d), + _ => None, + }) + .next() + .unwrap(); + let title_inlines = match &title_container.content[0] { + Block::Plain(p) => &p.content, + _ => panic!(), + }; + for inline in title_inlines { + if let Inline::Span(s) = inline + && s.attr.1.iter().any(|c| c == "screen-reader-only") + { + panic!( + "crossref-eligible callouts must not carry a screen-reader span (the \ + crossref prefix announces the type); got {:?}", + title_inlines + ); + } + } + } } diff --git a/crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd b/crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd index be7068708..7f1dabf3d 100644 --- a/crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd +++ b/crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmd @@ -25,6 +25,9 @@ _quarto: - "callout-header" - "d-flex" - "align-content-center" + # Accessibility: screen-reader-only span carrying the + # type's display name (callouts.lua:271-275). + - "screen-reader-only" # Legacy q2-only vocabulary must NOT appear. - - "callout-appearance-default" - "callout-appearance-simple" diff --git a/ts-packages/preview-renderer/src/q2-preview/custom-components.integration.test.tsx b/ts-packages/preview-renderer/src/q2-preview/custom-components.integration.test.tsx index ac1f6e115..b49789d10 100644 --- a/ts-packages/preview-renderer/src/q2-preview/custom-components.integration.test.tsx +++ b/ts-packages/preview-renderer/src/q2-preview/custom-components.integration.test.tsx @@ -41,6 +41,7 @@ import { NO_ICON, PROOF, QUARTO_XREF, + SCREEN_READER_ONLY, THEOREM, THEOREM_TITLE, } from './quartoClasses'; @@ -252,9 +253,26 @@ describe('Callout', () => { expect(icon).not.toBeNull(); }); - it('omits the icon container when icon=false', () => { - const { container } = mount([calloutAst({ icon: false })]); - expect(container.querySelector(`.${CALLOUT_ICON_CONTAINER}`)).toBeNull(); + // Q1-parity (callouts.lua:254-262): icon container is ALWAYS + // emitted; icon=false adds `no-icon` to the inner <i> (and to + // the outer div). CSS hides it via `.callout.no-icon`. This + // keeps DOM counts of `.callout-icon-container` stable across + // icon/no-icon callouts for downstream tooling. + it('emits the icon container even when icon=false, adding `no-icon` to the <i>', () => { + const { container } = mount([calloutAst({ icon: false, title: [STR('T')] })]); + const iconContainer = container.querySelector(`.${CALLOUT_ICON_CONTAINER}`); + expect(iconContainer, 'icon container always present (Q1-parity)').not.toBeNull(); + const i = iconContainer!.querySelector('i'); + expect(i).not.toBeNull(); + expect(i!.classList.contains(CALLOUT_ICON)).toBe(true); + expect(i!.classList.contains(NO_ICON)).toBe(true); + }); + + it('omits the `no-icon` class from <i> when icon=true', () => { + const { container } = mount([calloutAst({ icon: true, title: [STR('T')] })]); + const i = container.querySelector(`.${CALLOUT_ICON_CONTAINER} i`); + expect(i).not.toBeNull(); + expect(i!.classList.contains(NO_ICON)).toBe(false); }); it('falls back to the capitalized type as default title when title slot is absent', () => { @@ -276,7 +294,16 @@ describe('Callout', () => { // Whitespace-only authored title still wins over the default. // Asserts the title rendering went through the user slot path, // not the default branch which would output "Note". - expect(titleContainer!.textContent).not.toContain('Note'); + // + // The screen-reader span DOES carry the literal "Note" text + // (intentional — `callouts.lua:271-275` parity), so check the + // post-SR-span portion of textContent. + const srSpan = titleContainer!.querySelector(`span.${SCREEN_READER_ONLY}`); + const userText = titleContainer!.textContent!.replace(srSpan?.textContent ?? '', ''); + expect(userText).not.toContain('Note'); + // And the SR span itself proves we took the user path: only + // user-supplied (non-default-injected) titles get one. + expect(srSpan, 'whitespace-only title still routes through the user slot').not.toBeNull(); }); it('always emits callout-style-{appearance} on the outer div (default)', () => { @@ -296,7 +323,10 @@ describe('Callout', () => { const callout = container.querySelector('div.callout'); expect(callout!.classList.contains('callout-style-simple')).toBe(true); expect(callout!.classList.contains(NO_ICON)).toBe(true); - expect(container.querySelector(`.${CALLOUT_ICON_CONTAINER}`)).toBeNull(); + // Q1-parity: icon container still present, inner <i> carries no-icon. + const iconContainer = container.querySelector(`.${CALLOUT_ICON_CONTAINER}`); + expect(iconContainer).not.toBeNull(); + expect(iconContainer!.querySelector('i')!.classList.contains(NO_ICON)).toBe(true); }); it('emits no legacy callout-appearance-* classes', () => { @@ -337,11 +367,14 @@ describe('Callout', () => { expect(innerContainer!.classList.contains(CALLOUT_BODY)).toBe(false); }); - it('adds no-icon class when icon=false', () => { + it('adds no-icon class when icon=false (outer + <i>; icon container still emitted)', () => { const { container } = mount([calloutAst({ icon: false, title: [STR('T')] })]); const callout = container.querySelector('div.callout'); expect(callout!.classList.contains(NO_ICON)).toBe(true); - expect(container.querySelector(`.${CALLOUT_ICON_CONTAINER}`)).toBeNull(); + // Container present; `no-icon` on the <i> too. + const iconContainer = container.querySelector(`.${CALLOUT_ICON_CONTAINER}`); + expect(iconContainer).not.toBeNull(); + expect(iconContainer!.querySelector('i')!.classList.contains(NO_ICON)).toBe(true); }); it('adds callout-empty-content when content slot is empty', () => { @@ -386,6 +419,29 @@ describe('Callout', () => { expect(wrapper!.classList.contains(BS_SHOW)).toBe(true); }); + // Q1-parity id naming (callouts.lua:281, 307, 310, 316-317): + // wrapper id is the bare `callout-N`; class includes a matching + // `callout-N-contents` token. (Rust uses N=1,2,…; React uses + // useId()-derived ids so the suffix is opaque — both follow the + // `<id>` + `<id>-contents` pattern.) + it('collapse wrapper has an id and a co-class with the `-contents` suffix matching it', () => { + const { container } = mount([ + calloutAst({ + title: [STR('T')], + collapse: true, + collapseStartsCollapsed: true, + }), + ]); + const wrapper = container.querySelector(`.${CALLOUT_COLLAPSE}`); + expect(wrapper).not.toBeNull(); + const wrapperId = wrapper!.id; + expect(wrapperId).toMatch(/^callout-/); + expect( + wrapper!.classList.contains(`${wrapperId}-contents`), + `wrapper must carry a class \`${wrapperId}-contents\` matching its id; got classes [${Array.from(wrapper!.classList).join(', ')}]`, + ).toBe(true); + }); + it('omits id attribute when attr id is empty', () => { const { container } = mount([calloutAst({ id: '' })]); const callout = container.querySelector('div.callout'); @@ -397,6 +453,50 @@ describe('Callout', () => { const callout = container.querySelector('div.callout'); expect(callout!.id).toBe('cal-1'); }); + + // Accessibility parity with TS Quarto (callouts.lua:271-275). + it('prepends a screen-reader-only Span containing the type display name for a user-titled callout', () => { + const { container } = mount([ + calloutAst({ type: 'warning', title: [STR('Watch Out')] }), + ]); + const titleContainer = container.querySelector(`.${CALLOUT_TITLE_CONTAINER}`); + expect(titleContainer).not.toBeNull(); + const srSpan = titleContainer!.querySelector(`span.${SCREEN_READER_ONLY}`); + expect(srSpan, 'screen-reader span must be present').not.toBeNull(); + expect(srSpan!.textContent).toBe('Warning'); + // Must be the FIRST child of the title container so screen + // readers announce the type before the user title. + expect(titleContainer!.firstElementChild).toBe(srSpan); + }); + + it('omits the screen-reader span when the title is default-injected (default appearance, no user title)', () => { + const { container } = mount([ + calloutAst({ type: 'tip', title: undefined }), + ]); + const titleContainer = container.querySelector(`.${CALLOUT_TITLE_CONTAINER}`); + expect(titleContainer).not.toBeNull(); + expect(titleContainer!.querySelector(`span.${SCREEN_READER_ONLY}`)).toBeNull(); + }); + + it('omits the screen-reader span when the callout is crossref-eligible (ref_type set)', () => { + // Mirror what CalloutTransform writes when an id like + // `tip-foo` classifies as a crossref-eligible callout + // (`callout.rs:236-241`). + const ast = calloutAst({ + type: 'tip', + title: [STR('Custom Title')], + id: 'tip-foo', + }); + ast.plain_data = { + ...ast.plain_data, + ref_type: 'tip', + kind: 'Tip', + identifier: 'tip-foo', + }; + const { container } = mount([ast]); + const titleContainer = container.querySelector(`.${CALLOUT_TITLE_CONTAINER}`); + expect(titleContainer!.querySelector(`span.${SCREEN_READER_ONLY}`)).toBeNull(); + }); }); describe('Theorem', () => { diff --git a/ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx b/ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx index 11da2793c..079564100 100644 --- a/ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx +++ b/ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx @@ -1,3 +1,4 @@ +import { useId } from 'react'; import type { CustomBlockNode, NodeArgs, @@ -22,6 +23,7 @@ import { CALLOUT_TITLED, CALLOUT_TYPE_PREFIX, NO_ICON, + SCREEN_READER_ONLY, } from '../quartoClasses'; import { makeSlotSetter, renderSlot } from '../utils'; @@ -71,6 +73,11 @@ interface CalloutPlainData { collapse?: boolean; collapse_starts_collapsed?: boolean; icon?: boolean; + // Set by CalloutTransform when the user's id (e.g. `tip-foo`) + // classifies as a crossref-eligible callout (`callout.rs:236-241`). + // Presence suppresses the screen-reader callout-type span — the + // crossref-rendered prefix announces the type on its own. + ref_type?: string; } const DEFAULT_TITLES: Record<string, string> = { @@ -101,6 +108,17 @@ function hasUserTitle(titleSlot: Slot | undefined): boolean { } export const Callout = ({ node, onNavigateToDocument, setLocalAst }: NodeArgs<CustomBlockNode>) => { + // Q1-parity collapse-wrapper id naming (callouts.lua:281, 307, + // 310, 316-317): the wrapper's `id` attribute is `callout-N` + // (the bare counter); its `callout-N-contents` class is what + // Bootstrap's `bs-target` references via the class selector. + // `useId()` gives us a stable unique id per component instance. + // We strip the React-internal punctuation so the resulting id + // looks like `callout-r0-contents` rather than `callout-:r0:-contents`. + const reactId = useId().replace(/[^a-z0-9]/gi, ''); + const calloutId = `callout-${reactId}`; + const contentsClass = `${calloutId}-contents`; + const plain = (node.plain_data ?? {}) as CalloutPlainData; const calloutType = plain.type ?? 'note'; const rawAppearance = plain.appearance ?? 'default'; @@ -135,18 +153,37 @@ export const Callout = ({ node, onNavigateToDocument, setLocalAst }: NodeArgs<Cu const setSlot = makeSlotSetter(node, setLocalAst); const ctx = { onNavigateToDocument }; - const iconContainer = icon ? ( + // Q1-parity (callouts.lua:254-262): icon container is ALWAYS + // emitted. When `icon=false`, the inner <i> carries an + // additional `no-icon` co-class which CSS uses to hide it + // visually via `.callout.no-icon`. Keeps DOM counts of + // `.callout-icon-container` stable across icon/no-icon callouts. + const iconContainer = ( <div className={CALLOUT_ICON_CONTAINER}> - <i className={CALLOUT_ICON}></i> + <i className={icon ? CALLOUT_ICON : `${CALLOUT_ICON} ${NO_ICON}`}></i> </div> - ) : null; + ); const bodyContents = renderSlot(contentSlot, setSlot('content'), ctx); if (isTitled) { - const titleNode = userTitled + // Accessibility (callouts.lua:271-275): when the title is + // user-supplied AND the callout isn't crossref-eligible, + // prepend a screen-reader-only Span carrying the type's + // display name so screen readers announce e.g. "Warning: + // Watch Out" instead of just "Watch Out". Default-injected + // titles already ARE the display name; crossref-rendered + // titles already announce the type via the rendered prefix. + const isCrossref = !!plain.ref_type; + const userTitleNode = userTitled ? renderSlot(titleSlot, setSlot('title'), ctx) : defaultTitle(calloutType); + const titleNode = (userTitled && !isCrossref) ? ( + <> + <span className={SCREEN_READER_ONLY}>{defaultTitle(calloutType)}</span> + {userTitleNode} + </> + ) : userTitleNode; const headerClass = [CALLOUT_HEADER, BS_D_FLEX, BS_ALIGN_CONTENT_CENTER].join(' '); const bodyContainerClass = [CALLOUT_BODY_CONTAINER, CALLOUT_BODY].join(' '); @@ -160,8 +197,11 @@ export const Callout = ({ node, onNavigateToDocument, setLocalAst }: NodeArgs<Cu </div> ); const wrappedBody = collapse ? ( + // id="callout-N" + class="callout-N-contents callout-collapse + // collapse [show]" — mirrors Rust resolver and TS Quarto. <div - className={[CALLOUT_COLLAPSE, BS_COLLAPSE, !startsCollapsed ? BS_SHOW : null] + id={calloutId} + className={[contentsClass, CALLOUT_COLLAPSE, BS_COLLAPSE, !startsCollapsed ? BS_SHOW : null] .filter(Boolean) .join(' ')} > diff --git a/ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts b/ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts index c0b5d6dfd..f5d292fee 100644 --- a/ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts +++ b/ts-packages/preview-renderer/src/q2-preview/quartoClasses.ts @@ -87,3 +87,9 @@ export const PROOF = 'proof'; // CrossrefResolvedRef — crates/quarto-core/src/transforms/crossref_render.rs:707 export const QUARTO_XREF = 'quarto-xref'; + +// Accessibility — TS Quarto prepends a `<span class="screen-reader-only">` +// to a titled callout's title inlines so screen readers announce the +// callout type ("Note", "Warning", …) even though the visible title is +// user-supplied. callouts.lua:271-275 / callout_resolve.rs (mirror). +export const SCREEN_READER_ONLY = 'screen-reader-only';