[css-grid] Grid items should only stretch the inline direction for justify-self:stretch

RESOLVED FIXED in Firefox 45

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mats, Assigned: mats)

Tracking

Trunk
mozilla45
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Grid items should only stretch the inline direction for justify-self:stretch
but items that were blockified and became block frames stretch regardless
of justify-self value because that's the default behaviour for block frames
in the inline direction (until they also support the css-align properties).

So, for a grid item with a 'justify-self' value other than 'stretch'
we need to trigger shrink-wrap behavior on its block frame when we
reflow it.
(Assignee)

Comment 1

3 years ago
Created attachment 8687313 [details] [diff] [review]
fix
Attachment #8687313 - Flags: review?(dholbert)
Comment on attachment 8687313 [details] [diff] [review]
fix

Review of attachment 8687313 [details] [diff] [review]:
-----------------------------------------------------------------

Just one nit on the commit message:
> Bug 1224634 - [css-grid] Grid items should only stretch the inline direction for justify-self:stretch. r=dholbert

The application of "only" here is ambiguous & easy to misread.  I initially read this as "only stretch the inline direction [i.e. not block direction]", which confused me.   Really, you mean "only... for justify-self:stretch. [not other justify-self values]"

Please reword to avoid this ambiguity. One possible rewording:
> Bug 1224634 - [css-grid] Make grid items shrink-wrap when
> measuring their size, unless they have justify-self:stretch. r=dholbert

(or whatever you think)
Attachment #8687313 - Flags: review?(dholbert) → review+
Comment on attachment 8687315 [details] [diff] [review]
test tweaks

Review of attachment 8687315 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/css-grid/grid-col-max-sizing-max-content-001.html
@@ +17,5 @@
>   grid-auto-columns: minmax(min-content,max-content);
>   border: dashed blue;
>   float:left;
>   clear:left;
> + justify-items: stretch;

Why do we need to add this declaration in all of these tests?

"justify-items: auto" (the default) should compute to "stretch" on a grid already, according to https://drafts.csswg.org/css-align-3/#propdef-justify-items.  Maybe I'm misreading something, though? Or, does that not work correctly right now?
(Assignee)

Comment 5

3 years ago
> Why do we need to add this declaration in all of these tests?

Not really no.  I figured I should have some 'justify-items: stretch' and some
'justify-self: stretch' just for testing.  I'll remove one the declarations here
so we test the 'auto' behavior too.
OK, sounds good.
(Assignee)

Updated

3 years ago
Flags: in-testsuite+

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2906aec76e64
https://hg.mozilla.org/mozilla-central/rev/5e66054666ac
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.