fix(codegen): preserve hard-break spaces in height-auto text - #49
Merged
Conversation
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.
A space the designer typed before an explicit line break is collapsed away by CSS, so a paragraph reads one way in Figma and another on screen.
The other eleven screens are unchanged.
This continues an existing rule rather than inventing one
W8 established that painted text must equal
characters- it stopped JSX inserting a space the design does not have, and stopped a design line break becoming a space. It then reported the remaining collapse as an explicit finding because no emission it had could avoid it.pre-wrapavoids it for a bounded case, so that case moves from reported to fixed andDEVUP_CODEGEN_TEXT_WHITESPACE_COLLAPSEstays for everything still unavoidable. One lossy fidelity impact is retired rather than left double-counted beside the diagnostic.The condition is narrow on purpose:
TEXTwhose inline width Figma fixes while height grows (textAutoResize == HEIGHT), no positivemaxLines, no list options, no tabs, and a space actually adjacent to a break. HUG text is excluded becausepre-wrapcounts trailing spaces in max-content width and would change intrinsic sizing; clamped text keeps its existing projection.On the four goldens
Four plugin goldens gain six
whiteSpaceattributes. This is the opposite case to Koreankeep-all, and the distinction is the whole argument.With
keep-all, matching Figma would have produced worse Korean - words chopped mid-break - so the compensation was deliberately kept and the pixel gain rejected. Here the design contains a space and CSS eats it, so preserving it makes the output match the source. The plugin drops that space; here the plugin is the one diverging from the design.Each golden was reviewed against its collected fixture:
165(node1:18),191(107:32),252(284:18751/18753/18754),253(213:7494). Six attribute additions, no other output change, only those four checksums touched, corpus back to 268/268.docs/about-landing-mobile-golden-review.mdrecords the per-node review so a future reader can tell these two cases apart without re-deriving them - they look superficially identical and the answers are opposite.Verification
Measured independently with binaries built from
mainand this branch, full 15-screen acquire and render on each side, repeated. My figures match the worker's to the decimal, and the baseline run fails correctly against all four tightened thresholds - confirming the two runs measure different code rather than reusing inputs.New label
derived-hard-break-whitespacestates it preserves source whitespace without claiming intrinsic HUG sizing, glyph parity or identical wrapping. No viewport, breakpoint or node id in production code.Gates:
fmt0,clippy0,cargo test --workspace1084 passed / 0 failed / 2 ignored,insta --checkno drift, corpus 268/268.Still open
A separate hero-export defect is quantified in the report but not fixed: it needs an isolated-fill export contract with crop and multi-paint validation first. Left documented rather than forced.