[webkit-reviews] review granted: [Bug 82128] Implement new flex property and deprecate flex function : [Attachment 135412] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Tony Chang <tony at chromium.org>'s
request for review:
Bug 82128: Implement new flex property and deprecate flex function
https://bugs.webkit.org/show_bug.cgi?id=82128

Attachment 135412: Patch
https://bugs.webkit.org/attachment.cgi?id=135412&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=135412&action=review


> Source/WebCore/rendering/RenderFlexibleBox.cpp:348
> +Length RenderFlexibleBox::preferredFlexLengthForChild(RenderBox* child)
const

Not a huge fan of this name. I feel like it could easily be confused with the
length after we've done the flex algorithm. How about
mainAxisPreferredLengthForChild?

> Source/WebCore/rendering/RenderFlexibleBox.cpp:650
>  float RenderFlexibleBox::positiveFlexForChild(RenderBox* child) const
>  {
> -    return isHorizontalFlow() ? child->style()->flexboxWidthPositiveFlex() :
child->style()->flexboxHeightPositiveFlex();
> +    return child->style()->positiveFlex();
>  }
>  
>  float RenderFlexibleBox::negativeFlexForChild(RenderBox* child) const
>  {
> -    return isHorizontalFlow() ? child->style()->flexboxWidthNegativeFlex() :
child->style()->flexboxHeightNegativeFlex();
> +    return child->style()->negativeFlex();
>  }

Don't necessarily need to do it in this patch, but these helper functions don't
seem terribly useful to me.