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-multicol] Shouldn't column-fill: auto take min-height into account? #3064

Closed
benface opened this issue Aug 29, 2018 · 17 comments
Closed
Labels
Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-multicol-1 Current Work

Comments

@benface
Copy link

benface commented Aug 29, 2018

I was playing with the column-fill property in different browsers, and I noticed something interesting. It seems a height is required on the multicol element for column-fill: auto to take effect (with an automatic height, the columns are always balanced), but I feel like a min-height should be enough. In fact, due to the way it is now, it's not safe to use column-fill: auto because it requires a fixed height, which means text can be clipped if that height is not large enough to display all of it.

To show you what I'm talking about, check out this editable example: https://www.w3schools.com/cssref/tryit.asp?filename=trycss3_column-fill

Resize your browser window down until you can't see some of the text (due to the height of the div being restricted to 100px). Now change the height: 100px; declaration to min-height: 100px in order to fix that, and column-fill: auto loses its effect (the browser has to be large enough to notice the effect in the first place). Reproduced on Safari 11.1.2 and Chrome 68 which I'm guessing follow the spec, whereas Firefox 61 behaves weirdly and only fills the first column and leaves the other two empty when there's a min-height.

So, my question is: shouldn't column-fill: auto take min-height, or rather the computed height rather than only the height property, into account?

@fantasai
Copy link
Collaborator

Makes sense to me.

@mstensho
Copy link

If min-height is large enough to fit all column content, I too think it makes sense to disable balancing if column-fill is auto. One complication here, though, is that we still need to resort to balancing, if it turns out that min-height isn't large enough. The demo above is a good example.

Or this:

<p>There should be a green square below.</p>
<div style="width:300px; columns:3; column-gap:0; column-fill:auto; min-height:200px;">
  <div style="height:900px; background:green;"></div>
</div>

Test for min-height that's large enough (failing in all browsers, I guess, but it's the spec change we'd want):

<p>There should be a green square below.</p>
<div style="width:300px; columns:3; column-gap:0; column-fill:auto; min-height:200px;">
  <div style="height:400px; background:green;"></div>
</div>

@rachelandrew
Copy link
Contributor

That makes sense to me. If an author has given a combination of min-height and auto that does seem to infer that they want sequentially filled columns where - assuming enough content - the first one at least will be that min-height. If, though that min-height is reached and all columns are full then use balance.

@AmeliaBR
Copy link
Contributor

I strongly agree that this is a sensible use case, and would help avoid the bad typography of ultra short columns when you don't have a lot of content.

The downside is that it requires running the column-breaking code twice. But it's not an infinite loop, and only the fragmentation code needs to be re-run, not a full layout.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Shouldn't column-fill: auto take min-height into account?.

The full IRC log of that discussion <dael> Topic: Shouldn't column-fill: auto take min-height into account?
<dael> github: https://github.com//issues/3064
<dael> fantasai: If you have colum-fill:auto it doesn't take effect if you have auto height multicol container. Goal of auto is fill first column then move rather then balance. Fixed height or page we can fill.
<dael> fantasai: Commentor said if height works, why not fill up to min-height?
<dael> fantasai: Reasonable to me. rachelandrew agrees
<dael> florian: Max is reasonable, but why min?
<dael> florian: Why would you stop at a min.
<dael> fantasai: Because at the min you balance
<dael> dbaron: If you have more than that? You've filled all columns at min and you have more, do you go back and do it again?
<dael> rachelandrew: Yes, I think so.
<dael> dbaron: Seems like a very special case. I don't feel it's work adding the complexity of redoing it a second way because someone suggested it's nice
<dael> myles: second that
<dael> florian: I'm a big fan of multicol being smarter, but I'm not convinced on this
<dael> fantasai: What's happening now is if you have a min-height and you say auto it will balance and not fill the column. You have all this extra space because you have a min-height. Was max-height you don't necessarily have extra space.
<dael> florian: What do you mean you balance?
<dael> astearns: Useful for case where there is not enough content when columns balanced to meet min-height. In that case all columns don't fill min-height. CHange is some columns would fill min-height. Too much content, though, is a second pass
<dael> rachelandrew: Request makes sense. If it's worth impl is different issue. Expectation makes sense.
<dael> florian: I need to re-read. astearns explanation sounded different
<dael> astearns: I might be wrong, that's my take
<dael> dbaron: I get the model, but having trouble imaging the design were someone wants this
<dael> fantasai: Can go back to commentor and ask for more info
<dael> rachelandrew: Using min-height we haven't seen much because only float was available. Now that we have grid people thinking about layout different. Using min-height is common and powerful. If there's more content I want to use min-height because I don't want ti to be smaller then viewport.
<dael> rachelandrew: At first glance this looks like a powerful use case that would be helpful. But I've been looking for 2.5 minutes
<dael> dbaron: Broader then min-height?
<dbaron> s/?/? whenever the height comes from something else?/
<dael> florian: Original comment was more shouldn't make auto take computed height?
<dael> fantasai: Question is shouldn't column-fill auto...okay, yea.
<dael> dbaron: Another example is if you have a multicol whose height is stretched should column-fill: auto work? I think not right now
<dael> rachelandrew: As an author I want it to workt he same everywhere
<dael> rachelandrew: If the height is created by a grid track or a min-height...al the many ways to create height. They should all look the same
<dael> dbaron: More sympatetic for a general approach then just min-height
<dael> florian: Same here
<dael> ??: Makes sense
<dbaron> s/??/rachelandrew/
<dael> astearns: rachelandrew do you think there's enough for you to work on this?
<dael> rachelandrew: I'll think about it. Maybe discuss at F2F?
<dael> astearns: A whiteboard would be good for this
<dael> rachelandrew: I'll have athink on this
<dael> astearns: Let's add F2F Agenda tag to this

@frivoal
Copy link
Collaborator

frivoal commented Oct 17, 2018

I'm failing to find the part of the spec that says that a height is required on the multicol element for column-fill: auto to take effect. I may very well be missing something, but it seems to me that column-fill: auto is already supposed to fill column sequentially regardless, and if the height of the column is not constrained, then the first column gets all the content.

(with an automatic height, the columns are always balanced)

These does not seem to be required by the spec, and while Chomre/Safari seem to do that, Firefox does not.

Example 27 in the spec is also trying to show that the first column should get all the content when both height and column-fill are auto (although I don't think it is conclusive due to interference from the widow and orphan properties, and only having 3 lines).

So, with that, it seems to me that letting min-height expand the auto calculated height of the column that needs to be filled is a work-around a webkit/blink bug, and that the real solution is to do it the way Firefox does it.

Am I missing something?

@rachelandrew
Copy link
Contributor

I don't think it is properly defined in the spec, no. So whichever way this is decided I think there are edits to be made to ensure it is clear.

@mstensho
Copy link

From the CR version of the spec (which is what WebKit, Chromium and Edge implement, and also what happens to still be stuck in my brain):

https://www.w3.org/TR/2011/CR-css3-multicol-20110412/#column-fill

In continuous media, this property [column-fill] will only be consulted if the length of columns has been constrained. Otherwise, columns will automatically be balanced.

I don't know where that wording went, but it's not in the current version of the spec.
Such an important behavior change should be in the changelog, but I can't find it. I don't think the spec is in a good state now. We're still supposed to force balancing before spanners, but not if height is unconstrained? Not forcing balancing if height is unconstrained makes little or no sense in the first place, in my opinion, but if we really want to go there, it should apply before spanners, too. I think there's still time to turn back (or make improvements), since nobody has apparently updated their implementation yet.

But I digress. :)

With the current version of the spec (not supported by anyone, except Gecko's ongoing implementation work?), this is actually a non-issue. But do we really want the spec to say what it currently does?

@rachelandrew
Copy link
Contributor

@mstensho I've just had a look and discovered that text is commented out in the spec, and still exists, see line 1059. Digging back to before I started editing, it was certainly commented out as far back as 2015 when the spec was converted to Bikeshed, so that is a long way before my time. Some digging through the archives might show something up, or perhaps we figure out what we actually want this to say and make sure it is said.

Please raise as issues anywhere else you feel the spec isn't in a good state, that would be really helpful.

@frivoal
Copy link
Collaborator

frivoal commented Oct 18, 2018

@mstensho aha, that explains a lot of the confusion.

I don't know the history of that sneaky change, but setting that aside, I think what we should spec is:

  • always balance before spanners.
  • when column-fill is auto, only fill columns other than the first (after spanners) if there is a forced break or if constrained by the used value of the height
  • the min content intrinsic size in the block direction is the same for column-fill:auto and column-fill:balance
  • The max content intrinsic size in the block direction for column-fill:auto is the height you get when (after any spanners and their preceding balanced content) if you only wrap to the next column at forced breaks.

That seems to be the more useful behavior to me.

Now, that does not match what Chrome/Safari/Edge implement and what the old CR say, but it does (at least superficially, I have not looked too close) what Firefox does and what the new ED suggests (even though it's not very specific). I am not 100% sure what that implies in terms of web compat, but I'd like to be optimistic and hope that having both behaviors means we're not too constrained.

@mstensho
Copy link

@rachelandrew Håkon made this change in 2012: e0a990c (commented out the paragraph).

I have raised issue #3224 to fix column-fill.

@rachelandrew
Copy link
Contributor

I believe that this issue is covered when I make the edits for #3224 where we have resolved to take min and max constraints into account. Please let me know if not.

@BezPowell
Copy link

I'd just like to chime-in here and say that as a dev while I prefer the way that Firefox currently handles this I'd rather not have to set max-height to get columns to wrap. Most of the sites I work on being designed for less tech-savvy people, for me, the ideal situation would be that if I could set the styles on the container to something like:

column-count: 4;
column-fill: auto
min-height: 20em;

and any content would:

  1. Fill a single column starting from the first until it reaches the min-height value.
  2. Start filling the next column as soon as the current column has reached 20em.
  3. Start balancing as soon as all 4 columns have reached min-height.

That way if the client only pastes in two sentences it will only fill 1 column. Or if they happen to insert the complete works of Shakespeare it will have 4 balanced columns without overflowing the container.

My only worry with having additional columns only start to fill when max-height has been reached will be that if the container happens to have a novels worth of text in it you'll get serious layout problems for anything following it and if no max-height is set all text will be forced into one very tall column.

Hopefully that makes sense, and if I have simply misunderstood the conversation so far and that is what has already been decided upon feel free to ignore me!

@rachelandrew
Copy link
Contributor

@BezPowell I think the edits based on the resolution in the linked issue #3224 will give you want you want (once browsers implement them).

@rachelandrew
Copy link
Contributor

@benface I believe that the edits closing #3224 have addressed your original concern. Could you let me know?

@benface
Copy link
Author

benface commented Mar 23, 2019

Yes, looks like they have @rachelandrew! Thank you very much. :)

@rachelandrew rachelandrew added the Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. label Mar 23, 2019
@rachelandrew
Copy link
Contributor

Closing as fixed in #3224

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-multicol-1 Current Work
Projects
None yet
Development

No branches or pull requests

9 participants