Repository navigation
fix(canvas): Pool, Register and Parameter value rows sit inside the node (v0.21.2) - #333
Merged
Merged
Conversation
…ode (v0.21.2) Issue #332. Version 0.21.2. Cause: the rows under a node's title (a Pool's value and capacity, a Parameter's value and unit, a Register's result and `= expr`) started at the content edge, 14 px before the title text. On the Pool's slanted side the value sat on or across the drawn outline (down to -9.7 px), and a Register's value sat inside the keyboard-focus ring. This has been so since the first implementation. Fix: src/components/nodes/rowFit.ts (pure) decides, from what NodeFrame measures, where each row starts and how wide the node must be: each row starts at the title text along the stack's physical start (the canvas is left-to-right in every language), at least 8 px (the focus ring plus 2) inside the fill at the row's own height at both ends. A node keeps its width when that already holds; otherwise only that node widens, by the minimum, up to 260 px, and only for a row that is then whole. A title that had 8 px or more keeps at least max(8, its clearance - 0.5); one that had less keeps all of it. A widening that would unwrap a title, and so change the node's height, does not happen. Where a rule stops a widening, the row is cut with a visible ellipsis; only the display is cut. silhouette.ts gains fillSpanAt, which reads the fill straight from the drawn path. The fit is taken in the same pass as the height, when the rendered strings, the language, the fonts or the height change, never per animation frame; it reaches index.css as custom properties (a physical margin-left, a plain px max-width) and a min-width. Source, Drain, Converter, Gate and End rows are unchanged: their pointed, notched or slanted outlines grow with the width, so the same rule widened 37 template nodes by up to 49 px (deferred). Measured against main 7e66da2 on the same machine: in the three templates exactly one node widens (early MMO r_income, 126.84 to 132.5 px); node heights, positions and every drawn connection path are unchanged (0 of 232 paths); two Registers whose `= expr` row is cut differ by 0.016 px. 246 in-scope rows, smallest clearance 8.05 px, none crossing. A 2,400-Parameter import: median 3899 ms on main, 4303 ms here over five alternating rounds (+10.4 %, a one-time mount cost); the pure fit is 69 ms of it. Tests: src/components/nodes/rowFit.test.ts (the fill, the fit on a grid of widths, the title and height rules). e2e/value-row-alignment.spec.ts (13) and value-row-alignment.mobile.spec.ts (3) read every row against the drawn outline (Range glyphs, isPointInFill) in a synthetic graph and the three templates, light, dark, forced colours, ar and a phone; states that move nothing; Source, Drain and Converter untouched; the content digest; a dev-only measurement counter that rises only on a frame whose row text changed. e2e/whats-new.spec.ts pins 0.21.2. Visual baselines: 26 updated, each one reviewed against main's own capture: the rows moving, and Registers and Parameters in the test fixtures widening with their outline, ports, invalid flag and rings. No title, height, position, port or connection change outside a widened node. The 20 snapshots that changed within tolerance are not updated, nor state-inspector-light, which alternates between two images on main itself. Release: three release-note lines in 18 languages, 16 without native review; they say "nodes that show a value" rather than naming the kinds, so the copy guards' per-kind key counts stay as they are. The guards move only their exact pins (catalog 1033 to 1036, runtime 1255 to 1258); all three lines read the same in pt-BR and pt-PT, so pt-PT's difference stays at 277 (249 outside the password keys), and no ru line carries a new ё word. Docs: docs/node-shell-content-in-vessel.md, CHANGELOG.md and README.md.
Deploying cozy-loop-studio with
|
| Latest commit: |
8e05244
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1d34280f.cozy-loop-studio.pages.dev |
| Branch Preview URL: | https://fix-value-row-alignment.cozy-loop-studio.pages.dev |
…e draws them alike Issue #332, CI of the first commit. Two screenshot specs failed on the runner (register-unit-row, and the mobile forced-colors-L2): a Parameter's and a Pool's value and detail rows were drawn half a pixel left of the new baselines. The titles and every other row matched the runner exactly. Cause: rowFit.ts rounded each row start UP to a half pixel. A title start measured a hair above a whole pixel (29.00001) became 29.5 on this machine and stayed 29 on the runner, so the row sat half a pixel right of its title locally and the baselines captured that. Fix: a row aligned to the title takes the title's own start to the nearest 1/64 px (a layout unit), so it shares the title's sub-pixel phase; a clearance to the outline is only rounded up and a row's room only down, both to 1/64 px. The start and the minimum width keep four decimals. Re-rendered here, the two rows are now identical to the runner's capture (offset 0.000, same ink). Baselines: three of the 26 already updated in this pull request change again, the same rows half a pixel left and one invalid Register's outline by a sub-pixel width (register-unit-row, forced-colors-L2 on chromium, flow-colour-states). No other snapshot changes.
Issue #332, CI of the second commit. Two screenshot specs failed on the runner, each with an identical retry: matrix light L2 on chromium (108 px) and forced-colors L2 on mobile (313 px), both in shard 4. Cause: the second commit replaced only the three baselines that failed here against the half-pixel ones. All 26 baselines this pull request updates had been captured with the half-pixel rounding, and 21 of them still showed it; most stayed within this machine's tolerance, two did not on the runner's. Fix: every snapshot re-captured on the final code (da47662, clean tree), and of the 26 exactly the 21 that differ are replaced; the 5 identical ones stay, and no other baseline, no product code and no tolerance changes. Playwright's own comparator, at each spec's tolerance, judges the committed baselines against the runner's two images as 108 and 313 px, the runner's own numbers, and the new ones as passing. The runner's rows now sit where these do; what is left between the machines is the `≤` and `Δ` glyphs' anti-aliasing (likely a fallback font), within tolerance.
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.
Part of #332, released as v0.21.2 (the release date is set to the actual merge day before merging).
The bug
In a Pool, a Parameter and a Register, the title starts after its kind chip, but the rows under it started at the content edge, 14 px before the title text. On the Pool's slanted side the value sat on or across the drawn outline (down to -9.7 px), and a Register's value sat inside the keyboard-focus ring. Measured on
main7e66da2and on productionv0.21.1(the two agree within 0.6 px). This has been so since the first implementation; it is not a regression of #325.Scope
= exprpreview.The fix
src/components/nodes/rowFit.ts(pure,fitRows) decides, from whatNodeFramemeasures, where each row starts and how wide the node must be.silhouette.tsgainsfillSpanAt, which reads the fill straight from the drawn path.dir="auto"still starts on the chip's side), and stays at least 8 px (the focus ring plus 2) inside the fill at the row's own height, at both ends.max(8 px, its clearance - 0.5 px); one that had less keeps all of it.1234567.89reads12345…). Only the display is cut; stored values, files and calculations are unchanged.src/index.cssas custom properties (a physicalmargin-left, and a plain pxmax-width: a%insidemin()is cyclic in the intrinsic pass and drops the wholemax-width) and amin-width.Before and after
Measured against
main7e66da2on the same machine,mainserved from its own copy on its own port:r_income, 126.84 to 132.5 px (+5.66)= exprrow is cut differ by 0.016 pxmain, 4303 ms here (+10.4 %, a one-time mount cost; the pure fit is 69 ms of it)Visual baselines
26 updated, each reviewed against
main's own capture:No title, height, position, port or connection changes outside a widened node. 20 other snapshots changed within tolerance and are not updated, nor
state-inspector-light, which alternates between two images onmainitself.Tests
src/components/nodes/rowFit.test.ts: the fill read from the path, the fit on a grid of widths, the title and height rules.e2e/value-row-alignment.spec.ts(13) andvalue-row-alignment.mobile.spec.ts(3): every Pool, Parameter and Register row against the drawn outline (Range glyphs,isPointInFillat the row's top, middle and bottom) in a synthetic graph and the three templates, light, dark, forced colours,arand a phone; selection, keyboard focus, Focus mode and the activity overlay moving nothing; Source, Drain and Converter untouched; the content digest; and a dev-only measurement counter that rises only on a frame whose row text changed (idle, pan, zoom and playback frames take no measurement).e2e/whats-new.spec.tspins 0.21.2. The existing long-label tests (a value never clipped) pass unchanged.Release
.changes/value-row-alignment.json, release noterelease:0.21.2with three lines in 18 languages, 16 without native review. They say "nodes that show a value" rather than naming the kinds, so the copy guards' per-kind key counts stay as they are.docs/node-shell-content-in-vessel.md("Follow-up — value and detail rows"),CHANGELOG.md,README.md.Verification (local, at the head commit)
npx tsc -b, oxlint (39 warnings, the existing baseline, 0 errors), 3,223 unit tests, all 21 source checks of the CIchecksjob; the web, portable, PWA and production-branch builds with their closure and notice checks.main's 1,997 plus exactly these 16 (13 chromium, 3 mobile), none removed, each in exactly one of the 5 shards.__loopin the web, portable and PWA bundles, and the production and portable specs assert there is no bridge.mobileproject 112 passed, 4 skipped; the related specs (What's new, i18n, nodes, templates, canvas, routing, data import, frames) 853 passed, 3 skipped; the production bundle 16 of 16, the PWA 19 of 19, the portable file 16 of 16.CI
Run 37621453781 at
9e83e1c, the first commit, failed two screenshot specs, each with its retry:register-unit-row(shard 3, chromium, 117 px) andforced-colors-L2on mobile (shard 4). Every other job passed. It is kept as it is, not re-run.Cause: the runner drew a Parameter's and a Pool's value and detail rows half a pixel left of the new baselines, while the titles and every other row matched to the pixel (ink offset 0.000).
rowFit.tsrounded each row start up to a half pixel, so a title start measured at 29.00001 became 29.5 on the machine that made the baselines and stayed 29 on the runner. The runner's rows were the aligned ones.Fix,
da47662: a row aligned to the title takes the title's own start to the nearest 1/64 px (a layout unit), so it shares the title's sub-pixel phase; only a clearance to the outline is rounded up, and a row's room down, both to 1/64 px. Re-rendered locally, the two rows are identical to the runner's capture. Three of the 26 baselines change again, the same rows half a pixel left and one invalid Register's outline by a sub-pixel width:register-unit-row,forced-colors-L2on chromium andflow-colour-states. No other snapshot changes.Checked again at
da47662, locally: every screenshot spec on chromium and mobile with the new specs and the data import, twice in a row without an update, 337 passed, 5 skipped by design, both times; smallest row clearance 8.05 px, the title rule held;npx tsc -b, oxlint (39), 3,223 unit tests, all 21 source checks; the list 2,013 (the same as at9e83e1c), each test in exactly one of the 5 shards, the same placement.Run 37628234548 at
da47662failed two screenshot specs in shard 4, each with an identical retry: matrix light L2 on chromium (108 px) and forced-colors L2 on mobile (313 px). Every other job passed. It is kept as it is, not re-run.Cause:
da47662replaced only the three baselines that failed locally against the half-pixel ones, but all 26 baselines this pull request updates had been captured with the half-pixel rounding; 21 of them still showed it, and two of those went over the tolerance on the runner. The runner's trace shows the same row starts as here (for example the Gold Pool's14px).Fix, the third commit: every snapshot re-captured on the final code (
da47662, clean tree), and of the 26 exactly the 21 that differ are replaced; the 5 identical ones stay; no other baseline, no product code and no tolerance changes. Playwright's own comparator, at each spec's tolerance, judges the committed baselines against the runner's two images as 108 and 313 px (the runner's own numbers) and the new ones as passing. What is left between the two machines is the anti-aliasing of the≤andΔglyphs (likely a fallback font), within tolerance.Run 37637340044 at
8e05244, the third commit, passed every job without a retry. The five shards ran exactly the 2,013 listed tests, each once (2,006 passed, 7 skipped by design, 0 failed, 0 retried); the production bundle 16 of 16, the PWA 19 of 19. This third run is the final approval evidence; the two failed runs above stay as they are.The preview at
8e05244(the production bundle on its Cloudflare preview origin, through the UI only, in fresh Chrome contexts): it readsv0.21.2 · build 8e05244on a desktop andv0.21.2 · 8e05244in the phone's ⋯ menu, and has no dev bridge. Every Pool, Parameter and Register row in view, in the synthetic graph and the three templates, in light, dark, forced colours and on a phone (125 rows), is 8 px or more inside the outline at both ends and starts at the title; the smallest clearance is 8.03 px. No page error. Two earlier passes of this check read the version from the wrong element (the document title, then the desktop brand on the phone); only the corrected pass counts.Not claimed