[css-grid] Stretching image grid items in both dimensions #523
Comments
|
We were discussing that yesterday at TPAC (@fantasai, @jensimmons and me). |
|
Well, actually manually setting the width won't cause the aspect ratio to be observed: it just sets the width in that dimension, and since the initial value of align-self is Squishing images seems a little odd, but as a starting point it's useful: the |
|
(Plus it's consistent with how blocks behave so not surprising at all.) |
MatsPalmgren
commented
Sep 22, 2016
•
|
@mrego I don't think that solution works, because setting width:100% My proposal doesn't have that problem. |
MatsPalmgren
commented
Sep 22, 2016
|
@fantasai, I don't think object-fit:contain works either since |
|
Cover example, image proportionally sized and clipped to fit:
Contain example, margin/border/padding fitted to grid slot and image proportionally sized into it:
Contain example, margin/border/padding fitted to proportionally scaled-down image:
Contain example with future syntax, margin/border/padding fitted to proportionally scaled-down image:
|
MatsPalmgren
commented
Sep 25, 2016
•
As you say, this leads to the image being clipped and is thus
This leads to the image overflowing when the window is very wide
Suggesting magic future tech to solve this problem isn't serious.
I've made a demo of I see the following problems:
My proposal doesn't have any of the above problems and it provides |
dauwhe
added the
css-grid-1
label
Sep 26, 2016
dbaron
added the
Agenda+
label
Sep 27, 2016
MatsPalmgren
commented
Sep 27, 2016
|
To be clear, what I'm proposing is that spec says something like this
|
|
(It's possible you might want that rule to be conditional on the element having |
cbiesinger
commented
Sep 28, 2016
|
It seems that the proposed text is a bit underdefined in terms of how min and max sizes interact with the "ratio-preserving way". Also I'm not sure that the current definition of min sizes in grid actually allow you to preserve the aspect ratio if the image is too big in both axes and does not have a width or height explicitly specified on the image. |
MatsPalmgren
commented
Sep 30, 2016
There are some pros and cons with that, but I don't have a preference. |
Presumably using the rules in CSS2 section 10.4 that apply to this case. |
|
Just discussed in today's teleconference (I'll try to link minutes later). The conclusion is that we can change the mapping of |
MatsPalmgren
commented
Oct 5, 2016
•
|
OK, so |
MatsPalmgren
commented
Oct 6, 2016
•
|
I really like the proposed solution to make Could you please clarify which behavior you want when the *-self value I think it should fill the grid area in the axis that has |
astearns
removed the
Agenda+
label
Oct 11, 2016
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this issue
Nov 5, 2016
|
|
MatsPalmgren |
49f3373
|
xeonchen
pushed a commit
to xeonchen/gecko
that referenced
this issue
Nov 5, 2016
|
|
MatsPalmgren |
9afdc53
|
@fantasai / @tabatkins, I think this ^ is a question for you. Mats' suggestion here makes sense to me, FWIW (and I can't come up with anything that's more sensible). |
KuoE0
pushed a commit
to KuoE0/gecko-dev
that referenced
this issue
Nov 10, 2016
|
|
MatsPalmgren |
dd1a9c6
|
freesamael
pushed a commit
to freesamael/gecko-dev
that referenced
this issue
Nov 15, 2016
|
|
MatsPalmgren |
aff3dc7
|
fantasai
added a commit
that referenced
this issue
Nov 24, 2016
|
|
fantasai |
10aa992
|
|
Checked in an attempt at fixing this per above discussion. :) Please let me know if anything is askew. I took Mats's suggestion for handling Wrt basing on |
fantasai
added Closed Accepted by CSSWG Resolution Agenda+
labels
Nov 24, 2016
|
Agenda+ for certification of edits and the details wrt |
|
Oh, just wanted to check that we're all understanding that |
MatsPalmgren
commented
Nov 26, 2016
|
No, I would summarize what we have implemented as:
IOW, the intrinsic ratio is preserved if at least one of the values is It gets more complicated when one or both axis also has min-/max-size constraints. I think we're following the CSS2 rules[1] there -- we're re-using the code that already implements that. The stretching occurs before applying those rules. Also, when the track max-sizing functions are definite, clamping the item to fit the grid area (per §6.6 in CSS Grid https://drafts.csswg.org/css-grid/#min-size-auto) also affects the above rules (the "may overflow..." above should not be allowed when clamping in that axis IMO, but let's sort out the clamping effect on this in #767) Here's a couple of testcases to illustrate our stretching behavior (I intentionally Testcase for shrinking an image to fit a smaller grid area: (You need a Nightly build for these tests to display correctly: https://nightly.mozilla.org/ ) |
|
OK, so, what I want to point out is that from an authoring point of view, the intrinsic aspect ratio should be preserved unless there's specific instructions not to. Maintaining the intrinsic aspect ratio trumps preserving the intrinsic size in any single dimension. Always. Changing a dimension effectively modifies the user-perceived "intrinsic size" in the other. Internally, we can define terms however we want, but I'm pretty sure that as far as the author is concerned, anywhere that we are failing to do this is an error. An alignment value of To address your feedback that the default behavior should be to preserve the aspect ratio, we decided to add an independent behavior for p.s. Fwiw, I'm a little concerned that the new |
MatsPalmgren
commented
Nov 29, 2016
•
|
What you are suggesting is that That seems a lot less useful to authors, IMHO. As you can see in the testcase: I think your suggestion is a big mistake and I would like to hear what others think about this before changing our implementation. Web developers in particular. |
yisibl
commented
Dec 1, 2016
|
I agree with @MatsPalmgren . In any case, we should not rely on |
yisibl
commented
Dec 1, 2016
•
|
I think this would be a regular use by developers, centering and preserving the aspect ratio(demo). Equivalent to: grid > img {
width: 100%;
max-height: 100%;
object-fit: cover; // or contain
}And I was surprised to find that Chrome was not in the same height(line-height), |
Yes, this makes sense to me having just re-read this discussion. As an author you don't expect the ratio of an image to be changed unless you specifically request that to happen. I can see situations occurring where in development test data doesn't highlight the ratio destroying behaviour. However once the site is live and real data is added via a CMS etc. images start to look weirdly stretched. By making the default ratio preserving then in that scenario images might overflow a box or have unwanted gaps but I think that is closer to how authors expect things to behave. |
|
Just trying to clarify the issue, the part in which there is disagreement is in the difference between By default, everything is But for
Note that if you use Also see the difference in the example above, using the current implementation by Firefox that follows option 2): |
rchrdnsh
commented
Dec 1, 2016
|
I think I support @fantasai's proposal, as I cannot think of any situation where I would want any default behavior to alter the dimensions of an image, unless I explicitly required this, using stretch on both the x and y axis. As a side note, I would like the ability to choose a focal point on an image that would center the image if the viewport in changed in size in any way smaller than the original image, while preserving the image dimensions and cropping any aspect of the image that falls outside of the viewport. Not sure I'm explaining that too well, though. thegrid.io adversities that it does this, although i have not used it in practice yet. But anyway, a +1 for @fantasai's suggestion :-) |
MatsPalmgren
commented
Dec 1, 2016
It seems to me you have misunderstood this issue. The default value is |
MatsPalmgren
commented
Dec 1, 2016
|
As a reminder, the current CSSWG resolution is supporting my view on this:
so that is what we will ship in Gecko, unless there is a new CSSWG resolution on the matter that says otherwise. |
|
The resolution was based on a discussion of the value specified in both dimensions. Prior to the introduction of 'normal' as distinct from 'stretch', it seemed to me quite obvious that specifying "align-axis: center" would not cause distortion, but cause the vertical axis to trigger "shrink to fit" behavior (which preserves the aspect ratio) and use the extra space for centering. I don't see why introducing special behavior for 'normal' -- special behavior whose sole purpose was to solve the default distortion brought on by the 'stretch stretch' combined default, should cause previously-existing combinations to change behavior. |
|
I also want to remind everyone that the purpose of the alignment properties is to manage alignment, not sizing. Stretch is a bit of an anomaly, but it does fit within the description of "specify what to do with the extra space after sizing". |
MatsPalmgren
commented
Dec 2, 2016
Fair enough, do you mind bringing it up again in a CSSWG meeting to discuss whether all values involving Fwiw, note that Firefox has always implemented ratio-destroying resizing for |
|
Sure, I'll bring that up. It didn't even occur to me that the "shrink-to-fit" sizing of the non-stretchy alignment values would be interpreted as aspect-ratio-destroying. (The only reason that 'stretch stretch' distorts the image is because you specifically requested a size: in CSS we otherwise always preserve the aspect ratio on auto-sized images, if we can.) Agenda+ to decide what happens if |
fantasai
added Agenda+ and removed Agenda+
labels
Dec 2, 2016
jensimmons
commented
Dec 7, 2016
|
I like Mats proposal better. The default is normal. I switch to stretch in the dimension where the image does not yet fill the space, and I get the result of breaking aspect ratio and having it stretch. If I want to 'fix' the aspect ratio, I can start telling the browser what I want it to do (likely with |
|
removing agenda+ because we discussed this on the last call and decided to keep the conversation going here. |
astearns
removed the
Agenda+
label
Dec 13, 2016
|
Ok, so Tab and I discussed this today and came up with the following conclusions. Things to consider:
Reasoning:
Proposal:
Related improvements that need to be made:
|
fantasai
added the
Agenda+
label
Dec 19, 2016
MatsPalmgren
commented
Dec 21, 2016
|
Is Chrome committed to making |
|
@MatsPalmgren |
MatsPalmgren
commented
Jan 4, 2017
|
Well, I wanted to know to be able to properly critique your current proposal to solve this issue. It's still not clear to me what the suggested "workaround" is to get the desired behavior. I don't see how |
MatsPalmgren
commented
Jan 9, 2017
|
So, to recap, the problem at hand is that stretching images in a ratio-preserving way would fill the grid area in only one axis but (likely) not the other and that an author might want to align (start/center/end) the image in the non-filled axis. IMHO, this seems like an edge case and a fairly minor problem. @fantasai's proposal to solve this problem is to not stretch images at all by default. This is a very drastic and invasive change to solve such a minor problem. I think that stretching all grid items to fill their grid area by default is a fundamental part of Grid layout and any proposed solution must preserve that. To single out images to not do that is inconsistent and unexpected. Also, I think your reasoning is flawed:
Well, there is a I agree with:
Yes, this is the problem at hand. Let's solve that in a reasonable way without destroying Grid layout completely. Here's my proposal to solve the align-the-non-filled-axis problem: The drawback with A is that it is two keywords and thus can't be used with the I think this solution is much more in proportion to this relatively minor problem. So, I'd like the CSSWG to reconsider my proposal that image grid items should stretch in a ratio-preserving way by default. Authors will expect all grid items to stretch by default, and as you agree, images are expected to preserve the ratio by default. The remaining alignment problem can be solved by adding some minor CSS Box Alignment feature as I suggested above (there are likely other Align solutions too). |
|
I also believe that this proposal has some issues:
|
MatsPalmgren
commented
Jan 10, 2017
|
@mrego Regarding Flexbox - see my original comment at the top. The CSSWG actually agreed at the time that it's undesirable to stretch images without preserving ratio, but there were some other issues in Flexbox that made it impossible to implement that. Those issues don't apply to Grid, which is why I think we can and should implement the optimal behavior here. |
MatsPalmgren
commented
Jan 10, 2017
|
@fantasai I have already thoroughly debunked the myth that |
astearns
removed the
Agenda+
label
Jan 24, 2017
|
Since we discussed this in Seattle I'm assuming it doesn't need to be on the agenda again so soon. |
|
The CSSWG discussed this issue again, and stands by the conclusion in #523 (comment) This ensures that the alignment properties do not devolve further into sizing controls, and that behavior in grid is consistent with behavior in block layout, etc. We will however add a @MatsPalmgren Wrt #523 (comment) I listed |
fantasai
added a commit
that referenced
this issue
Jan 24, 2017
|
|
fantasai |
f4ca22f
|
fantasai
closed this
Jan 24, 2017
triple-underscore
added a commit
to triple-underscore/triple-underscore.github.io
that referenced
this issue
Jan 25, 2017
|
|
triple-underscore |
d0e4c06
|
MatsPalmgren
commented
Jan 28, 2017
That's not what the CSWG actually decided according to the IRC log at https://log.csswg.org/irc.w3.org/css/2017-01-13/#e761792
@fantasai's proposal above says "things with an aspect ratio" should use |
fantasai
added the
Agenda+
label
Feb 1, 2017
|
We actually did mean "things with an aspect ratio" - this was very clear from the discussion in-person, but didn't make it into the minutes very well. (Not helped by our continued use of "replaced element" as the term.) So we'll need the WG to amend the resolution to clarify that it was about things with an aspect ratio. Additionally, we then need to verify what behavior we want for replaced elements without aspect ratios. In particular:
|
MatsPalmgren
commented
Feb 3, 2017
|
Fwiw, I agree that an intrinsic size should also make |
|
Adding myself to the conversation since it seems we might change the spec again. Also, here is a test case:
|
FremyCompany
reopened this
Feb 3, 2017
|
Also variant https://jsfiddle.net/1fd948nz/1/ |
|
We amended the Seattle resolution on the call today: |
astearns
removed the
Agenda+
label
Feb 15, 2017
tabatkins
added a commit
that referenced
this issue
Feb 23, 2017
|
|
tabatkins |
98893bc
|
tabatkins
closed this
Feb 23, 2017
|
Initial draft of |
triple-underscore
added a commit
to triple-underscore/triple-underscore.github.io
that referenced
this issue
Feb 25, 2017
|
|
triple-underscore |
2821c88
|
|
I'm going to implement this on Chromium, just to be sure that I'm not missing anything, I understand this only applies to Grid Layout. So |
mrego
added a commit
to mrego/csswg-test
that referenced
this issue
Feb 27, 2017
|
|
mrego |
af8c629
|
mrego
referenced
this issue
in w3c/csswg-test
Feb 27, 2017
Merged
[css-grid] Add test for Grid Items Sizing and Alignment #1219
mrego
added a commit
to mrego/csswg-test
that referenced
this issue
Mar 1, 2017
|
|
mrego |
f8dfabb
|
MXEBot
pushed a commit
to mirror/chromium
that referenced
this issue
Mar 2, 2017
|
|
mrego + |
fad39c7
|
gsnedders
pushed a commit
to jgraham/css-test-built
that referenced
this issue
Mar 2, 2017
|
|
CssBuildBot |
2068174
|
FremyCompany
referenced
this issue
Mar 16, 2017
Open
[flex] Sizing images with intrisic aspect-ratio: ¿harmonize with grids? #1112
|
Just a ping to allow people to review issue 1112 and give an opinion there? |
|
Wrt conclusion stated in #523 (comment) ... here are the minutes. The key excerpt is that three solutions were considered for handling replaced elements:
The problem with option 3 is, as described in #523 (comment) , that what should be an alignment control devolves into a sizing control, which is objectionable. This left two options: 1 & 2. Both allow for all of the relevant behavior to be specified, they just differ on the default. Several arguments tilted in favor of option 1:
The WG therefore resolved on option 1, and actioned me and Tab to draft up a spec for @MatsPalmgren Sorry for taking so long to post a proper summary. Please let us know what you think, and if this is acceptable to you. For my part, I am OK with either 1 or 2, but I would have to object to 3. |
fantasai
added the
Commenter Response Pending
label
Mar 23, 2017
MatsPalmgren
commented
Apr 19, 2017
•
|
I still think image grid items should grow (or shrink) to fill their grid area while preserving their intrinsic ratio by default. I think
Opting out from that can be done by specifying If option 2 makes image grid items grow/shrink to fill their grid area in a "contain sizing" way by default, that's certainly acceptable to me. I'd be interested to review a more fleshed out proposal though... in particular how it interacts with |
fantasai
added Commenter Not Satisfied Agenda+ and removed Commenter Response Pending
labels
Apr 19, 2017
css-meeting-bot
commented
May 10, 2017
|
The CSS Working Group just discussed The full IRC log of that discussion<dael> Topic: Stretching image grid items in both dimensions<dael> Github Topic: https://github.com/w3c/csswg-drafts/issues/523#issuecomment-295075752 <dael> Rossen: I don't see fantasai yet. <zcorpan> Github Topic: https://github.com/w3c/csswg-drafts/issues/523 <dael> Rossen: Anyone from Mozilla want to handle this? Or do we move on? Current tag is customer not satisfied. <dael> TabAtkins: We should wait on fantasai because she might have more to the topic to explore. |
css-meeting-bot
commented
May 10, 2017
|
The CSS Working Group just discussed
The full IRC log of that discussion<dael> Topic: Stretching image grid items in both dimensions<TabAtkins> Yeah, I think that's probably the best error, zcorpan <dael> Github topic: https://github.com/w3c/csswg-drafts/issues/523 <dael> fantasai: In seattle we had three options. One we wouldn't take b/c violates design guidelines. Other two we would have contain & intrinsic size behavior. You could opt into either. We picked intrinsic for the default. There's details in https://github.com/w3c/csswg-drafts/issues/523#issuecomment-295075752 <dael> fantasai: We closed that, but Mats disagrees with the resolution. All other issues on Grid are closed as of when we stopped accepting issues. <dael> fantasai: This issue has a commentor disagreement so comes back to WG. Do we stick or do we change to contain sizing by default and opt-in for intrinsic via keyword-max-content or something like that. <dael> Rossen: Opinions? <dael> jensimmons: My opinion is I agree #3 should not be considered. I'd be okay with leave as-is, but I agree with Mats that as an author having the default be contain would be more handy and make more sense. We need both as an option and the tool to switch definitely. The default I'd lean contain. <dael> TabAtkins: Reason why we didn't go for contain is it puts sizing control in this prop... <dael> fantasai: TabAtkins we changed. <dael> jensimmons: That was 3 which we agreed not to do <dael> fantasai: Option was default is contian on all keywords, not jsut normal. <tantek> do we have comments from anyone else who is actually implementing this besides Mats? <tantek> reviews the issue <dael> jensimmons: Ideally we would have tool to switch between, but the wayt he world works if we set default to contain we would get the switching tools quicker. Not certainly, but might. <rachelandrew> I'm still keen on the resolution <dael> Rossen: But forcing a less then optimal default as a forcing tool i sn't doing much good. <tantek> long issue is long <dael> jensimmons: Agreed. I still like contain as a default. That's a secondary non-heavy <dael> TabAtkins: I'm concerned that when we have contain sizing....having different defaults based on layout of parent it makes the design confusing. It's better if we have consistancy. <dael> jensimmons: [missed] <Rossen> +1 to what Tab said <dael> fantasai: Flexbox resizes to fit hte container. It does only do that with a single line. If you have wrapping it takes intrinisic <dael> TabAtkins: It obeys constraint of flexbox and a bit of aspect ratio magic. The sizing is normal flexbox, not just images. <dael> jensimmons: You can make the same arguments here. <dael> TabAtkins: I disagree. Part of flex is doing this one thing. We're not adding a different way of sizing. <dael> jensimmons: I think from author prospective there would be something similar to having default of flexbox where they are both text and image contained. If there's an image just left with intrinisic size it starts to overflow and that's not typically what authors want. Authors expect content to stay in grid cells. <tantek> "won't spill outside" e.g. the [CSS is Awe]some problem <dael> fantasai: Things that are smaller then grid size, contain will cause them to size up. <dael> rachelandrew: I don't like the upscalling. I like the original resolution b/c i t's the most consistant and easier to explain. the upscalling will cause problems. <tantek> didn't follow Rachel's comment <gregwhitworth> +1 to TabAtkins rachelandrew <dael> fantasai: Another thing to consider is grid is being deployed and we don't have a definition of contains that WG approved. Changing will c ause impl stress. <dael> TabAtkins: Adding that, even if we had a contain definition, it's still transition stress because it's shipped without this. They're getting things to look well and then this change will break pages. <dael> fantasai: I would say if it was significantly better thing to do it might be worth doing, but since we're debating and there's good for both sides, given the status we're at it makes sense to go with intrinsic a nd add the contain keyword. That would be my position. <dael> TabAtkins: I agree. <rachelandrew> +1 to fantasai <dael> fantasai: I'm sympathetic, but it's not so clearly better that it's definitely the right answer. <dael> TabAtkins: I would also threat it differently if it was obvious fix instead of possibly better. <dael> Rossen: I'm seeing people favor option 1. Let's try for resolution. <dael> Rossen: Unless jensimmons you feel there's something else you want to add. <dael> jensimmons: I don't want to block it, but I think we need to define contain. <dael> fantasai: We have a definition, we need comments. THere is a definition in sizing 4. Mostly waiting for dbaron and Mats there. <dael> jensimmons: We should keep going forward on getting that done b/c it's desperately needed. <dael> dbaron: Does option 1 match what impl ship today? <dael> Rossen: I believe so. <dael> tantek: It does. <dael> s/tantek/ TabAtkins <dael> jensimmons: I'm curious about that. I'm not sure what Mats impl and he did Gecko. <dael> TabAtkins: I'd hope he's not impl something different. Even if he is, the other browsers aren't. <dael> dbaron: I would disagree with TabAtkins' hope. But I don't know what he did. <tantek> this issue feels like it has many aspects, are there at least some aspects we can converge on? <dael> Rossen: Let's go back to this. Sincec Mats' isn't here I'd liket o call for consensus on #1. We'll continue working on contain. <tantek> I'm having trouble keeping track of n resolutions over the past on this and n options <dael> Rossen: Obj to sticking with Seattle resolution: making the default instrinsic? <fantasai> testcase - <jensimmons> we don’t have interop on https://jsfiddle.net/1fd948nz/1/ at the moment, fyi <dael> RESOLVED: ticking with Seattle resolution: making the default instrinsic |
css-meeting-bot
removed the
Agenda+
label
May 10, 2017
|
Test case for the above discussion added by fantasai: |







MatsPalmgren commentedSep 22, 2016
I'm wondering about how we should stretch grid items that has
an intrinsic ratio, such as images. The Grid spec just refers to
css-align which says:
https://drafts.csswg.org/css-align-3/#justify-self-property
"the stretch keyword sets the box’s used size to the length necessary
to make its outer size as close to filling the alignment container
as possible while still respecting the constraints imposed by
min-height/min-width/max-height/max-width."
which I think means that having 'stretch' in both dimensions
will resize the image to fill the grid area without respecting
the image aspect ratio. I think this is a rather unfortunate
default behavior for Grid.
Some background; this topic was discussed for flex items here:
https://lists.w3.org/Archives/Public/www-style/2012Oct/0781.html
where everyone seems to agree that respecting the ratio is desirable,
but for various flex layout specific reasons, this could not be
achieved and it was decided to ignore the ratio (IIUC).
As far as I can tell, those reasons do not apply to Grid, so I see
no reason why we can't respect the ratio when stretching grid items
in both dimensions.
I think web authors generally prefer to preserve aspect ratios,
so that's what I think we should do for grid items.