Closed Bug 939905 Opened 7 years ago Closed 7 years ago
Support "flex-flow" shorthand for multi-line flexbox
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
OS: Linux → All
Hardware: x86_64 → All
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).
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.
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.
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
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.