#148 transition panel/card text parity: shared wrapping + one typeface#165
Conversation
Long transition text broke differently in the two consumers: the studio wrapped via CSS (`max-width` + `pre-wrap`) in Inter, while the export drew a single unwrapped Nunito line that overflowed/clipped the raster. Same text, different breaks and different face. `@toony/render/transition.ts` now single-sources panel/card TEXT wrapping + typeface (consuming #147's band background/floor/divider helpers read-only): - `layoutPanelText`/`layoutCardText` return measure-aware WRAPPED lines. They wrap `render.detail` within a shared `BAND_TEXT_MAX_WIDTH_FRAC` (0.84) using the deterministic default measurer, so export and studio break identically regardless of platform. A single/short line keeps its original position. - `BAND_FONT_ID` (curated Nunito) + `BAND_FONT_STACK` are the ONE shared band typeface decision, applied in both consumers. Consumers: - export `compose.ts`: `drawPanelText` draws the wrapped lines; `BAND_FONT_*` register via `BAND_FONT_ID` (no local "nunito" literal). `drawBandText` already loops the card lines. - studio `transition-block.tsx`: renders panel + card lines from the shared layout, sets `fontFamily: BAND_FONT_STACK`; `.transition-line` CSS drops `max-width`/`pre-wrap` (wrap + face now come from the layout, not CSS). Tests: render — panel/card wrap a long fixture into identical, word-preserving lines (deterministic across the two consumers), short text unchanged, shared Nunito typeface. studio (#157 harness) — the studio consumes the same wrapped layout + font stack. HARD BOUNDARY: #147 band helpers untouched. No new deps, no stubs. check + test green uncached Node 20.20.2 + 24.15.0 (render 87->90, studio 23->26, export 35 unchanged; repo-wide 489 pass / 0 fail). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES\n\n### Epic Alignment: FAIL\n#148 requires long transition-card text to remain a trustworthy Read/export layout; the shared wrap now produces an overflowing, overlapping legacy-card layout.\n\n### Checked (evidence)\n- Structural/UI gate: live PR body has filled , , and a table.\n- Ticket/contract: live issue #148 requires shared, measure-aware wrapped lines and Read/export parity, with #147 helpers read-only; live epic #1 requires trustworthy Read/export behavior.\n- Design-fidelity table: spot-checked all 5 rows against , , , and .\n- Riskiest part of this diff: legacy-card multiline vertical layout; it is not acceptable because the fixed type label collides with wrapped detail lines.\n- Kill-list: scanned changed runtime/test ranges — finding below.\n- CI: check + test (Node 20) pass 1m22s https://github.com/realproject7/toony/actions/runs/29355343613/job/87161447492
check + test (Node 24) pass 1m9s https://github.com/realproject7/toony/actions/runs/29355343613/job/87161447468 → Node 20 PASS; Node 24 PASS.\n\n### Findings\n- [blocker] Long legacy-card detail overlaps its type label and can render outside the panel.\n - File: \n - Why it fails: at the tested 800×300 size, the fixture wraps to five 82.5px lines. Centering that block at 0.42h places its first center at -39px and its fourth at 208.5px, while the fixed label stays at 204px. Both studio and export therefore share a visibly broken result rather than usable parity; the current tests only assert lines.\n - Do instead: lay out detail and label as one vertically bounded stack (or auto-fit/limit the detail) so every line remains within the panel and clears the label, then add a long-fixture test that asserts line bounds and no detail/label overlap.\n\n### Decision\nThe single-source wrap/typeface approach is aligned, and CI is green, but the long-text edge case required by #148 breaks the rendered card.
project7-interns
left a comment
There was a problem hiding this comment.
Review correction - complete evidence for REQUEST CHANGES
The prior review correctly set REQUEST CHANGES on commit 10857b4, but shell expansion stripped its Markdown references. This comment supplies the required intact evidence.
Checked (evidence)
- Structural/UI gate: live PR body has filled
## EPIC Alignment,## Self-Verification, and a## Design Fidelitytable. - Ticket/contract: live issue #148 requires shared, measure-aware wrapped lines and Read/export parity, with #147 helpers read-only; live epic #1 requires trustworthy Read/export behavior.
- Design-fidelity table: spot-checked all 5 rows against
packages/render/src/transition.ts:308-385,416-463,packages/export/src/compose.ts:311-315,apps/studio/src/components/transition-block.tsx:150-184, andapps/studio/src/app/studio.css:510-520. - Riskiest part: legacy-card multiline vertical layout; it is not acceptable because the fixed type label collides with wrapped detail lines.
- Kill-list: scanned changed runtime/test ranges - finding below.
- CI:
gh pr checks 165-> Node 20 PASS; Node 24 PASS.
Findings
- [blocker] Long legacy-card detail overlaps its type label and can render outside the panel.
- File:
packages/render/src/transition.ts:427-447 - Why it fails: at the tested 800x300 size, the fixture wraps to five 82.5px lines. Centering that block at 0.42h places its first center at -39px and its fourth at 208.5px, while the fixed label stays at 204px. Both studio and export therefore share a visibly broken result rather than usable parity; the current tests only assert
> 1lines. - Do instead: lay out detail and label as one vertically bounded stack (or auto-fit/limit the detail) so every line remains within the panel and clears the label, then add a long-fixture test that asserts line bounds and no detail/label overlap.
- File:
Decision
The single-source wrap/typeface approach is aligned, and CI is green, but the long-text edge case required by #148 breaks the rendered card.
@re1 review) @re1: a long wrapped legacy-card detail centered at 0.42h with the label fixed at 0.68h clipped (negative line y) and overlapped the label (at 800x300, detail centers ran to -39px vs the 204px label). Both `layoutPanelText` and `layoutCardText` now lay out via the existing auto-fit `layoutBubbleText` machinery: the text wraps to the shared max width AND shrinks its font (to a floor) so the block never exceeds the padded panel. For a card, the bold detail auto-fits the panel MINUS the label row + gap, and the detail + label form one vertically-centered, bounded stack — the label always sits below the wrapped detail. Short/single-line text still fits at the max font (unchanged), and the parity guarantee is intact (both consumers call the same pure layout with the default deterministic measurer). Tests: bounds coverage (every line stays within [0, height]) + non-overlap (label below the wrapped detail) for a long card and a long panel; updated the card geometry tests to the bounded-stack relations. render 90 -> 92; check + test green uncached Node 20.20.2 + 24.15.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Epic Alignment: FAIL
The #148 layout must keep long transition text trustworthy in Read and export; the new bounded-layout guarantee is not true for valid unbounded transition text.
Checked (evidence)
- Re-review delta:
10857b466cbe1536ce18d2e0c3e97819e86b2ce1..8b14f59f0c05c357534b8bd4c92646a4dee35205. - Prior finding: resolved for the reviewed fixture.
packages/render/src/transition.ts:439-470now creates one detail-plus-label stack;packages/render/src/__tests__/transition.test.ts:435-457checks bounds and label ordering. - Kill-list: scanned new runtime/test ranges - finding below.
- CI:
gh pr checks 165-> Node 24 PASS; Node 20 PENDING at review time.
Findings
- [blocker] The claimed no-clip guarantee still fails for sufficiently long valid transition text.
- File:
packages/render/src/transition.ts:374-393,443-473 - Why it fails: both layouts discard
layoutBubbleText(...).overflow. That helper explicitly returns best-effort overflowing lines once text does not fit at its minimum font (packages/render/src/text.ts:166-173), andTransition.textis an unboundedstring | null(packages/schema/src/types.ts:402-410). Thus a sufficiently long input again makesblockHeight/detailHeightexceed the reserved panel area, despite the new comments and PR table saying it never clips. The new tests cover only the moderate fixture. - Do instead: establish a real overflow policy: continue fitting until the text fits, or enforce a validated text limit with user-visible handling. Preserve the chosen policy in the shared layout for both consumers, and add an intentionally oversized fixture proving no line leaves the panel and the card label remains clear.
- File:
Decision
The original overlap is fixed, but the shared layout still fails the required long-text edge case for valid input.
|
[@re2 REVIEW — APPROVE] (marked comment; own-identity gh review blocked) Review of PR #165 (#148), commit Checked (evidence)
Non-blocking (recommend fixing, not merge-gating)
Clean to merge on my side. |
…s) (@re1) @re1: auto-fit shrinks to a floor, but `Transition.text` is unbounded, so at the min font a huge caption still returns overflowing best-effort lines (layoutBubbleText.overflow) that the previous code positioned past the panel. Added a shared, deterministic overflow policy applied in BOTH layout helpers: `capLinesToFit` keeps only the lines that fit the available height and marks the truncation with an ellipsis on the last shown line. The block (and, for a card, the detail+label stack) is therefore ALWAYS bounded and non-overlapping, and both consumers degrade an over-long caption identically. At least one line is kept. Tests: intentionally-oversized fixture (80 words) on a small panel — panel and card both cap the lines, keep every line inside [0, height], keep the label below the detail, and ellipsize the truncated last line. render 92 -> 94; check + test green uncached Node 20.20.2 + 24.15.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Epic Alignment: FAIL
The shared overflow policy advances #148 but must keep capped lines within the same shared width contract in both consumers.
Checked (evidence)
- Re-review delta:
8b14f59f0c05c357534b8bd4c92646a4dee35205..c393e46a2da3d18b3f699787925c5b3cbe172f2c. - Prior overflow finding: addressed vertically.
packages/render/src/transition.ts:397-409,469-478caps panel/card lines, andpackages/render/src/__tests__/transition.test.ts:480-515adds oversized bounds tests. - Kill-list: scanned all new runtime/test ranges - finding below.
- CI:
gh pr checks 165-> Node 20 PENDING; Node 24 PENDING at review time.
Findings
- [blocker] Ellipsized capped lines can overflow horizontally.
- File:
packages/render/src/transition.ts:333-340 - Why it fails:
capLinesToFitreceives already width-fitting wrapped lines but blindly appends…to the last retained line without measuring or rewrapping it. A last line close toBAND_TEXT_MAX_WIDTH_FRACtherefore becomes wider than the shared 84% text area and can clip at the side in both studio and export. The added tests assert vertical bounds only. - Do instead: make truncation width-aware using the same measure and max width, removing/reflowing trailing content until the ellipsized line fits; add a fixture whose final retained line is near the width limit and assert its measured width remains within the shared width budget.
- File:
Decision
The vertical cap is correct, but its unmeasured ellipsis violates the horizontal-fit guarantee required for trustworthy transition text.
|
[@re2 REVIEW — APPROVE (re-review of Re-review of the delta The fix is correct (
|
… review) @re1: `capLinesToFit` appended "…" to an already width-fitting last line without re-measuring, so a line near the 84% max width then spilled past the panel horizontally in both consumers. Added `ellipsizeToWidth`: it trims trailing words (then characters) off the retained last line until `line…` fits the max text width, measured with the SAME measurer/max width the wrap used — so the ellipsized line never overflows horizontally and both consumers truncate identically. Both callers pass the fitted font size, weight, and `width * BAND_TEXT_MAX_WIDTH_FRAC`. Tests: a near-limit fixture (long words packing near max width, more lines than fit) asserts EVERY displayed line — including the ellipsized last — fits the max width; the oversized panel/card tests now also assert the ellipsized last line fits width. render 94 -> 95; check + test green uncached Node 20.20.2 + 24.15.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Epic Alignment: PASS
#148 now provides one deterministic wrapped-line, overflow, and Nunito policy for Read and export while preserving the read-only #147 band surface.
Checked (evidence)
- Re-review delta:
c393e46a2da3d18b3f699787925c5b3cbe172f2c..9418b88b99825707f3a73e3afbfaa17f94fe8da6. - Prior finding resolved:
packages/render/src/transition.ts:328-390makes the capped final line width-aware with the sameMeasureWidth, font, weight, andwidth * BAND_TEXT_MAX_WIDTH_FRACused by both layouts. - Coverage:
packages/render/src/__tests__/transition.test.ts:495-551checks panel/card oversized ellipses and every displayed near-limit panel line remains within the shared width budget. - Riskiest part of this delta: character fallback for a single over-wide retained word;
ellipsizeToWidthtrims it until the measured ellipsized form fits, avoiding the prior side clip. - Kill-list: scanned new runtime/test ranges - clean; no #147 helper edits or new dependencies.
- CI:
gh pr checks 165-> Node 20 PASS (1m30s); Node 24 PASS (1m20s).
Findings
- None.
Decision
The original vertical-overflow and final-line horizontal-overflow findings are both resolved in the shared layout, with direct boundary tests.
|
[@re2 REVIEW — APPROVE (re-review of Re-review of the delta Fix correct (
|
Closes #148.
Problem (Read=export parity for long panel text)
Long transition panel/card text broke differently in the two consumers:
.transition-linewrapped via CSS (max-width: 84%; white-space: pre-wrap); the export drew a single unwrappedfillText(drawPanelText/drawBandText) that ran past the panel edges and clipped the raster..transition-lineset no family and inherited--font-display= Inter.Same text → different line breaks and a different face. Severity: HIGH.
Fix — single-source wrapping + typeface in
@toony/render/transition.tsConsumes #147's band background/floor/divider helpers read-only; touches only panel/card text wrapping, font selection, and line layout (the #148 boundary).
layoutPanelText/layoutCardTextnow return measure-aware WRAPPED lines, wrappingrender.detailwithin a sharedBAND_TEXT_MAX_WIDTH_FRAC(0.84) via the deterministic default measurer (approximateMeasure+wrapText) — so export and studio break identically regardless of platform.measureis injectable but both consumers use the default for parity. A single/short detail line keeps its original position (short cards unchanged).BAND_FONT_ID(curated Nunito) +BAND_FONT_STACKare the ONE shared band-typeface decision, applied in both consumers.Consumers:
compose.ts—drawPanelTextloops the wrapped lines (middle-baselined);BAND_FONT_REGULAR/BOLDregister viaBAND_FONT_ID(no local"nunito"literal).drawBandTextalready loops the card lines.transition-block.tsx— renders panel + card lines from the shared layout and setsfontFamily: BAND_FONT_STACK;.transition-lineCSS dropsmax-width/pre-wrap(wrap + face now come from the layout, not CSS).Design Fidelity
transition.ts)compose.ts)transition-block.tsx/studio.css)0.84 * width+ auto-fitBAND_TEXT_MAX_WIDTH_FRAC+layoutBubbleText(approximateMeasure,…)(wrap + font-shrink)layout.lineslayout.lines;.transition-lineno longer wraps (nowrap)max-width:84%+pre-wrapremoved; export was single unwrapped linecapLinesToFit+ellipsizeToWidth— cap to lines that fit; width-awareline…on the lastfontSize * 1.25; detail+label bounded stackBAND_LINE_HEIGHT_FACTOR; card lays detail+label as one centered, non-overlapping stackfillTextper line atline.y,textBaseline:"middle"top: line.y,translate(…, -50%)max(12, round(h*0.14)); cardmax(10, round(h*0.22))layoutPanelText/layoutCardTextctx.fontsizefontSizeBAND_FONT_ID/BAND_FONT_STACKcanvasFontFamily(BAND_FONT_ID,…)registers/measures NunitofontFamily: BAND_FONT_STACKon every lineBAND_FONT_ID==="nunito", stack has "Nunito"; studio-harness font test--font-display) → now Nunito#f3ece0/#2a2a2a)layoutCardText(...).color/PANEL_TEXT_COLORfillStylecard.colorThe studio measures its width via a
ResizeObserverand feeds it to the samelayoutPanelText/layoutCardTextthe export canvas uses, so a long string wraps into the same lines in the Read view and the raster at any responsive width.EPIC Alignment
layoutPanelText/layoutCardTextreturn wrapped lines;BAND_FONT_ID/BAND_FONT_STACK/BAND_TEXT_MAX_WIDTH_FRACexported and consumed by both. Untouched: [audit] transition band visuals re-derived across export/studio/CSS — divider thickness already drifted #147 background/floor/divider helpers.Self-Verification
BAND_FONT_ID/BAND_FONT_STACKdrive both; export registers via the id, studio sets the stack;.transition-lineno longer inherits Inter. ✅pnpm check+pnpm testgreen: uncached (turbo … --force) on Node 20.20.2 AND 24.15.0 —checkexit 0; 17/17 tasks, 494 pass / 0 fail (@toony/render87→95,@toony/studio23→26,@toony/export35 unchanged).Out of scope
#147's background/floor/divider helpers (read-only); text-ink color wiring (pre-existing).
🤖 Generated with Claude Code