Skip to content

Export movie cells, size the exported figure box by its shape, and fix three report defects - #158

Open
CSSFrancis wants to merge 10 commits into
mainfrom
fix/report-export-defects
Open

CSSFrancis wants to merge 10 commits into
mainfrom
fix/report-export-defects

Conversation

@CSSFrancis

Copy link
Copy Markdown
Member

Why

  • Going through the report layer for the embed cleanup turned up three real defects and one missing export branch. None of them need the new embed contract, so they go first, on their own.
  • A movie cell exported as nothing at all: no still, no caption, in static HTML, interactive HTML, the slides deck and the PDF. An unhandled cell type is indistinguishable from an empty report.
  • Exported figures all got the same 480 px box whatever their shape, so a wide 1×3 row was letterboxed and a square pattern was stretched. The sidebar cell already sizes by aspect ratio; the export now does the same.

The three defects

  • compose._finalize_edit was one of five copies of the per-cell teardown and the only one that never cleared the baked PNG, the offline flag and the photo bytes. There is now one ReportManager.drop_cell_resources, called from every per-cell site; _finalize_edit no longer leaks.
  • The sidebar's copy of figurePayloadFromDrop lacked the peekWindowDrag() fallback the other two copies had, so a real window drag onto the body between two cells could resolve to nothing. One exported function in dnd.ts now, used by all three callers.
  • The split cell's text pane was the markdown editor without its toolbar and without Ctrl-B / Ctrl-I. MarkdownPane is extracted from ReportCell and rendered in both.

Export changes

  • Movie cells export as their poster still with a play badge and their caption; in interactive mode a rendered GIF or MP4 is inlined when it fits.
  • EMBED_BUDGET_BYTES = 100 * 2**20 is the one budget for anything a report file inlines. Over it the cell exports as its still plus a note naming the size and the budget. The embed contract PR will move this constant to its home and reuse it.
  • A figure with a caption but no pixels keeps the caption above an empty framed box instead of vanishing.
  • The figure box is sized by aspect ratio like the sidebar cell, the iframe keeps the figure's natural size and is scaled into the box, and an embed that reports its own height overrides both. The slides deck gets the same box.
  • The renderer's aspect constants and the Python ones are pinned against each other by parsing the TSX, and the drift test runs the same helper over an edited copy so it actually proves the guard bites.

Lines

  • Part 1 (movie export, aspect box) adds about 290 production lines, nearly all in export_html.py. That is a missing feature, not duplication.
  • Part 2 (the three defects) is net about −85 production lines: five teardowns became one, three drop readers became one, two markdown editors became one.

Tests

  • Failing test first for each defect and for the movie export. Python: test_report_export.py (cell-kind coverage over model.CELL_TYPES, the figure box, the TSX guard, the embed height), test_report_compose.py (the teardown), test_report_present.py.
  • e2e: report_export_movie.spec.ts (still + badge in static, inline GIF in interactive, the box sized past a fixed height, a blender embed filling its box) and compose_real_drag.spec.ts (a real titlebar drag onto the sidebar gap lands a cell; Ctrl-B in the split pane wraps the selection).
  • Full Python suite 3476 passed; typecheck clean; the report specs 30 passed.

Changelog

  • Five one-sentence fragments under upcoming_changes/, one per user-visible change.

A movie cell exported as nothing at all, caption included, in static HTML,
interactive HTML, the slides deck and the PDF that renders from the static
file. Give it a branch: the poster still with a play badge everywhere, and the
rendered movie inlined when an interactive export has a file to inline.
EMBED_BUDGET_BYTES is the one ceiling on what a report file inlines; a movie
over it exports as its still plus a note naming the size and the budget.
A poster-less movie now counts as a dropped asset, so the save warns rather
than reporting clean.

A figure with no pixels keeps its caption above an empty framed box. Dropping
the whole figure deleted the caption with it, which reads as "the author wrote
nothing".

The interactive figure box is sized from a CSS aspect ratio the way the sidebar
cell is, rather than one fixed 480 px height that letterboxed a wide row and
stretched a square pattern. The ratios mirror ReportFigureCell.tsx and the test
parses that file rather than trusting a comment.

Claude-Session: https://claude.ai/code/session_01VdNjbxjn9vb1eMUxqsu5Kg
Five copies of the same teardown had drifted apart, and the copy that runs when
a compose edit empties a cell kept the baked PNG, the offline flag and any held
photo bytes. Those tables are keyed by cell id, so a placeholder cell went on
owning assets a later save would write.

ReportManager.drop_cell_resources is now the only one. It always drops the live
half (window, snapshots, edit mode, wiring, selection, memoized explorer page);
keep_displayable keeps what a cell that stays in the document is still shown
from, which is what an un-built figure window needs and nothing else does.
Deleting a cell no longer branches by type: every kind loses everything.

Claude-Session: https://claude.ai/code/session_01VdNjbxjn9vb1eMUxqsu5Kg
Two sidebar defects that share ReportSplitCell.tsx and one spec file, so they
land together.

The drop-payload reader had three copies and they had drifted. The sidebar
body's lacked the in-process stash the other two fall back to, so a real drag
whose payload arrives unreadable resolved no source window and the drop did
nothing at the one target a user aims at to ADD a cell. It now lives beside
the stash it reads, in dnd.ts, and all three targets call it.

A split block's text side was the markdown editor stripped of its formatting
toolbar and its Ctrl-B / Ctrl-I, so which editor you got depended on which kind
of cell you had double-clicked. MarkdownPane is the editor, rendered by a text
cell and by a split block's text side; testidPrefix keeps each host's testids.

Claude-Session: https://claude.ai/code/session_01VdNjbxjn9vb1eMUxqsu5Kg
An interactive embed now carries its own natural pixel size, which the deck's
stylesheet cannot override, so without the box and the fit script a large figure
ran off the slide.

Claude-Session: https://claude.ai/code/session_01VdNjbxjn9vb1eMUxqsu5Kg
The box clips what it holds, so an embed that posts no measured height and is
emitted with only a width collapsed to the browser's default 150 px: that is
the orientation explorer and the overlay blender. The rule both page skeletons
share now sets height:100%, and the fit script still skips these because it
reads the inline width and height a figure carrying its natural size has.

The deck's two iframe rules predate the box and no longer match anything, so
they go. The rules the two skeletons had byte-identical are one constant now;
only the colours stay per skeleton.

A placeholder movie card was counted as a dropped asset, so adding one and
saving warned about pixels nothing had ever been asked to render. Un-building a
figure keeps the cell's edit mode and panel selection, which only report_state
is allowed to change: the flag that says so is now figure_returns.

The TSX aspect guard anchors on the code around each ratio rather than on where
that code sits, so reflowing the component cannot fail it, and the drift test
runs the real patterns over an edited copy instead of re-typing one.

Claude-Session: https://claude.ai/code/session_01VdNjbxjn9vb1eMUxqsu5Kg
The comments were recording what the code used to do rather than why it does
what it does. `const window` shadowed the global in figurePayloadFromDrop; the
value it holds is a window id, so it is named one.

Claude-Session: https://claude.ai/code/session_01VdNjbxjn9vb1eMUxqsu5Kg
The contract is one sentence per file; this was six over two paragraphs.

Claude-Session: https://claude.ai/code/session_01VdNjbxjn9vb1eMUxqsu5Kg
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