Flexbox: don't wrap auto-height column containers against ancestor-derived available space - #1176
Merged
Merged
Conversation
…rived available space
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
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.
Objective
Fixes #1175.
An auto-height
flex-wrap: wrapcolumn container could wrap its lines againstAvailableSpace::Definitemain-axis space handed down by an ancestor (typically during aComputeSize/ContentSizemeasurement round), even though its own main size is content-based. This showed up as one-frame layout flicker while animating a child's height: for the frames where the content total exceeded the ancestor-derived available height by a couple of ulps, the container split into two columns. Chrome keeps a single line for the equivalent DOM.The wrap constraint in
collect_flex_lines(andcollect_balanced_flex_lines) previously fell back toavailable_space.main(dir)whenever the container had no max main size:Now, for a column container with no definite main size and no max main size, definite available space is replaced with
MaxContent(no wrapping), since an automatic block-axis size never resolves against available space:Rows are unchanged: an automatic inline-axis size does resolve against available space (fill-available), so row containers still wrap against definite available width. Min/max-content constraints and containers with a definite main size or a max main size are also unchanged.
Context
set_style+compute_layout_with_measure) produced 2 anomalous frames onmain; 0 after this change.wrap_column_auto_height_does_not_wrap_against_available_space: a non-stretched auto-height column-wrap container (2×100px items) inside a 150px-tall row parent. Chrome-generated expectation is a single 60×200 column; onmainall 4 generated variants fail (taffy wrapped into two columns), with this change all 6069 generated tests pass (default and--all-features).Feedback wanted
The fix keys on
!dir.is_row()as a proxy for "main axis is the block axis" (taffy has no writing modes). If you'd prefer the same treatment for rows under some condition (e.g. only duringContentSizesizing rounds), happy to adjust — but as-is, rows keep the fill-available wrapping behavior that matches browsers for block-level row containers.Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/b637316906704d10ba77a4da9f248d4e
Open in Devin Desktop: https://dioxus.staging.devinenterprise.com/desktop/session/b637316906704d10ba77a4da9f248d4e?variant=devin-insiders
Requested by: @nicoburns