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] Flex basis and box-sizing #316

Closed
fantasai opened this issue Jul 12, 2016 · 14 comments
Closed

[css-flexbox] Flex basis and box-sizing #316

fantasai opened this issue Jul 12, 2016 · 14 comments
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-flexbox-1 Current Work Needs Testcase (WPT) Tracked in DoC

Comments

@fantasai
Copy link
Collaborator

fantasai commented Jul 12, 2016

I noticed a problem while working with flexbox last week:

<div style="display: flex">
    <div style="flex: 2"></div>
    <div style="flex: 1"></div>
</div>

If we add padding or borders, the flex items are not actually distributed at thirds according to the algorithm, even if box-sizing is set to border-box. This is in direct contradiction to our examples/descriptions in https://www.w3.org/TR/css-flexbox-1/#flex-common

The way things are, it's not possible to get things to align unless you do some fancy math with the padding and borders... the avoidance of which is exactly the point of having the box-sizing property. I'm somewhat afraid it's not possible to fix this anymore. :( But for sure it's got to be annoying for authors...

@fantasai fantasai added the css-flexbox-1 Current Work label Jul 12, 2016
@tabatkins
Copy link
Member

Yeah, it would be nice if flex would work with the box-sizing property, but it does complicate things; the border+padding would have to act as an additional min-size constraint, effectively.

I'm also very unsure whether it's compatible to make such a change now. Perhaps we could add a property switch that makes it pay attention?

@fantasai
Copy link
Collaborator Author

fantasai commented Mar 15, 2017

The original intent of flex: <integer> was to specify flexibility as absolute ratios... we're failing at that if we don't/can't make it work!

@fantasai
Copy link
Collaborator Author

fantasai commented May 2, 2017

@dholbert @cbiesinger @atanassov Comments here? Is this something we can fix?

@tabatkins
Copy link
Member

In particular, would it be okay to make the necessary edits so that the following:

.flexbox {
  display: flex;
  width: 300px;
}
.flex-item {
  box-sizing: border-box;
  border: 20px;
}
.flex-item:nth-child(1) {
  flex: 1;
}
.flex-item:nth-child(2) {
  flex: 2;
}

results in the first item being 100px wide (20px border, 60px content, 20px border) and the second item being 200px wide (20px border, 160px content, 20px border)?

Today you instead, unfortunately, get the first item being 113px wide (20px border, 73px content, 20px border) and the second being 186px wide (20px border, 146px wide, 20px border), because they're distributing the 220px of free space in a 1:2 ratio.

@FremyCompany
Copy link
Contributor

Adding myself to the thread, but given how many websites are using flexbox today combined with a framework that sets border-box on all elements by default, this breaking-change seems unlikely to be webcompatible to me.

Maybe adding flex-sizing: border-box | content-box | ... would work. Future frameworks could set it on by default. This isn't exactly a high-priority problem to solve though...

@fantasai
Copy link
Collaborator Author

True, but I think in most cases the current behavior is either unintended or unnoticed.

I think it's worth fixing if we can. This is the one place we screw up and effectively don't use border-box sizing even when you've specified it.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Absolute flex ratio, and agreed to the following resolutions:

  • RESOLVED: box-sizing is accounted for in the flex algorithm so that the flex ratios reflect the distribution of the specified box
The full IRC log of that discussion <gregwhitworth> Topic: Absolute flex ratio
<astearns> github: https://github.com//issues/316
<fantasai> https://drafts.csswg.org/css-flexbox/issues-cr-20160526#issue-15
<gregwhitworth> TabAtkins: one of the original things with flex, was relative & abs flexing
<gregwhitworth> TabAtkins: absolute flex allowed you to setup a ratio of flexing. That works great as long as your margin/border/padding are 0
<gregwhitworth> TabAtkins: it's impossible to do this because flexing always increases the content box size
<gregwhitworth> TabAtkins: the total size of the elements end up not matching the ratio *gave an example*
<gregwhitworth> TabAtkins: change the ratio to take box sizing into account
<gregwhitworth> fantasai: they are actually, they take it into account but not correctly
<gregwhitworth> fantasai: we use the margin box, so your starting point will be bigger than 0
<gregwhitworth> fantasai: so we have to take margin/border/padding into account
<gregwhitworth> Florian: is this web compatible
<dbaron> Florian: and box-sizing doesn't have 'margin-box'
<gregwhitworth> TabAtkins: border is the important one here
<gregwhitworth> TabAtkins: we think this is web compatible, the ratios are probably still close enough that they're not noticing differences right now
<gregwhitworth> fantasai: they set box-sizing on *
<gregwhitworth> TabAtkins: if it turns out that this is not web compatible we can come up with a different solution
<gregwhitworth> TabAtkins: but we'd like to try and use that first rather than introduce new things
<gregwhitworth> TabAtkins: we wanted working group approval
<gregwhitworth> fantasai: I think a lot of these sites are slightly off and will probably be slightly improved
<gregwhitworth> Rossen: do we have interop
<gregwhitworth> TabAtkins: we do
<gregwhitworth> TabAtkins: but it would be a better improvement, hopefully
<gregwhitworth> dbaron: who's going to make the change first?
<gregwhitworth> dbaron: I think I'd like someone to be the first implementation as we are normally hesitant
<fantasai> Florian proposes the browser with dominant market share
<fantasai> TabAtkins: guaranteed not to affect how boxes wrap or are arranged, just slight difference in how they are sized
<gregwhitworth> astearns: those with web compat concerns, any objections
<gregwhitworth> TabAtkins: I'm pretty confident that I can volunteer Christian to try it
<gregwhitworth> astearns: any objections?...
<gregwhitworth> Rossen: well - yes
<gregwhitworth> Rossen: I would like to believe it, but I'd hate to have you start flooding this and then they fix it and they do, and then we change our mind later on then
<gregwhitworth> fantasai: I'm pretty confident this is the right thing to do for the web
<dbaron> s/first?/first? We have a record of making changes that are possibly risky and then nobody willing to be the first implementer./
<gregwhitworth> Rossen: I have no problem with anyone trying it
<gregwhitworth> fantasai: I think we need the spec to change for anyone to do it
<gregwhitworth> Florian: put a note in there to provide feedback on it
<gregwhitworth> TabAtkins: we've done that before, implementations please test is it and send it back
<gregwhitworth> Rossen: with a note would be better
<gregwhitworth> PROPOSED RESOLUTION: Include box-sizing in flex sizing with a note
<fantasai> RESOLVED: box-sizing is accounted for in the flex algorithm so that the flex ratios reflect the distribution of the specified box
@fantasai
Copy link
Collaborator Author

fantasai commented Aug 2, 2017

CSSWG resolved to try to fix this! Blink team has volunteered to try implementing; if they can pull it off without breaking anything, other vendors will follow.

@fantasai fantasai added Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. and removed Needs Edits labels Sep 6, 2017
@astearns
Copy link
Member

Reopening for testcase need.

@astearns astearns reopened this Sep 27, 2017
@fantasai fantasai added this to the Published css-flexbox-1 2017-10-19 milestone Mar 29, 2018
@MartinMa
Copy link

Just to be clear. The behavior has not change since this issue report right? The example given by @tabatkins still leads to a distribution of 113px and 187px with a 300px-wide flexbox with two border-box-enabled items that have a border of 20px in a flex ratio of 1:2.

I made a quick pen to demonstrate the issue again:
https://codepen.io/MartinMa/pen/bGdMrjx

The spec reads

When determining the flex base size, the item’s min and max main sizes are ignored (no clamping occurs). Furthermore, the sizing calculations that floor the content box size at zero when applying box-sizing are also ignored. (For example, an item with a specified size of zero, positive padding, and box-sizing: border-box will have an outer flex base size of zero—and hence a negative inner flex base size.)

So can we agree that box-sizing: border-box is essentially ignored?

@jonjohnjohnson
Copy link

jonjohnjohnson commented Mar 7, 2022

@cbiesinger I can't find any ticket in Blinks tracker that relates to the resolution/commit discussed here.

Since @tabatkins mentioned volunteering your efforts for the change in Blink and you've recently worked on flex/box-sizing interactions with aspect-ratio (https://bugs.chromium.org/p/chromium/issues/detail?id=1166834 / ede56bd#r40863567), I'm wondering if you have any ideas on the state of this issue?

cc @fantasai

@rinarakaki
Copy link

Any update?

@cbiesinger
Copy link

cc @davidsgrogan , I haven't worked on flex in several years now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-flexbox-1 Current Work Needs Testcase (WPT) Tracked in DoC