Skip to content
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

[css-flexbox-1] Intrinsic sizing algorithm seems to produce 0 for many common cases #1435

Closed
dholbert opened this issue May 21, 2017 · 7 comments
Closed

Comments

@dholbert
Copy link
Member

@dholbert dholbert commented May 21, 2017

I'm reviewing @cbiesinger 's testcase for the (not-implemented-anywhere AFAIK) flexbox intrinsic sizing algorithm, over in web-platform-tests/wpt#5791 , and I think we might've uncovered some unintended spec behavior with the intrinsic sizing algorithm. (And maybe it's intended? But it doesn't match biesi's testcase's expectations, so I want to be sure.)

In particular: it seems to me that in many common cases (with empty flex items at least), the algorithm requires that flex items contribute 0 to their container's intrinsic size, and the container will end up with an intrinsic size of 0 as a result.

For example, consider these two cases:

  <div style="display:inline-flex;">
    <div style="flex: 1 1 200px"></div>
  </div>

  <div style="display:inline-flex">
    <div style="flex: 1 1 auto; width: 200px"></div>
  </div>

Here, you might reasonably expect that each flex item & flex container to be 200px wide. But in both cases, the algorithm ends up sizing the item & container to 0px. This is because:

This behavior (ending up 0-sized) kind of makes sense, because the items have no content and no min-width/max-width properties, which mean's they're "OK" being 0-sized, in a sense -- there's no overflow. Still: it seems odd for an element with width:200px to end up being 0-sized, in a scenario with no constraints.

@tabatkins @fantasai , am I reading the spec correctly here? And is this zero-sizing the intended spec behavior for cases like this?

@tabatkins
Copy link
Member

@tabatkins tabatkins commented Jun 22, 2017

It wasn't explicitly intended, no. But I'm also not sure it's something we actually want to change. ^_^ @fantasai and I will have to think on this.

@astearns astearns removed the Agenda+ F2F label Aug 1, 2017
@fantasai fantasai added the Agenda+ label Aug 8, 2017
@fantasai
Copy link
Collaborator

@fantasai fantasai commented Aug 8, 2017

I think the options we have are:

  • No change: flex basis is ignored for determining min-content/max-content sizes of flex containers, we only care about the min-content/max-content contributions of the item's contents.
  • Treat width/height, but not flex-basis as a floor on the max-content size.
  • Treat flex-basis, but not width/height, as a floor on the max-content size.
  • Take max(flex-basis, width/height) as a floor on the max-content size.

I'm not actually sure what's the best idea here.

@rachelandrew @jensimmons ?

@dbaron
Copy link
Member

@dbaron dbaron commented Aug 23, 2017

Should these be constraining the max-content contribution (to the parent) rather than the max-content size? Otherwise I think it breaks the max-content (etc.) values.

@fantasai
Copy link
Collaborator

@fantasai fantasai commented Aug 23, 2017

Yeah, it's the max-content contribution of the items / max-content size of the container we're concerned with here.

@css-meeting-bot
Copy link
Member

@css-meeting-bot css-meeting-bot commented Aug 23, 2017

The CSS Working Group just discussed Intrinsic sizing algorithm seems to produce 0 for many common cases, and agreed to the following resolutions:

  • RESOLVED: Change the spec to treat width and height as input for the max content size
The full IRC log of that discussion <dael> Topic: Intrinsic sizing algorithm seems to produce 0 for many common cases
<dael> Github topic: https://github.com//issues/1435
<fantasai> Options: https://github.com//issues/1435#issuecomment-321098718
<dael> fantasai: There is an issue filed by dholbert pointing out max-content of a flex container is only defined by max content size of its content and flex-basis isn't taken into account. Empty elements in flex with a basis it shrinks to 0.
<dael> fantasai: dholbert pointed out this is unintuitive. We did give it a size
<dael> fantasai: We have options here. No one has a good arguement for one over another. We need WG or community opinion. We could do nothing and leave as-is. We could treat specified width or height as a size that needs honor and ignore flex-basis. WE could do flex-basis and ignore width and height. And this would all be inflated by max-content.
<dael> fantasai: If I was going to pick randomly I would say use width and height but not flex-basis
<dael> Bert: Sounds like it's not a serious problem, but I may be misreading. Any opinions?
<dael> gregwhitworth: I'd agree with width and height but not flex-basis. My only worry is, I don't expect people to hit this on web but they prob wouldn't know they have empty flex.
<dael> gregwhitworth: Any compat issues?
<dael> rachelandrew: As an author it's not an issue I"ve seen.
<dael> rachelandrew: I'd prob agree width/height not flex-basis makes sense.
<dael> Bert: That's one in support. Other opinions?
<dael> Bert: Other options...flex-basis as well as width/height and take the max, they are less desireable. Did ne not want to discuss those?
<dael> fantasai: flex-basis is a start place for flexing, it's an input. Width and height are set. In that case you're setting something explicit and everywhere else in CSS we take that.
<dael> Bert: Anybody want to argue in favor of flex-basis or max of that and width?
<dael> Bert: Maybe we drop those.
<dael> Bert: So only no change or look at width/height
<myles> present+: myles
<dael> Florian: Sounds like a good change, but is anyone rushing to impl?
<dael> fantasai: I'm not sure but dholbert is planning to look at flex box.
<dael> gregwhitworth: Looking at the test cae we have interop. It's a worthwhile change, we should make it.
<dael> Bert: Okay, they we need buy in from more impl.
<dael> Florian: We have 2 at least. That's a good start.
<dael> Bert: Is TabAtkins on?
<dael> fantasai: He's just IRC.
<bradk> Is there existing content that depends on current interop behavior?
<dael> Bert: I'm hearing some support for looking at width/height but not flex-basis which is a change in spec.
<dael> fantasai: Yes.
<dael> Bert: Do we put that up? Change the spec to treat width and height as input for the max content size
<dael> Bert: Who is in favor, who is against?
<dael> Florian: In favor
<rachelandrew> +1
<dael> Bert: I only hear in favor.
<dael> RESOLVED: Change the spec to treat width and height as input for the max content size
<dael> Bert: fantasai will you write text? or TabAtkins ?
<dael> Bert: Whose action?
<TabAtkins> We'll take care of it, don't worry
<dael> fantasai: We'll do it, don't worry.
<dbaron> I guess I'm surprised they weren't already an input to the max-content contribution, given that they're an input for other layout modes.
<TabAtkins> As long as it shows up in the issue
@fantasai
Copy link
Collaborator

@fantasai fantasai commented Sep 6, 2017

Edits checked in! @dholbert, would please let me know if they are satisfactory. :)

@astearns
Copy link
Member

@astearns astearns commented Sep 27, 2017

Reopening for testcase need.

@astearns astearns reopened this Sep 27, 2017
@tabatkins tabatkins closed this Sep 29, 2017
@fantasai fantasai added this to the Published css-flexbox-1 2017-10-19 milestone Mar 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.