Skip to content

[css-flexbox-1] Should Flex Container Intrinsic Main Size algorithm really use _scaled_ flex shrink factor? #6909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
davidsgrogan opened this issue Dec 23, 2021 · 8 comments
Labels
css-flexbox-1 Current Work

Comments

@davidsgrogan
Copy link
Member

davidsgrogan commented Dec 23, 2021

https://drafts.csswg.org/css-flexbox-1/#intrinsic-main-sizes says

For each flex item, subtract its outer flex base size from its max-content contribution size.

  • This result has pixel units.

If that result is positive, divide by its flex grow factor floored at 1

  • This result has pixel units.

if negative, divide by its scaled flex shrink factor having floored the flex shrink factor at 1.

  • This result has no units because scaled flex shrink factor has pixel units. scaled flex shrink factor is equal to flex shrink factor * inner flex base size (https://drafts.csswg.org/css-flexbox-1/#scaled-flex-shrink-factor).

    Put another way and expanding some terms:

    (flex base size - max-content contribution size) / (inner flex base size * max(1, flex shrink factor))

    Numerator and denominator both have pixel units so the result is unitless.

This is the item’s max-content flex fraction


So items in the same flexbox can have max-content flex fractions of different units depending on if the result of the first step is positive or negative. That doesn't seem right. Maybe

if negative, divide by its scaled flex shrink factor having floored the flex shrink factor at 1.

should be the following?

if negative, divide by its flex shrink factor floored at 1.

/cc @fantasai and @dholbert who appear to have developed this algorithm originally

@davidsgrogan davidsgrogan added the css-flexbox-1 Current Work label Dec 23, 2021
@dholbert
Copy link
Member

/cc @fantasai and @dholbert who appear to have developed this algorithm originally

In my recollection, I can't take credit for developing the spec's current intrinsic sizing algorithm (and we haven't actually implemented it in Firefox yet), though I think I did offer feedback when it was being written.

As I recall: at a high level the algorithm here is just trying to reverse-engineer what the flexbox layout algorithm actually does, in order to produce an intrinsic size that lets each flex item achieve its own max-content size. So the correct wording here would be whatever achieves that result.

(And given that the flexbox layout algorithm primarily uses the scaled flex shrink factor -- and only uses the flex shrink factor in order to compute the scaled flex shrink factor -- my gut feeling is that it wouldn't be correct to swap one for the other here.)

@davidsgrogan
Copy link
Member Author

in order to produce an intrinsic size that lets each flex item achieve its own max-content size

That is helpful. I didn't know the goal.

[gut: we should use scaled flex shrink factor]

Maybe, but it doesn't work technically here. We end up doing nonsensical calculations, as described above. As is, the algorithm is unimplementable.

@dholbert
Copy link
Member

dholbert commented Jan 10, 2022

That is helpful. I didn't know the goal.

Glad that helps! Grounding it in some spec text and making it more concrete, the spec does in fact define it as-such here:
https://drafts.csswg.org/css-flexbox-1/#intrinsic-main-sizes
"The max-content main size of a flex container is the smallest size the flex container can take while maintaining the max-content contributions of its flex items, insofar as allowed by the items’ own flexibility".

And then the algorithm that follows is attempting to define how to compute that size (perhaps with flaws right now).

[gut: we should use scaled flex shrink factor]

This isn't what I was intending to say in #6909 (comment). Rather, I was just saying that the suggestion from your initial comment here (swapping in flex shrink factor as a drop-in replacement) is probably not the right fix.

We end up doing nonsensical calculations, as described above. As is, the algorithm is unimplementable.

Sounds like we do need some fix to how the scaled flex-shrink factor is used, then.

@davidsgrogan
Copy link
Member Author

That is helpful. I didn't know the goal.

Glad that helps! Grounding it in some spec text and making it more concrete, the spec does in fact define it as-such here: https://drafts.csswg.org/css-flexbox-1/#intrinsic-main-sizes "The max-content main size of a flex container is the smallest size the flex container can take while maintaining the max-content contributions of its flex items, insofar as allowed by the items’ own flexibility".

Heh, I'd never understood that sentence, but do now :)

@davidsgrogan
Copy link
Member Author

Ok, I figured out what the algorithm was trying to say, and indeed we should continue to use scaled flex shrink factor.

(The algorithm does specify to compare values of different units, which is undefined, but I see what the spec authors were getting at, and checked my understanding against EdgeHTML)

Thanks for your help. Closing.

@Loirooriol
Copy link
Contributor

The algorithm does specify to compare values of different units, which is undefined

This seems pretty bad. Can you share what you figured out "the algorithm was trying to say", so that the spec can stop doing undefined things?

@Loirooriol Loirooriol reopened this Jan 18, 2022
@davidsgrogan
Copy link
Member Author

davidsgrogan commented Jan 18, 2022

Here's the context from https://drafts.csswg.org/css-flexbox/#intrinsic-main-sizes

  1. For each flex item, subtract its outer flex base size from its max-content contribution size. If that result is positive, divide by its flex grow factor floored at 1; if negative, divide by its scaled flex shrink factor having floored the flex shrink factor at 1. This is the item’s max-content flex fraction.
  2. Place all flex items into lines of infinite length.
  3. Within each line, find the largest max-content flex fraction among all the flex items. Add each item’s flex base size to the product of its flex grow factor (or scaled flex shrink factor, if the chosen max-content flex fraction was negative) and the chosen max-content flex fraction, then clamp that result by the max main size floored by the min main size.

As described in comment 0, step 1 produces max-content flex fractions with different units for different flex items. The units hinge on the result of the first subtraction being positive or negative. Step 3 imposes an ordering on the max-content flex fractions of varied units, which is the undefined part.

I interpret step 3 as doing this:

  • If any item needs to grow to meet its max-content contribution size (i.e. the result from step 1 is positive), we're in "grow mode" where the goal is to increase the max-content size of the container past the sum of the flex base sizes. So, add enough to each flex base size so that after the flex algorithm runs, each item is at least as large as its max-content contribution.
  • Else, if no item needs to grow (this is given because we're not in the above step) and at least 1 item can shrink from its flex base size to get to its max-content contribution, we're in "shrink mode." Subtract as much as you can from each flex base size such after the flex algorithm runs, each item is at least as large as its max-content contribution, with at least one item's flex base size being exactly equal to its max-content contribution size.
  • The max-content size is the sum of the resultant flex base sizes.

That description ignores forced line breaks and items with min/max main sizes.

In step 3, the current spec amalgamates selecting which mode to be in with calculating how much to add to or subtract from the flex base sizes.

@tabatkins
Copy link
Member

Ah, I suppose it is technically undefined how to compare a flex fraction derived from the positive flex grow factor (units of "length") and a negative scaled flex shrink factor (units of "number"), but when we wrote it we figured it since the first case is always non-negative and the second case was always non-positive it was clear enough; magnitudes didn't need to be compared between the two cases, just the sign.

The sign of the chosen fraction determines which of the factors to use, which correctly unifies the result's unit back to "length" so it can be meaningfully added to the base szie.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-flexbox-1 Current Work
Projects
None yet
Development

No branches or pull requests

5 participants