Caption fit: reserve caption height, respect author positioning - #50
Conversation
The fit pass scaled the image to the full cell height, so the caption always overflowed below the cell (typically onto the footer). Reserve the caption's height plus the figure gap when computing scale, and skip figures the author has absolutely positioned so deck CSS can take over layout without fighting inline styles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 0.2.3 caption-fit pass scales figure images to the full cell height, overflowing the caption onto the footer, and stomps author positioning with inline styles. Track natolambert/colloquium#50 until 0.2.4 ships. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50c2276e9e
ℹ️ 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".
| var captionHeight = caption.offsetHeight || 0; | ||
| var figureStyle = window.getComputedStyle(figure); | ||
| var figureGap = parseFloat(figureStyle.rowGap || figureStyle.gap) || 0; | ||
| var availableHeight = container.clientHeight - captionHeight - figureGap; |
There was a problem hiding this comment.
For captioned figures using the default theme, figcaption has margin-top: 0.45em in colloquium/themes/default/theme.css:234-235, but caption.offsetHeight excludes margins. When the image is height-constrained, this computes the image height as container - caption content, then flex layout adds the caption's top margin afterward, so the figure can still exceed its cell and overlap the footer by the margin amount. Please include the computed caption margins in the reserved height, or measure the full caption block spacing another way.
Useful? React with 👍 / 👎.
Opt-in slide class: on a columns slide, the right column's captioned figure escapes the grid and runs from the slide top to the content bottom with the caption pinned beneath. Documented in the README, with tests; default layouts unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The full-height column-figure layout now ships in colloquium (natolambert/colloquium#50), so the deck-local CSS is removed and the pin advances to the commit that includes the utility and caption-fit fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a905a5b1c3
ℹ️ 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".
| top: 0; | ||
| left: 0; | ||
| right: 0; | ||
| height: calc(100% - 1.5em); |
There was a problem hiding this comment.
When a class: img-tall-right columns slide is printed or exported, this screen-only height is overridden by the existing @media print fallback later in this file for grid figures, which forces the same image to height: auto !important and restores a capped max-height. The figure remains absolutely positioned, but the image no longer fills the slide-height area, so PDF/print output diverges from the new utility; add a matching print override or exempt .img-tall-right from that fallback.
AGENTS.md reference: AGENTS.md:L47-L50
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added a print-media override for img-tall-right after the grid-figure fallback so export matches screen (width 55%, height calc(100% - 1.5em), caps lifted). — Claude
| | `<!-- class: no-figure-captions -->` | Disable deck-wide figure captions for a specific slide | | ||
| | `<!-- img-align: center -->` | Align images only (`left`, `center`, `right`) — title unaffected | | ||
| | `<!-- img-valign: top -->` | Vertically align standalone images in grid/row cells (`top`, `center`, `bottom`) | | ||
| | `<!-- class: img-tall-right -->` | Columns slides: the right column's captioned figure runs full slide height, caption beneath | |
There was a problem hiding this comment.
With the directive as documented here, the right-column markdown image is still rendered as a plain <p><img> unless the deck also enables figure captions via figure_captions: true or the figure-captions class. The new CSS and fit helper only target figure.colloquium-figure, so class: img-tall-right by itself is a no-op for the common markdown-image case; either document the required companion caption setting or make this class support bare image paragraphs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Documented the figure-captions requirement in the README table entry. — Claude
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per review: the utility targets figure.colloquium-figure so it needs figure captions enabled (now documented), and the print grid-figure fallback overrode its sizing in export (now exempted with a matching print rule). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f57e51aeff
ℹ️ 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".
| // height (plus the figure's own gap) so the scaled image + caption | ||
| // fit the cell together instead of the caption overflowing onto | ||
| // whatever sits below (usually the footer). | ||
| var captionHeight = caption.offsetHeight || 0; |
There was a problem hiding this comment.
In slides where a height-constrained figure gets narrower than its parent and the caption text is long enough to wrap at that narrower width, caption.offsetHeight is captured before the function assigns the final figure.style.width. The caption can therefore gain another line after sizing, so the reserved availableHeight is too large and the caption still spills into the footer; measure after applying the final width or iterate the fit.
Useful? React with 👍 / 👎.
Resolve the changelog conflict with #50 and stop the print fallback card from inheriting the iframe's full reserved height -- it now hugs its label, title, and link. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
The captioned-figure fit pass scales the image to the full container height, so the caption always overflows below its cell — in practice landing on the slide footer (seen on rlhf-book lec13 rows/columns figure slides). It also unconditionally writes inline sizes, fighting any deck CSS that positions figures deliberately.
Changes
offsetHeight(plus the figure's flex gap) when computing the fit scale, so image + caption fit the cell together.positionisabsolute, letting deck-level utilities (e.g. full-height column figures) own layout without!importantwars.Tests: 248 passed.
🤖 Generated with Claude Code