Support "flex-flow" shorthand for multi-line flexbox

RESOLVED FIXED in mozilla28

Status

()

Core
CSS Parsing and Computation
P3
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

({dev-doc-complete})

Trunk
mozilla28
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Bug 702508 adds support for "flex-wrap" property.

With that, we can now support the "flex-flow" shorthand property (which sets both "flex-direction" and "flex-flow").

http://dev.w3.org/csswg/css-flexbox/#flex-flow-property
(Assignee)

Updated

4 years ago
Duplicate of this bug: 880258
(Assignee)

Updated

4 years ago
Duplicate of this bug: 836881
(Assignee)

Updated

4 years ago
Keywords: dev-doc-needed
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 3

4 years ago
Created attachment 8334364 [details] [diff] [review]
fix v1

heycam, would you mind reviewing?

Link to spec is in comment 0. (and as implied by comment 0, this layers on the patches from that bug as well.)

Quoting the spec, for convenience:
> Name:	flex-flow
> Value:	<flex-direction> || <flex-wrap>

flex-direction and flex-wrap are both enum-valued properties. The "||" means they can be provided in either order, and either of them may be omitted (but at least one has to be there).
Attachment #8334364 - Flags: review?(cam)
(Assignee)

Comment 4

4 years ago
Comment on attachment 8334364 [details] [diff] [review]
fix v1

Actually, I think I might need to tweak the inherit/unset/initial handling a bit. Canceling review request for the moment.
Attachment #8334364 - Flags: review?(cam)
While I've begun looking at the patch, and before you put a new one up, I think it would be good to use ParseChoice to parse the two optional components, once you've got past the initial/inherit/unset checking.
Actually, ParseChoice can handle the parsing of initial/inherit/unset, too.
(Assignee)

Comment 7

4 years ago
Ah, that looks great - thanks! I'll give that a shot tomorrow. Looks like it should simplify things a good bit.
(Assignee)

Comment 8

4 years ago
Created attachment 8334819 [details] [diff] [review]
fix v2 (using ParseChoice)
Attachment #8334364 - Attachment is obsolete: true
Attachment #8334819 - Flags: review?(cam)
(Assignee)

Comment 9

4 years ago
https://tbpl.mozilla.org/?tree=Try&rev=3141d4d15d1d
Comment on attachment 8334819 [details] [diff] [review]
fix v2 (using ParseChoice)

Looks great!  r=me with the understanding that bug 940229 will land soon and so we don't need to include invalid_values for initial/inherit/unset explicitly.
Attachment #8334819 - Flags: review?(cam) → review+
(Assignee)

Comment 11

4 years ago
(In reply to Cameron McCormack (:heycam) from comment #10)
> Comment on attachment 8334819 [details] [diff] [review]
> fix v2 (using ParseChoice)
> 
> Looks great!

Thanks!
> r=me with the understanding that bug 940229 will land soon and
> so we don't need to include invalid_values for initial/inherit/unset
> explicitly.

Yup. (I briefly added some invalid_values like that (which caused testfailures in my first patch), but I took it out after verifying that bug 940229 catches those issues just as effectively.)
(Assignee)

Comment 12

4 years ago
Landed this (and the other helper-bugs for multi-line flexbox) on inbound:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/872e64c93fda
Flags: in-testsuite+
(Assignee)

Updated

4 years ago
Priority: -- → P3
https://hg.mozilla.org/mozilla-central/rev/872e64c93fda
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28

Updated

4 years ago
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Updated

3 years ago
Duplicate of this bug: 971145
You need to log in before you can comment on or make changes to this bug.