Closed Bug 939905 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: dholbert, Assigned: dholbert)

References

()

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 1 obsolete file)

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
Duplicate of this bug: 880258
Duplicate of this bug: 836881
Keywords: dev-doc-needed
OS: Linux → All
Hardware: x86_64 → All
Attached patch fix v1 (obsolete) — Splinter Review
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)
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.
Ah, that looks great - thanks! I'll give that a shot tomorrow. Looks like it should simplify things a good bit.
Attachment #8334364 - Attachment is obsolete: true
Attachment #8334819 - Flags: review?(cam)
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+
(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.)
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+
Priority: -- → P3
https://hg.mozilla.org/mozilla-central/rev/872e64c93fda
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Duplicate of this bug: 971145
You need to log in before you can comment on or make changes to this bug.