Skip to content

Flexbox: use hypothetical main sizes for column intrinsic contributions - #1165

Open
nicoburns wants to merge 3 commits into
mainfrom
devin/1787670292-column-hypothetical-main-size
Open

Flexbox: use hypothetical main sizes for column intrinsic contributions#1165
nicoburns wants to merge 3 commits into
mainfrom
devin/1787670292-column-hypothetical-main-size

Conversation

@nicoburns

Copy link
Copy Markdown
Member

Objective

Make column flex container intrinsic main sizing match what browsers actually ship, and replace the misleading comment about the row/column asymmetry with the real explanation.

As established in #351 (comment), browsers use two different item-contribution models depending on the axis:

  • Row (inline axis): the current spec's §9.9.3 intrinsic item contributions (content-based, capped/floored by flex base size depending on grow/shrink), as shipped in Chrome 136.
  • Column (block axis): the 2015-era rule — each item contributes its outer hypothetical main size (flex base size clamped by min/max main sizes, including the automatic minimum size). Grow/shrink factors and content (except via min-height: auto) are irrelevant.

Taffy previously approximated the column behavior with a content-based contribution floored by the flex basis:

// before (column branch)
(inner_main_size.max(item.flex_basis) + margin).maybe_clamp(style_min, style_max)

// after (column)
let hypothetical_inner_size = item
    .hypothetical_inner_size.main(dir)  // flex_basis clamped by resolved min (incl. auto min) / transferred sizes
    .maybe_min(style_max)
    .max(item.resolved_minimum_main_size);
hypothetical_inner_size + margin

This coincides with browsers whenever the automatic minimum size floors the item at its content size, but diverges when it doesn't. Distinguishing case: column container, item with flex-basis: 200px; min-height: 0, content 300px high — Chrome gives the container height 200, taffy previously gave 300.

Context

  • The column branch now short-circuits before the content-measurement match, so for column containers the intrinsic pass no longer measures item content at all (the auto minimum size was already computed and cached in determine_flex_base_size). The row path is unchanged apart from de-nesting the now-row-only match.
  • Three new gentest fixtures cover the distinguishing cases: min-height: 0 disabling the auto-min floor (expect 200), the default min-height: auto floor (expect 300), and max-height capping the flex basis (expect 100). All expectations are Chrome-generated.
  • The old comment attributing the asymmetry to "the automatic block size of a block-level flex container is its max-content size" is replaced with the actual history: Chromium migrated only the row axis to the new algorithm after web-compat experiments (crbug 40077556), and the spec is expected — though not certain — to be updated to match (csswg-drafts#8884).
  • Follows on from Flexbox: apply main-axis margin after flex-basis floor in intrinsic contributions #1164; relates to Implement "§9.9 Intrinsic Sizes" of the Flexbox Specification #351.

Feedback wanted

hypothetical_inner_size as stored by determine_flex_base_size is not clamped by the style max main size (only by the resolved minimum and aspect-ratio-transferred sizes), so the column branch applies .maybe_min(style_max) plus a re-floor by the resolved minimum (min wins over max). If that max-clamp should instead live in determine_flex_base_size itself, happy to move it.

Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/f379d8c9a11e428a92fe6198951c5e79
Open in Devin Desktop: https://dioxus.staging.devinenterprise.com/desktop/session/f379d8c9a11e428a92fe6198951c5e79?variant=devin-insiders
Requested by: @nicoburns

@staging-devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant