Skip to content

Nested + foldable Obsidian callouts#652

Merged
srid merged 6 commits intomasterfrom
dizzy-king
Apr 25, 2026
Merged

Nested + foldable Obsidian callouts#652
srid merged 6 commits intomasterfrom
dizzy-king

Conversation

@srid
Copy link
Copy Markdown
Owner

@srid srid commented Apr 25, 2026

The remaining two items in #465 are now in. > [!type]+ and > [!type]- give you foldable callouts (initially expanded and collapsed, respectively); nested callouts work by indenting an inner blockquote with > >. Closes the last two unchecked boxes on the issue.

Foldable callouts render as native <details>/<summary> so the toggle works without JavaScript, and the chevron rotates via a CSS [open] selector. Nested callouts mostly came for free — the recursive pandocSplice already re-applies the block renderer to inner blocks, so the parser only needed to keep the inner BlockQuote intact in Callout.body. The visible "feature" there is really the docs section plus a CSS rule that drops the trailing margin on the last inner callout so it hugs the parent body.

The header parser is split into parseCalloutHeader :: Text -> Maybe (CalloutType, Maybe FoldState) returning both pieces; parseCalloutType is preserved as a back-compat shim for the existing tests. A defensive absorbFoldSuffix merges adjacent Str "+"/Str "-" tokens onto the header in case Pandoc tokenises [!tip]+ as two Str nodes; the comment spells out why we deliberately do not absorb across Space.

Reviewed by /hickey and /lowy. One comment-clarification fix landed (commit 007c722c) and one CSS dead-selector fix from /code-police's fact-check pass (e3497610). Two structural items deferred — see the Hickey/Lowy Analysis PR comment for the disposition table.

Deferred: the three-branch template duplicates the <details> wrapper between expanded/collapsed (Heist doesn't expose conditional attribute presence cleanly without losing access to template-level color/icon binds), and the splice/template split for fold state isn't compiler-enforced for exhaustiveness. Both are quality-of-life refactors — neither is structurally wrong.

Try it locally

nix run github:srid/emanote/dizzy-king -- -L docs run

Then visit /callout to see the new Foldable callouts and Nested callouts sections render.

srid added 4 commits April 24, 2026 19:38
Extend the callout renderer to cover the two remaining items in #465:

- Nested callouts: an outer `> [!type]` whose body contains a `> > [!type]`
  blockquote now renders as a nested `<div data-callout=...>` inside the
  outer's `.callout-content`. The recursive `pandocSplice` already does the
  work; the parser change is only that `Callout.body` retains the inner
  BlockQuote untouched, and CSS drops the trailing margin so the inner
  callout hugs the parent body.
- Foldable callouts: `> [!type]+` (initially expanded) and `> [!type]-`
  (initially collapsed) per the Obsidian spec. The header parser is split
  into `parseCalloutHeader` returning `(CalloutType, Maybe FoldState)`;
  `parseCalloutType` is kept as a back-compat shim. `Callout` gains a
  `foldState` field, and three conditional Heist splices in `_base.tpl`
  switch between `<div>` (non-foldable), `<details open>` (expanded), and
  `<details>` (collapsed) — all with shared icon/body bindings.

Tests cover bare/foldable header parsing in both same-token and split-token
forms (defensive against pandoc's tokenisation), nested-blockquote body
preservation, and rejection of malformed headers.

Docs: callout.md gains a Foldable and Nested section; CHANGELOG notes the
feature under 1.6.0.0 unreleased.
Expand the comment on `absorbFoldSuffix` to spell out that fold markers
must be adjacent to the closing bracket: `[Str "[!tip]", Space, Str "-"]`
is intentionally not absorbed, since `> [!tip] -` is a tip callout whose
title starts with a hyphen, not a foldable callout.

Per Lowy review of #465.
The two-selector rule overspecified the foldable case: `.callout details
> div > .callout:last-child` doesn't match the actual DOM (foldable body
is `<details> > <div.mt-3> > <div.callout-content> > nested .callout`,
not `details > div > .callout`). The first selector already covers both
foldable and non-foldable parents since both wrap their body in
`.callout-content`.
…ocal helper

The upstream `Heist.Splices.ifISplice :: Monad m => Bool -> Splice m`
has the same shape and semantics as the local `whenSplice` (run children
if condition is True, disappear otherwise). Drop the local helper and
import the upstream one — one less inline definition to maintain.
@srid
Copy link
Copy Markdown
Owner Author

srid commented Apr 25, 2026

Hickey/Lowy Analysis

# Lens Finding Disposition
1 Hickey Unvalidated callout type can crash via lookupHtmlTemplateMust No-op — predates this PR; an allowlist would break the documented custom-callout extension point (#573)
2 Hickey absorbFoldSuffix doesn't handle Space, Str "-" form No-op — absorbing across Space would mis-parse > [!tip] - (a tip with a hyphen-prefixed title) as foldable; comment now spells this out
3 Hickey Three template branches duplicate ~90% of the wrapper structure Deferred — Heist doesn't support conditional attribute presence in ${...} interpolation, and a Haskell-side wrapper splice loses access to template-level color/icon binds
4 Hickey FoldState exhaustiveness isn't enforced across Haskell+template Deferred — same shape as #3; would require consolidating the three-way dispatch into a single Haskell case
5 Lowy absorbFoldSuffix encoded an undocumented Pandoc tokenisation assumption Fixed in this PR (007c722)
6 Lowy Maybe FoldState slightly indirect vs. flat ADT No-op — correct in context; flattening is a stylistic preference, not a boundary issue
7 Lowy callout:fold-state data attribute as a customiser hook No-op — Lowy endorsed this as the right seam; data-callout-fold is a stable string handle that degrades gracefully if a new state is added
8 Lowy Nested-callout CSS selector tightly coupled to DOM shape Fixed in this PR (e349761) — dead second selector removed; the surviving one matches both foldable and non-foldable parents
9 Lowy Custom callout types extension point unaffected by changes No-op — type templates just <apply template="_base">, so the new conditionals inherit automatically

Hickey rationale

The three-branch template — <div> for non-foldable, <details open> for foldable+expanded, <details> for foldable+collapsed — is structurally justified: these are three distinct HTML elements with different semantics. The duplication cost is the price of branching at the template layer rather than at the Haskell layer; the alternative (always-<details> plus CSS to disable the toggle) would complect the DOM with a presentational workaround and obscure the accessibility intent. The shared content (callout-icon-svg, callout-fold-marker, callout-body-block) is already factored via Heist <bind>. Pushing dedup further is a quality-of-life cleanup rather than a structural correction.

Lowy rationale

Three volatility axes are encapsulated independently: parsing (parseCalloutHeader + absorbFoldSuffix), render orchestration (the three conditional Heist splices), and presentation (CSS in styles.tpl). Each can change without ripple — if Obsidian extends the fold-suffix grammar, only the parser changes; if a new fold state is added, only the orchestration plus a CSS rule. The custom-callout extension point (#573) is unaffected because individual type templates delegate to _base.tpl without knowing about fold state. The callout:fold-state data attribute is the right customiser seam — a stable string handle that downstream CSS/JS can hook into without touching Haskell internals.

@srid
Copy link
Copy Markdown
Owner Author

srid commented Apr 25, 2026

/do results

Step Status Duration Verification
sync 0s git fetch ok; forge=github
research 6m 10s Mapped callout implementation; identified 4 files to change
branch 16s On feature branch dizzy-king
implement 9m 20s Parser + Callout.foldState + 3-branch template + CSS; nested verified end-to-end via emanote gen
check 5s cabal build all clean
docs 27s callout.md + CHANGELOG.md updated
fmt 30s cabal-fmt, fourmolu, hlint, nixpkgs-fmt all clean
commit 31s 887a62b (6 files, +219/-26) pushed
hickey+lowy 6m 15s Lowy 1 fixed (007c722); Hickey 3+4 deferred; rest no-op
police 17m 31s Fact-check: dead CSS selector fixed (e349761). Elegance: whenSpliceHeist.Splices.ifISplice (0031e5b)
test 30s cabal test: 32 examples 0 failures (incl. 9 new callout parser tests)
create-pr 1m 50s Draft PR #652 + Hickey/Lowy disposition table comment
ci 4m 5s e2e (live) 4m20s, e2e (static) 4m11s, flake-parts-docs 1m49s — all green on HEAD 0031e5b0
done 0s Workflow complete
Total 48m 3s

Optimization suggestions

  • police dominates at 36% of total (17m 31s). The bulk was the /simplify Phase 2 spawning three reviewer agents in parallel, then code-search inside each — for a diff this small, a single inline pass would have been faster than the multi-agent fan-out.
  • research (6m 10s) and hickey+lowy (6m 15s) ran similar lengths. Combined they're 26%. Pre-reading Emanote/Pandoc/Renderer/Callout.hs and the _base.tpl callout template before invoking /do would shave the research step substantially.
  • implement (9m 20s) included one fmt-induced rebuild and an end-to-end render verification via emanote gen. The render check was worth the time (caught zero issues here, but would have caught template-XML errors immediately).
  • CI (4m 5s) was the unavoidable e2e wait. For follow-up commits to this PR, --from ci-only skips everything else and just re-runs CI against the new HEAD.

Workflow completed at 2026-04-25T00:25:33Z.

Adds Cucumber+Playwright scenarios for each user-facing path the
renderer must distinguish: plain (non-foldable `<div>`), foldable+
expanded (`<details open>` with body visible), foldable+collapsed
(`<details>` with body hidden), click-to-toggle, and nested. Each
top-level callout in the fixture uses a unique type so the steps
can target it via `[data-callout="<type>"]` without ambiguity.

The visibility check uses `getBoundingClientRect().height` rather
than `getComputedStyle(display)` because `<details>` hides its
descendants via internal layout — children keep their own `display`
value, so a height probe is the only reliable signal.

CHANGELOG entry now also links to PR #652.
@srid srid marked this pull request as ready for review April 25, 2026 00:32
Drop the e2e additions per request — unit tests in CalloutSpec.hs
already cover the parser cases (basic, foldable+/-, defensive split-
token absorption, nested-blockquote body preservation). The CHANGELOG
PR link added in 230acfe is preserved.

This reverts the test-only portion of 230acfe.
@srid srid merged commit 31ef8a0 into master Apr 25, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant