The rendered note gets a reading measure, and the demo note stops faking one - #508
Merged
Conversation
lilseyi
force-pushed
the
claude/note-reading-measure
branch
from
September 13, 2026 09:15
553546d to
4be54e0
Compare
Measured in Chromium at 1440x900 against main: the element holding the note's first body sentence was 1160px wide with `max-width: none` on every one of its first eight ancestors, and neither of the editor's stylesheets contained a `max-width` at all. With a real, unwrapped paragraph in it that is 148 characters to a line — about double the 60-75 that is comfortable to read. `--lp-measure` is 62ch (`layout.readingMeasureCh`), and the column is cut out of `.cm-content` with `padding-inline: max(0px, calc((100% - var(--lp-measure)) / 2))`. In characters rather than pixels because the same note is drawn at 14.5px beside a file tree and at 16px on a phone, and a measure in characters is the same sentence at both; on a phone the max() floor is zero, so `--lp-pad-x` still governs and nothing there moves. On the column rather than on the line, because a table, a form and a rendered diagram are block children of the same element — measure the lines alone and each of those starts at a different left edge from the paragraph above it. Nothing is allowed to be wider than the prose; a wide table stays inside the column and scrolls in its own box, which `.cm-lp-grid` already did. Padding rather than `max-width` + auto margins, which draws the identical column and was measured and rejected: it leaves `.cm-content` 572px wide inside a 1192px pane, so a click in the 310px either side lands on `.cm-scroller`, the editor never takes focus, and clicking beside a line to put the caret in it does nothing. Half the note's apparent area would stop being the editing surface — a worse bug than this one, and invisible, because the page looks right. Declared in all three places a `--lp-*` has to be: the base `.cm-lp-root` rule, the guest's `:root`, and `themeVars`. An undeclared custom property makes its whole declaration invalid at computed-value time, which is how PR #487's bug withdrew colours across the editor without an error, and the existing guard in `liveEditorMount.test.ts` covers this one now too. `webviewBridge.test.ts` gains the same relationship from the guest's end, which it did not have. jsdom lays nothing out, so these tests assert the rules; the rendered result — the character count, the centring, and a click in the margin — is asserted in a browser in the following commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012pS75o4hy4RDLUegCyrLcx
…res the result `1-projects/context-lc.md` was hard-wrapped at about fifty characters in the fixture, and it is `defaultSelection` — what the console opens on, what the e2e fixture shows first, and what every screenshot of this editor has ever contained. So the demo text broke at a comfortable width no matter what the layout did, and the layout was doing nothing: measured in Chromium at 1440x900 on main, the element holding the first sentence was 1160px wide with `max-width: none` on every ancestor. Pre-wrapped demo data is why that survived every visual check there was. Same wording, same frontmatter, the browser's line breaks. Nothing asserts on those line strings — checked. `readingMeasure.spec.ts` is what the measure needed and could not have in jsdom, which lays nothing out and so cannot tell a `max-width` that binds from one that does not. It drives the built web export in a real engine and asserts, at 1440x900, the character count on the rendered line (measured: 148 with the measure removed in-page, 75 with it), that the column is centred, and that a click in the margin beside a line still puts the caret in the note; and at 390x844 that the line is exactly the pane less `--lp-pad-x` either side, so nothing about the phone moved. It measures the line box rather than `.cm-content`, because `.cm-content` is deliberately still the full width of the pane — see the previous commit. Run: `--project=chromium` in this sandbox, 43 passed. Layout is not what the WebKit/Chromium distinction in this suite is about, but a chromium pass is still not a WebKit result; CI runs the webkit project. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012pS75o4hy4RDLUegCyrLcx
The WebKit job caught this on the first push, which is exactly what it is for. The measure shipped as `62ch` and rendered **75 characters a line in Chromium and 91 in WebKit on the same Linux runner** — same declaration, same viewport, sixteen characters apart, and 91 is the defect this change exists to fix arriving by another route. `ch` is the advance of the digit zero, so it tracks a face's digits rather than its prose, and the ratio between the two is itself a property of the face. An `em` measure varies with one font metric where a `ch` measure varies with two. `ch` was ambiguous a second way as well. A font-relative length inside a custom property may be resolved where the property is *declared* or where it is *substituted*, and engines differ — and those are two different lengths here: measured in the built page, `.cm-lp-root` is Times New Roman at 16px (nothing sets a face on the wrapper) while the note is Instrument Sans at 14.5px. So the property now carries a bare number and the rule that draws the text multiplies by `1em` there, against the type it is measuring. Both hosts assert the value has no unit, because a unit sneaking back in is that ambiguity returning silently on one engine only. `layout.readingMeasureEm` is 36: measured at 522px and 71 characters at 1440x900, inside the 60-75 band. The rendered column is now exact arithmetic rather than a font's opinion — 36 × 14.5px — so the e2e case asserts the em multiple tightly and the character count loosely, and logs the width, the multiple, the count and the resolved face on every run so a surprise names the font it is arguing with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012pS75o4hy4RDLUegCyrLcx
lilseyi
force-pushed
the
claude/note-reading-measure
branch
from
September 13, 2026 09:32
e9a785c to
fb5ca96
Compare
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.
The defect
Driving the built web export in Chromium at 1440x900 against
main:x=248, width=1160max-width: noneon every one of its first 8 ancestorsmax-widthanywhere in either of the editor's stylesheetsProse was full-bleed across whatever width the editor pane had. With a real, unwrapped paragraph in it that is 148 characters to a line (measured, not estimated — the measure was removed in-page on the fixed build to get the number), against the 60-75 that is comfortable to read.
Why nobody had seen it:
1-projects/context-lc.mdwas hard-wrapped at about fifty characters inplaceholderData.ts, and it isdefaultSelection— what the console opens on, what the e2e fixture shows first, what every screenshot of this editor has ever contained. The newlines were doing the wrapping; the layout was doing nothing. Pre-wrapped demo data hid the entire class of bug from every visual check there was.The measure: 36 × the note's own type size
--lp-measureislayout.readingMeasureEm= 36, applied aspadding-inline: max(0px, calc((100% - var(--lp-measure) * 1em) / 2))on.cm-content.Relative to the type, not in pixels. The same note is drawn at 14.5px beside a file tree and at 16px on a phone; one multiple is the same line at both, where a pixel width would be two numbers to keep in step. Measured: 36em is 522px holding 71 characters at 1440x900 — inside the comfortable band. Prose in a system sans averages 0.45-0.55em a character, so 36 lands roughly 65-80 across faces; erring short is the cheaper error.
One value for both densities. On a phone the
max()floor is zero, so--lp-pad-xgoverns and nothing moves. Proved, not asserted (below).em, not thechthat nominally means "characters" — and CI is whyThe first push of this PR used
62ch. The WebKit job caught it: 75 characters a line in Chromium and 91 in WebKit on the same Linux runner.chis the advance of the digit zero, so it tracks a face's digits rather than its prose, and the ratio between them is itself a property of the face — achmeasure varies with two font metrics where anemmeasure varies with one.chwas ambiguous a second way too: a font-relative length inside a custom property may be resolved where the property is declared or where it is substituted, and engines differ. Those are different lengths here — probed in the built page,.cm-lp-rootis Times New Roman at 16px (nothing sets a face on the wrapper) while the note is Instrument Sans at 14.5px. So the property carries a bare number and the rule that draws the text multiplies by1emthere, against the type it is measuring. Both hosts assert the value has no unit: a unit sneaking back in is that ambiguity returning silently, on one engine only.The column is now exact arithmetic (36 × 14.5px = 522px), so the e2e case asserts the em multiple tightly and the character count loosely, and logs width / multiple / count / resolved face every run:
What may exceed the measure: nothing
The constraint is on
.cm-content, not.cm-line. A table, a form, a rendered diagram and a code fence are block children of the same element — measure the lines alone and each of those starts at a different left edge from the paragraph above it, which reads as broken layout rather than as a wide table.A table is the case with a real argument on the other side (twelve columns in a 36em column is cramped) and it still loses, because what it gets instead is its own horizontal scroller:
.cm-lp-grid { overflow-x: auto }— which #487 already shipped, with a comment about "a table wider than the measure" that had no measure to be wider than until now. Covered by a test.Padding, not
max-width— the second non-obvious line, and an adversarial findingThe obvious recipe is
max-width: var(--lp-measure); margin-inline: auto. It draws exactly the same column. I built it that way first, then measured it and rejected it: it leaves.cm-content572px wide inside a 1192px pane, so a click in the 310px either side lands on.cm-scrollerand the editor never takes focus. Measured in Chromium on the built export:Half the note's apparent area silently stops being the editing surface, and clicking beside a line to put the caret in it is something people do. So
.cm-contentkeeps the pane's full width and the column is cut out of it with padding; themax()floor hands the width straight back once the pane is narrower than the measure, which is the phone.readingMeasure.spec.tsclicks in the margin, so the tidier-looking recipe cannot come back quietly.Declared in both hosts (#487's lesson)
--lp-measureis declared in the base.cm-lp-rootrule (LiveEditor.web.tsx), in the guest's:root(files/webview/styles.ts), and inthemeVars(files/webview/host.ts). An undeclared custom property makes its whole declaration invalid at computed-value time — not an error, not a fallback — which is how #487's bug withdrew colours across the editor.liveEditorMount.test.ts's existing guard covers the new property, unweakened (still base-block only, so a property declared solely inside the phone media query cannot pass).webviewBridge.test.tsgains the same guard from the guest's end, which it did not have: every--lp-*the guest stylesheet reads is declared by its:root, and every one the host is responsible for is inthemeVars(--lp-inset-bottomnamed as the one runtime exception, with why).Measurements, before and after
Body text block (the leaf element holding the first sentence), Chromium, built web export:
main)x=248, width=1160—max-width: noneon all 8 ancestorsx=567, width=522(36.0em), centred — 335 either sidex=24, width=342x=24, width=342— unchangedCharacters on the longest rendered line of the first paragraph:
Gates
apps/mobile:pnpm test5,307 passed / 280 suites;pnpm lint0 errors (19 pre-existing warnings);pnpm typecheckclean.apps/mcp: 3,378 checks, ALL PASS — no regression.scripts/build-editor-bundle.mjs), sincewebview/styles.tsis a bundle source; the bundle job is green.--lp-measurefromthemeVarsand the padding from both stylesheets fails 4 of the new tests, including the one naming the phone.pnpm test:e2e:webkit:chromium— thechromiumproject, 43 passed, in a sandbox with no WebKit binary. Not a WebKit result; thewebkitproject is CI's, and it is what found thechproblem.Durable decision
docs/decisions/app-and-console.md— "The note is a measured column, and the demo note stopped faking one", indexed indocs/decisions/README.md.🤖 Generated with Claude Code
https://claude.ai/code/session_012pS75o4hy4RDLUegCyrLcx