Fix: recompute line breaking in placeSubviews against bounds.width (stale single-line cache clips items)#11
Open
sanghun0724 wants to merge 1 commit into
Conversation
placeSubviews reused cache.lines from the last sizeThatFits call, which assumes that proposal matches the bounds given to placeSubviews. SwiftUI does not guarantee this and may interleave an ideal/intrinsic measurement with an unspecified width (ViewThatFits, or UIHostingController sizingOptions .intrinsicContentSize), which caches a single line. Reusing that stale cache when bounds is narrower clips all items onto one line, most visibly when the container resizes dynamically (e.g. a bottom sheet detent shrinking). Extract the line-breaking into calculateLines(maxWidth:maxHeight:subviews:) and recompute it in placeSubviews using bounds.width. Behavior is identical for the existing cases; only the stale-cache path is fixed. Fixes FluidGroup#10
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.
What
placeSubviews(in:proposal:subviews:cache:)recomputes the line breaking against the actual placement width (bounds.width) instead of reusingcache.linesfrom the lastsizeThatFitscall.Why
placeSubviewsreused the cache produced by the lastsizeThatFits, assuming that proposal equals theboundspassed toplaceSubviews. SwiftUI doesn't guarantee that — it can interleave extra measurement passes with different proposals. When a host measures the ideal/intrinsic size with an unspecified width (maxWidth = proposal.width ?? .infinity→.infinity), every item is cached onto a single line. If layout then happens at a narrower width without a matchingsizeThatFits, that stale single-line cache is placed into the narrowerboundsand the items clip to one row.Reproduces with:
WrapLayoutinsideViewThatFits(measures each candidate's ideal size with a nil proposal),UIHostingControllerwithsizingOptions = [.intrinsicContentSize],Per the
Layoutcontract, placement should honor the givenbounds.Fixes #10
How
calculateLines(maxWidth:maxHeight:subviews:)helper.sizeThatFitsuses it with the proposed width (unchanged behavior).placeSubviewscalls it withbounds.width, then applies the existing horizontal/vertical alignment logic.For very large child counts a width-keyed cache would avoid the extra measurement; for typical wrap content the recompute cost is negligible. Happy to switch to a width-keyed cache instead if you prefer.
Notes on tests
The existing snapshot tests already fail on a current iOS simulator (iPhone 17 Pro) before this change — the recorded references were taken on an older toolchain, so text metrics differ (e.g.
testWrappingrenders at height 54.67 vs reference 93.0). With this change the freshly-rendered sizes are identical to the unchanged code, i.e. the change is behavior-preserving for the covered cases; only the stale-cache placement path is fixed. I left the reference images untouched to avoid re-recording on an unrelated toolchain — glad to regenerate them if you'd like.