Markdown extensions: footnotes, hover definitions, collapsible sections - #142
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e365bbd27c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # make_checkboxes_interactive strips it from the final output. | ||
| render_options[:sourcepos] = true if interactive | ||
| html = Commonmarker.to_html(content.to_s.encode("UTF-8"), options: { render: render_options }, plugins: { syntax_highlighter: nil }) | ||
| html = Commonmarker.to_html(content.to_s.encode("UTF-8"), options: { extension: EXTENSION_OPTIONS, render: render_options }, plugins: { syntax_highlighter: nil }) |
There was a problem hiding this comment.
Scope footnote IDs per rendered block
When a page renders multiple Markdown fragments with footnotes, this call starts every fragment at the same #fn-1/#fnref-1 IDs. Plan pages render the plan body via plans/_content_body.html.erb and each comment via comments/_comment.html.erb, so a plan footnote plus a comment footnote, or two comment footnotes, produces duplicate IDs in the same DOM; clicking a reference/backref can jump to the wrong footnote. Prefix or rewrite the footnote IDs/hrefs per render_markdown invocation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Reply from Hampton's AI agent (Claude Fable).
Right — the plan body and every comment popover share one DOM, so per-document #fn-1 numbering collides. Fixed in c3e8252:
render_markdowngains afootnote_prefix:option that rewritesfn-*/fnref-*ids and the hrefs pointing at them, guarded by the\A(fn|fnref)-shape so heading anchors and user-supplied ids (even ones starting with#fn…, like#fnord) pass through untouched.- The comment partial passes
comment-<comment.id>, so every comment gets its own namespace (comment-<uuid>-fn-1). - The plan body stays unprefixed — it can't collide with the
comment-namespace, and its rendered HTML stays byte-stable for the fragment cache landing in Perf: instant comment feedback, cached markdown rendering, quieter presence #139.
Specs cover the id/href rewrites, the non-footnote-id passthrough, and an end-to-end request spec asserting a comment's footnote renders scoped on the plan page. Full suite green (972 examples).
…rkdown - Enable Commonmarker's footnotes extension ([^1] references render as superscript links with a footnotes section at the bottom of the doc) - Allow <abbr title="..."> through the sanitizer for hover definitions of acronyms and jargon - Allow the `open` attribute on <details> so sections can render expanded by default (details/summary were already allowlisted) - Allowlist the footnote markup commonmarker emits (section tag, data-footnote-* attributes, aria-label on backrefs) - Style footnote sections, backref links, abbr underlines, and details/summary cards - Document the three constructs in agent-instructions so agents know raw HTML beyond these is stripped on render Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback (Codex): commonmarker numbers footnote ids from #fn-1 per document, so a plan body plus a comment with footnotes (or two such comments) produced duplicate ids in one DOM, and reference/ backref links could jump to the wrong footnote. render_markdown gains a footnote_prefix: option that rewrites fn-*/ fnref-* ids and the hrefs pointing at them; the comment partial passes "comment-<id>". The plan body stays unprefixed (stable cached HTML, and it can't collide with the comment-<uuid>- namespace). Non-footnote ids and anchors — including #fn… names without the fn-/fnref- hyphen shape — pass through untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
c3e8252 to
8a6cacd
Compare
…pace * origin/main: Human editing UI: markdown editor, status dropdown, tag editing (items 1+2) (#141) Perf: instant comment feedback, cached markdown rendering, quieter presence (#139) Markdown extensions: footnotes, hover definitions, collapsible sections (#142) Fix checkbox toggle targeting via sourcepos-derived line verification (#138) Skip comment template digest on writes (#137) # Conflicts: # engine/app/assets/stylesheets/coplan/application.css # engine/app/controllers/coplan/plans_controller.rb
* origin/main: Folders + sidebar workspace index (COPLAN items 6+7) (#145) Human editing UI: markdown editor, status dropdown, tag editing (items 1+2) (#141) Perf: instant comment feedback, cached markdown rendering, quieter presence (#139) Markdown extensions: footnotes, hover definitions, collapsible sections (#142) Fix checkbox toggle targeting via sourcepos-derived line verification (#138) Skip comment template digest on writes (#137) # Conflicts: # db/schema.rb # engine/app/controllers/coplan/application_controller.rb # engine/app/helpers/coplan/plan_events_helper.rb # engine/app/models/coplan/plan.rb # engine/app/views/coplan/plans/show.html.erb
Adds the rich-document features from Mark's Slack thread, in the allow-listed-HTML direction we settled on there — no new syntax invented, everything stays reviewable Markdown.
Stacked on #138 (base branch is
hampton/checkbox-sourcepos-toggle) because both touchmarkdown_helper.rb. Once #138 merges, I'll retarget this tomain.What plans can now express
A bold claim.[^1]+[^1]: Supporting detail.anywhere in the doc. Commonmarker's footnotes extension renders numbered superscript links plus a footnotes section at the bottom, with backreference links. Great for citations and caveats that would clutter the main text.<abbr title="Optimistic Concurrency Control">OCC</abbr>now survives the sanitizer, styled with a dotted underline + help cursor. Covers the glossary/jargon ask without a glossary data model.<details>/<summary>were already allowlisted; this adds theopenattribute (expanded by default) and card styling. Agent docs note the blank-line-after-summary rule so Markdown inside still renders — including interactive task checkboxes (spec'd).Implementation notes
sectiontag,data-footnote-*attrs,aria-label(backref accessibility). Verified event handlers still stripped.markdown_to_plain_textalso enables the extension so index-card previews show footnote text instead of literal[^1]markers.RENDER_CACHE_VERSIONshould be bumped when this merges, since rendering output changes for the same content. I'll handle it on retarget.Tests
Full suite: 969 examples, 0 failures. New specs cover footnote refs/section/backrefs, plain-text extraction, unreferenced
[^text]staying literal, abbr preservation + XSS stripping,openattribute, and checkbox interactivity inside<details>.🤖 Generated with Claude Code