callout: align class vocabulary with TS Quarto / Bootstrap#236
Open
gordonwoodhull wants to merge 2 commits into
Open
callout: align class vocabulary with TS Quarto / Bootstrap#236gordonwoodhull wants to merge 2 commits into
gordonwoodhull wants to merge 2 commits into
Conversation
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
<toggle button>? (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
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
<span class="screen-reader-only">{DisplayName}</span>
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 `<i>`
(callouts.lua:254-262)
Previously, q2 omitted the `<div class="callout-icon-container">`
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 `<i class="callout-icon">` 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 `<id>` + `<id>-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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Callouts were constructed correctly but had incorrect Bootstrap classes.
Copy the behavior from Quarto 1. Add various refinements also found in Quarto 1.
Output is identical in HTML and q2-preview.
Summary
CalloutResolveTransformand the q2-preview React Callout component with the canonical TS Quarto / Bootstrap class vocabulary (callout-style-{default,simple},callout-titled,no-icon,callout-empty-content,d-flex align-content-center,callout-collapse + callout-N-contents) so the vendored Bootstrap SCSS rules apply — callouts now render styled in bothquarto renderHTML output and the hub-client preview.~/src/quarto-cli/src/resources/filters/modules/callouts.lua: screen-reader callout-type span (a11y), icon container always emitted (.no-iconon<i>instead of omitting the div), and split collapse-wrapper id / class naming (id="callout-N"+class="callout-N-contents").Background
q2's resolver had been emitting a non-canonical class scheme (
callout-appearance-{x}only when non-default; nocallout-titled/no-icon/callout-empty-content; collapse as a single class on the outer). The vendored Bootstrap SCSS rules atresources/scss/bootstrap/_bootstrap-rules.scssare gated on the TS Quarto class vocabulary — so almost no rules ever applied, andformat: htmlrendered callouts as unstyled divs.The fix touches three surfaces in lock-step:
crates/quarto-core/src/transforms/callout.rs,callout_resolve.rs) — full rewrite ofresolve_calloutwith titled/untitled paths, appearance-conditional default-title injection, minimal-normalization, collapse-id threading, and the three Q1-parity refinements.ts-packages/preview-renderer/src/q2-preview/custom/Callout.tsx+quartoClasses.ts) — mirrors the resolver output for the React-iframe preview path.theme: noneCSS (crates/quarto-core/resources/styles.css) — rewritten against the canonical selectors so documents without an explicit theme also render correctly.Two commits land the work:
Follow-up beads issues filed:
@tip-foocrossref of callouts still works after the alignment (P1).findDoc(indexDocId)5s timeout on cold boot (P2; unrelated to callouts but blocked q2-preview visual verification during this work).Plan:
claude-notes/plans/2026-05-22-callout-class-vocabulary-fix.md.Test plan
cargo nextest run --workspace— 9444/9444 pass, 0 failures.cargo nextest run -p quarto-core callout_resolve— 25/25 (7 pre-existing + 13 canonical-vocabulary + 5 Q1-parity).cargo nextest run -p quarto --test smoke_all— green; includes the newcallouts-matrix.qmdfixture.vitest run --config vitest.integration.config.ts custom-components— 53/53 pass.cargo xtask lint --quiet— clean.cargo run --bin q2 -- render crates/quarto/tests/smoke-all/quarto-test/callouts-matrix.qmdproduces HTML with the full canonical class vocabulary; matching selectors confirmed in the bundled Bootstrap stylesheet; styling correct in the browser.q2 previewSPA route renders callouts with the React component + theme-CSS blob URL (after the full WASM → SPA → q2 binary rebuild chain).