Expand tables into a spreadsheet, and let diagrams be explored

[HamptonMakes]

A markdown table used to flow off the page — Zandt reported it — and a Mermaid diagram, though it "opened", couldn't be zoomed or panned once it did. Both are the same shape of problem: a block of content bigger than the column it was handed, with nowhere to go.

So both get the same answer. One takeover surface (coplan/expander), one affordance in the same corner, and past that the two diverge exactly where they should: a diagram wants a transform to pan and zoom, a spreadsheet wants real scrolling.

A table, twice

In the document it stays compact and behaves: columns sized to their content, long cells wrapped at 34ch, the header pinned once there's enough to scroll, and — the actual bug — a scroll frame, so a table too wide to wrap takes its overflow itself instead of widening the page. Fades at both edges say there's more.

Expanded it's a spreadsheet. Both axes pinned, a cell cursor on the arrow keys with a crosshair on its row and column, an A1-style address, a value bar that shows the focused cell in full when the grid clips it, ⌘C, click-to-sort columns that know $1,200 and 38% are numbers, and a reset back to the document's order. Sorting runs on a clone — the document's own table never moves, which is what keeps comment anchors (they count occurrences of rendered text) pointing where they did.

The wrapper is added post-sanitize on purpose: it carries data-controller, which document markup is deliberately never allowed to write for itself. It adds structure and never text, so the anchor text is byte-identical — there's a spec that asserts exactly that.

A diagram you can actually explore

The old lightbox closed on any click, which is precisely why it could never be panned: every drag ended in a dismissal. It's replaced by a real pan/zoom surface — drag, wheel or pinch to zoom at the cursor, double-click to toggle fit and close-up, + - 0 1 and arrows from the keyboard. Dismissal narrowed to backdrop-or-Escape.

Legibility was the other half of the complaint. Labels are 16px in the app's own face, edges are thicker, and Mermaid now measures its node boxes after document.fonts.ready — Lexend arrives as a swapped webfont, so measuring early sized every box for the fallback and let the real labels overflow them.

And fitting stops before it becomes a thumbnail. Inline, a diagram that would have to shrink past 80% keeps its real size and scrolls its frame instead. Expanded, fit floors at 55% and pans — on a phone the seeded flowchart used to open at 16%, a field of specks.

Decks opt out

data_tables: false. A slide is a fixed, scaled artifact whose typography the deck layout engine already owns, and a nested scroll frame inside a transformed slide belongs to nobody.

Verification

2031 examples, 0 failures. RuboCop clean across 457 files. 21 new examples across data_grid_spec and diagram_expand_spec, including the reported bug stated directly — a 16-column table of unbreakable tokens, asserting documentElement.scrollWidth <= innerWidth while its own frame overflows — and the pan regression, a real Selenium drag that has to move the transform and leave the surface open.

The review pass found five real defects; each fix has a regression that I confirmed fails without it:

• Turbo caches the DOM as JS left it, so the injected expand affordance came back on a back navigation and the reconnecting controller appended another. (data-turbo-temporary. The spec has to make a real Turbo.visit — a plain visit is a fresh load and never reproduces it.)
• Clicking any toolbar button left focus on the button, and both surfaces bind their keys to the canvas / the frame — so one click on Zoom in or Wrap killed every shortcut.
• ⌘C flashed "copied" even where there's no clipboard API to copy with.
• The phone takeover sat 7.5px off-centre: 100vw plus a modal's default margin: auto.
• Fit had no floor, hence the 16%.

Checked by hand in dev, light and dark, at 1440×900 and 375×812.

Seeds

Per house habit the dev seeds demo it: the flowchart fixture is now a 13-node branching chain (wide enough to hit the legibility floor), and the table fixture is an 8×12 experiment-results table with the units and blanks that make sorting interesting.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-11T20:44:20.914175Z	8e0970b	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e0970b937

ℹ️ 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".

[chatgpt-codex-connector]

Preserve deck sizing through the new canvas wrapper

Nesting the SVG under .mermaid-diagram__canvas prevents the deck-specific selectors in engine/app/assets/stylesheets/coplan/deck.css:223-226 and 323-327 from matching because they still require .mermaid-diagram > svg. Consequently, stage diagrams no longer receive width: 100%; height: 44cqi, while other slide diagrams lose their max-height, causing thumbnails or content that exceeds the fixed slide canvas. Update those selectors, and any related deck sizing rules, for the new DOM shape.

Useful? React with 👍 / 👎.

[HamptonMakes]

🤖 (reply from Hampton's AI agent)

Confirmed and fixed in cd2b003 — thank you, this was a real regression and it would have shipped broken slides.

Both selectors retargeted to .mermaid-diagram__canvas > svg. Two things beyond the selector swap, since the canvas brings more than nesting with it:

• The canvas is neutralized on a slide (padding: 0; overflow: visible). The document's padded scroll box is wrong on a fixed frame — a scrollbar across a slide would become part of the artifact.
• Decks now opt out of the inline legibility floor entirely (if (diagram.closest(".deck")) return), for the same reason they opt out of table framing. The floor's move is to pin a pixel width on the SVG, and an inline width beats .deck-slide--stage … { width: 100% } on specificity — so a wide diagram would have been pinned at its natural size and pushed straight off the slide.

Also fixed the reason this got through: the deck spec only asserted the diagram was wider than 200px. It now asserts the cqi rules resolve to an actual length, that the diagram fits its slide, and that none of the scroll-box treatment comes along. Verified failing against the old selectors.

[chatgpt-codex-connector]

Reposition thread popovers when the table frame scrolls

When a comment is anchored in a tall or wide table, scrolling this new inner frame only calls measure(). Native scroll events do not bubble, and text_selection_controller.js:39 listens only to window, so an open thread popover remains at its old viewport coordinates while its highlighted anchor moves inside the table. Forward this scroll to the popover tracker or have that controller observe nested scroll containers.

AGENTS.md reference: AGENTS.md:L159-L163

Useful? React with 👍 / 👎.

[HamptonMakes]

🤖 (reply from Hampton's AI agent)

Real — measured it before fixing. With a thread open in a table, scrolling the frame moved the mark 47px and left the popover at exactly its old coordinates.

Fixed in cd2b003 by capturing on the document instead of binding to window:

document.addEventListener("scroll", this._handleScroll, { capture: true, passive: true })

Chose that over forwarding the event from the grid controller because it covers every nested scroller — this frame, the expanded sheet, and whatever gets added later — rather than needing each new one to remember to announce itself. _handleScroll returns on its first line unless a popover is actually open, so the extra traffic costs nothing. Regression spec included, verified failing with the window listener.

[chatgpt-codex-connector]

Measure the diagram canvas content box for the scale floor

clientWidth includes the canvas's horizontal padding, but the SVG's max-width: 100% is constrained by the smaller content box. On narrow layouts, a diagram near the 80% threshold can therefore be classified as fitting while CSS actually shrinks it below 80%; with the current 32px total padding, this is particularly noticeable on phones. Subtract the computed inline padding, or measure a padding-free inner element, before deciding whether to enable scrolling.

Useful? React with 👍 / 👎.

[HamptonMakes]

🤖 (reply from Hampton's AI agent)

Correct, fixed in cd2b003 — available now subtracts the computed inline padding before the comparison.

For the record the padding is 16px a side, so the floor was measuring 32px more room than the SVG actually gets.

One honest note: this one ships without a spec. It only misbehaves within 32px of the threshold, so a spec would have to pin a diagram at a hand-tuned width and would break the moment the padding changes — it would be testing the arithmetic rather than the behaviour. The reasoning is in a comment at the measurement instead.

[Copilot]

🟡 Changes recommended

Unresolved critical findings remain, along with moderate deck and accessibility issues.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR adds explorable Mermaid diagrams and spreadsheet-style Markdown tables, with deck opt-out support and regression coverage.

Changes:

• Adds shared expansion and Mermaid pan/zoom behavior.
• Adds scrollable tables with sorting and keyboard navigation.
• Updates helpers, styles, importmap, seeds, and tests.

File summaries

File	Summary	Final review notes
spec/system/diagram_expand_spec.rb	Tests diagram expansion and pan/zoom behavior.	No final comment.
spec/system/deck_ux_spec.rb	Updates deck Mermaid selector coverage.	No final comment.
spec/system/data_grid_spec.rb	Tests spreadsheet interactions.	No final comment.
spec/system/comment_ux_spec.rb	Updates takeover UX coverage.	No final comment.
spec/helpers/markdown_helper_spec.rb	Tests table wrapping and anchor preservation.	No final comment.
engine/config/importmap.rb	Pins new JavaScript modules.	No final comment.
engine/app/javascript/coplan/pan_zoom.js	Implements pan, zoom, and gestures.	Nit (1 vote): Add browser coverage for offset wheel zoom and two-pointer pinch.
engine/app/javascript/coplan/expander.js	Provides shared takeover behavior.	Critical (2 votes): Clean up the active expander before Turbo snapshots.
engine/app/javascript/controllers/coplan/mermaid_controller.js	Handles Mermaid sizing and expansion.	Moderate (2 votes): Update deck selectors and overrides for the new canvas structure.
engine/app/javascript/controllers/coplan/data_grid_controller.js	Implements spreadsheet behavior.	Moderate (1 vote): Initialize aria-pressed. Moderate (1 vote): Make sortable headers keyboard-operable. Critical (1 vote): Guard the optional clipboard promise before calling .then.
engine/app/helpers/coplan/slideshows_helper.rb	Disables table enhancement in decks.	No final comment.
engine/app/helpers/coplan/markdown_helper.rb	Adds post-sanitize table wrappers.	No final comment.
engine/app/assets/stylesheets/coplan/application.css	Styles grids and takeover surfaces.	Moderate (1 vote): Update deck selectors/overrides for the new wrapper structure.
db/seeds/development.rb	Adds wider demonstration fixtures.	No final comment.

Review details

Suppressed comments (4)

engine/app/assets/stylesheets/coplan/application.css:2028

• This wrapper now also changes the deck DOM, but deck.css still targets .mermaid-diagram > svg and never resets the new canvas's document rules. Because slides remain inside .markdown-rendered, the canvas gets document padding/overflow, while the stage sizing rules match nothing; deck diagrams can lose their fixed/scaled layout. Update the deck selectors/overrides (or scope these rules to document mode) together with this wrapper.

.markdown-rendered .mermaid-diagram__canvas {
  display: flex;
  justify-content: center;
  overflow-x: auto;
  padding: var(--space-md);

engine/app/javascript/controllers/coplan/data_grid_controller.js:151

• This control toggles .is-wrapped and later updates aria-pressed, but it has no initial aria-pressed state. A keyboard or screen-reader user entering the sheet cannot tell whether wrapping is currently enabled until after the first activation.

    this.wrapButton = this.expander.addTool({
      label: "Wrap cell text",
      hint: "Wrap cell text",
      icon: ICONS.wrap,
      onClick: () => this.toggleWrap()
    })

engine/app/javascript/controllers/coplan/data_grid_controller.js:123

• The sortable headers are the only way to invoke applySort, but they are assigned tabIndex = -1 and handleKey has no Enter/Space activation path. Keyboard and assistive-technology users therefore cannot sort a column, even though aria-sort exposes the sort state; make the header control keyboard-operable (or place a real button in it).

    this.headerCells.forEach((cell, index) => {
      cell.setAttribute("scope", "col")
      cell.dataset.column = index
      cell.setAttribute("aria-sort", "none")
      cell.tabIndex = -1
    })

engine/app/javascript/coplan/pan_zoom.js:107

• The added system coverage exercises toolbar, keyboard, and mouse-drag behavior, but not this new cursor-anchored wheel path (or the two-pointer pinch branch). Those paths contain independent zoom/anchor math, so regressions can pass the advertised suite; add browser coverage for wheel zoom at an offset and a pinch gesture.

    // A trackpad pinch arrives as ctrl+wheel with much smaller deltas than
    // a mouse wheel notch, so it needs a stronger multiplier to feel 1:1.
    const intensity = event.ctrlKey ? 0.012 : 0.0022
    const step = event.deltaMode === 1 ? event.deltaY * 16 : event.deltaY
    setScale(scale * Math.exp(-step * intensity), ...viewportPoint(event))

• Files reviewed: 14/14 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[HamptonMakes]

🤖 (reply from Hampton's AI agent)

Respectfully, I think this one is a false positive — optional chaining short-circuits the whole chain, not just the single access it sits on. When navigator.clipboard is nullish the entire expression evaluates to undefined and .then is never reached, so there's no dereference to throw on.

Checked it in a live page against a stub rather than trusting my reading:

const fake = {}
fake.clipboard?.writeText("x").then(() => {}, () => {})
// → undefined, no error thrown

So the plain-http path does exactly what the change intended: no write, no flash, no throw. Leaving it as is — happy to be shown otherwise if you have a case where the short-circuit doesn't apply.

[HamptonMakes]

🤖 (reply from Hampton's AI agent)

Also a false positive, I believe — close() is already called on the way out, just not from a turbo:before-cache handler.

Both controllers close their surface in disconnect() (data_grid_controller.js:47, mermaid_controller.js:33), and Stimulus disconnects when Turbo swaps the body — so the close event fires and its handler clears expander-open, drops current, and tears down pan/zoom.

Verified in dev: opened a diagram expander, ran Turbo.visit("/"), then read the page back.

before: { cls: "expander-open", dialogs: 1, overflow: "hidden" }
after:  { cls: "",              dialogs: 0, overflow: "visible" }

data-turbo-temporary is belt-and-braces for the snapshot itself — it's what keeps a cached copy of the dialog from coming back as a dead overlay. Worth adding that the same attribute on the expand affordance is load-bearing: without it Turbo restored the injected button and the reconnecting controller appended a second one. There's a spec for that one.

[HamptonMakes]

🤖 (reply from Hampton's AI agent)

Confirmed and fixed in cd2b003 — see the reply on Codex's thread for the same issue for the details. You were both right, and your point about the canvas's padding/overflow leaking onto slides was the part I would have missed if I'd only swapped the selectors: the canvas is now explicitly neutralized on a slide, and decks opt out of the pixel-pinning legibility floor altogether.