Fix flexbox aspect-ratio min sizing and floated flex container sizing - #1081
Open
nicoburns wants to merge 3 commits into
Open
Fix flexbox aspect-ratio min sizing and floated flex container sizing#1081nicoburns wants to merge 3 commits into
nicoburns wants to merge 3 commits into
Conversation
🤖 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:
|
…ross available space by item margins
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
Fix layout regressions observed in DioxusLabs/blitz#625 (bumping blitz's taffy dependency from 0.12.x to git main), diagnosed via WPT
css/css-flexbox/flexbox-min-height-auto-002b.html.Four fixes:
Flexbox automatic minimum size (
src/compute/flexbox.rs): the content size suggestion is now clamped by min/max cross sizes transferred through the aspect ratio (per css-sizing-3 §5.1), not just the transferred max size:Regression from Flexbox: apply transferred min/max sizes to flex base size and hypothetical sizes #989 (bisected to 0972b84): an item like
<img style="height: 100px; min-width: 30px">(aspect-ratio 1) in a 1px-high column container resolved its automatic min-height to ~0 and collapsed instead of flooring at 30px.Flexbox used cross size (
src/compute/flexbox.rs): when an item's cross size is transferred from its main size through its aspect ratio (cross size property isauto),determine_used_cross_sizenow re-derives it from the used (post-flexing) main size instead of using the hypothetical cross size that was transferred from the flex base size before flexing. E.g. after the item above is shrunk from 100px to 30px main size, its cross size is now 30px rather than 100px. The hypothetical cross size (which feeds the container's intrinsic cross sizing) is unchanged, matching Chrome.Floated flex/grid containers are now shrink-to-fit sized (
src/compute/block.rs): the float layout path passedAvailableSpace::Definite(available_width)with no known width. Block containers implement fit-content sizing internally in that situation, but flex/grid containers treat definite available space as stretch-fit, so a floated flex container's items were stretched to the full available width. The float path now computes the fit-content width (available.clamp(min_content, max_content)clamped by the float's min/max width) up front and passes it as a known dimension.Cross-axis available space is clamped by the item's own margins, not the container's (
src/compute/flexbox.rs):determine_flex_base_size/determine_hypothetical_cross_sizeaddedconstants.margin(the container's margin) to the item's min/max cross sizes when clamping the cross-axis available space. A container margin (e.g.margin-right: 2px) could inflate a stretched item's cross size beyond the container's own width. This dates back to Fix: available space in the presence of a min width #407 but was latent until floats/definite available space exercised it.Context
Found while investigating flexbox regressions in DioxusLabs/blitz#625. With these fixes plus a small blitz-side fix, both regressed WPT tests (
flexbox-min-height-auto-002b.html,flexible-box-float.html) pass again.Includes gentest fixtures with Chrome-generated expectations:
aspect_ratio_flex_column_min_height_auto_transferred_min_width,float_flex_container_shrink_to_fit, andfloat_flex_container_max_width_margin_stretch_item.cargo test --workspace, fmt, and clippy pass.Feedback wanted
aspect-ratio: 1gives height 34 where taffy gives 30). That behavior is not implemented here; the WPT test passes in blitz because images take blitz's replaced-element path.Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/40e9a29868044e77a6dbf77a7b4b11e3
Requested by: @nicoburns