Skip to content

Put each code fold inside its language tab - #34

Merged
d-morrison merged 2 commits into
mainfrom
claude/project-thread-sswd45
Oct 31, 2026
Merged

d-morrison merged 2 commits into
mainfrom
claude/project-thread-sswd45

Conversation

@d-morrison

Copy link
Copy Markdown
Contributor

Requested by Ezra · project thread

Quarto hoists a cross-referenced figure's details.code-fold blocks above the figure, so the R and Python folds stacked outside the tabset (example: https://morrison-lab.github.io/mds/optimization-convexity.html#fig-convex-set).

This adds tabset-code-in-tabs.html, an after-body script that moves each fold into the tab for its language, matching by language and falling back to order. It skips a tabset unless the folds before it equal its tab count. .gitignore gets an exception for the new file. The fold labels here already say "Show code".

Checked with jsdom on the published mds page; the PR preview should confirm it in a real render.


Generated by Claude Code

Quarto hoists a figure's code folds above the figure, so R and Python
folds stacked outside the tabset. A small after-body script moves each
fold into the tab for its language, and the fold label no longer says R.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xfc9pT9FcXoVbp61GC7NJ
Copilot AI balanced review requested due to automatic review settings October 30, 2026 20:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@d-morrison d-morrison self-assigned this Oct 30, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

The slide tabsets use tabby markup, so the script now selects panes and
tabs without Bootstrap class names, and a fold with no language match
takes the first unused tab.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xfc9pT9FcXoVbp61GC7NJ
Copilot AI balanced review requested due to automatic review settings October 30, 2026 20:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@github-actions

Copy link
Copy Markdown

Claude finished review — View run


Commit a6262ed (reviewed as the range 3953bc7..a6262ed) adds one suggestion about the new slide selectors. I read the diff and made no render; Quarto isn't installed in this run. The earlier verdict on 3953bc79 is here: Ready for merge, no blocking findings.

Findings

tabset-code-in-tabs.html:11, suggestion (not blocking, unverified). The commit's goal is to make the fold move work on slides too. The new pane selector is :scope > .tab-content > div. As I recall, Quarto's reveal.js (tabby) tabsets put the pane divs directly under div.panel-tabset, beside the ul, with no .tab-content wrapper. If so, panes is empty on slides. The script then returns at the folds.length !== panes.length check and does nothing, with no error, so slides would keep the stacked folds. The website path is unaffected.

I couldn't confirm either way without a render. Before relying on the claim in the comment on line 7, render one slide deck and inspect the tabset DOM. If the panes are direct children, use a selector that matches both layouts. For example, take :scope > .tab-content > div when that wrapper exists, and otherwise :scope > div.

tabset-code-in-tabs.html:34-38, minor. unused() returns -1 when every tab is already used. The length checks above make that unreachable today. A later edit that relaxes them would make panes[-1].insertBefore throw. A one-line guard would avoid that. The new unused() fix does correct the earlier index-collision issue.

Checked

  • _quarto-website.yml:206 adds include-after-body under the revealjs format, and the file path matches.
  • No page paths, anchors, math or prose changed.

Verdict

Ready for merge. Nothing here blocks the merge. The line 11 selector is worth a render check, though.

Structured Review Data (JSON)
{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "a6262ed4a7a5d2028be598d222f2a4bb9ff6a287",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "In tabset-code-in-tabs.html the slide pane selector may not match tabby's DOM, which would make the script a silent no-op on slides; unverified, and the unused() helper can return -1 only if the length guards are relaxed.",
  "holistic_assessment": "The change is small and scoped, the include-after-body wiring for revealjs is correct, and no page paths or anchors change; validation was by reading only because Quarto was unavailable to render."
}

Reviewed commit: a6262ed

@github-actions

Copy link
Copy Markdown

💰 Cost: $0.2101 (review) — run

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.

2026-10-31 00:28 UTC

@d-morrison
d-morrison merged commit 8f64002 into main Oct 31, 2026
23 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.

3 participants