fix(examples): the carousel sets leading in points, not multiples - #500
Merged
Conversation
Every lineSpacing call in this file carried a typographic multiplier — 1.03, 1.06, 1.25, 1.4, 1.45. ParagraphBuilder.lineSpacing takes points: TextFlowSupport adds it between wrapped lines as `(lineCount - 1) * gap`, on top of the font's own line height. So the deck asked for a point and a half of extra leading on type ranging from 26 to 156 points, and rendered at effectively default leading throughout. Every slide read as a wall. The values are now expressed in the file's own design units, tapering with type size because display faces need proportionally less: 8 * SCALE on the 132pt headline, 6 on the 82pt slide titles, 5 on the 40pt lead paragraph, 4 on the 28pt card body, 3 on the chart labels and the footnote. On the 33pt card body that moves the gap from 1.4pt to 4.7pt. Measured on the rendered pages at 110 dpi: a four-line card grows 10.5pt, a three-line card 6.5pt, a two-line card 2.6pt — three, two and one inter-line gaps of 3.3pt each, which is what the arithmetic predicts. The deck stays six slides and the last card on the tallest slide still clears the bottom margin by 193pt. ./mvnw -B -ntp clean verify — BUILD SUCCESS, 692 tests in the closing module, CommittedAssetDriftTest green: this deck is an unpublished preview, so no committed asset moves with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Every
lineSpacingcall in the carousel carried a typographic multiplier —1.03,1.06,1.25,1.4,1.45. The API takes points:ParagraphBuilder.lineSpacingis documented@param lineSpacing line spacing in points;private double lineSpacing = 0.0— a multiplier's default would be1.0, and0.0would collapse every line onto one;TextFlowSupport:681-689adds it astotalHeight += (lineCount - 1) * gap, on top of the font's own line height.So the deck asked for one to one-and-a-half points of extra leading on type ranging from 26 pt to 156 pt, and rendered at effectively default leading throughout. That is why every slide read as a wall of text.
What changed
The values move into the file's own design units and taper with type size, because display faces need proportionally less leading than body copy:
8 * SCALEon the 132 pt headline,6on the 82 pt slide titles,5on the 40 pt lead paragraph,4on the 28 pt card body,3on the chart labels and the footnote. On the 33 pt card body that moves the gap from 1.4 pt to 4.7 pt. A comment at the first site records what the unit is, since the next person will guess the same way.Verification
Rendered before and after, rasterized at 110 dpi, and measured the card panels rather than judging by eye:
Three, two and one inter-line gaps of 3.3 pt each — exactly what the arithmetic predicts, which is the check that the change did what it claims rather than something else.
The deck stays six slides; the last card on the tallest slide clears the bottom margin by 193 pt. The three-line wrap of the cover headline is pre-existing — it is in the before render too.
./mvnw -B -ntp clean verify—BUILD SUCCESS, exit 0, 692 tests in the closing module.CommittedAssetDriftTestgreen: this deck is inUNPUBLISHED_PREVIEWS, so no committed asset moves with it.A larger finding this exposed — not fixed here
The same misreading is repo-wide. Counting numeric literals in
src/main:coretemplatesexamplesZero call sites in the engine or the published templates use the API as documented.
TimelineBuilder:287sets1.3on every timeline entry body — that is engine code that ships. All the CV, cover-letter, invoice and proposal presets do the same, so every preset a user renders is running at essentially default leading while its author intended 1.3–1.5×.docs/templates/v1-classic/authoring.md:189is likely where it started: it says "default 1.0 squashes \n-joined lines", which is wrong — the default is0.0.Fixing that is a separate decision with real consequences: correcting 35 call sites in shipped code moves every layout snapshot and visual-regression baseline. Flagged for a call, not folded in here.
Lane: examples. No production code, no public API.