[webkit-reviews] review granted: [Bug 230755] [css-flexbox] Improve & simplify the flex-basis computation : [Attachment 439157] Patch

[bugzilla-daemon at webkit.org]

Manuel Rego Casasnovas <rego at igalia.com> has granted Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 230755: [css-flexbox] Improve & simplify the flex-basis computation
https://bugs.webkit.org/show_bug.cgi?id=230755

Attachment 439157: Patch

https://bugs.webkit.org/attachment.cgi?id=439157&action=review




--- Comment #10 from Manuel Rego Casasnovas <rego at igalia.com> ---
Comment on attachment 439157
  --> https://bugs.webkit.org/attachment.cgi?id=439157
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=439157&action=review

r=me

>> Source/WebCore/rendering/RenderFlexibleBox.cpp:999
>> +class ScopedFlexBasisAsMainSize {
> 
> Wow, this is quite a hack I guess we don't have a better alternative though.
:-(
> 
> I'm not really sure about the class name, would be better
ScopedFlexBasisAsChildSize? Or something else, not sure...

Could we add some brief documentation about this class? Is it not very big, but
it's a kind of very special case, so it's worth explaining it for the future.

>>> Source/WebCore/rendering/RenderFlexibleBox.cpp:1040
>>> +	 // 9.3.2 E. Otherwise, size the item into the available space using
its used flex basis in place of its main size.
>> 
>> I miss C) and D) before getting to E). Maybe we could add a comment.
> 
> They've not been implemented yet.

I'd add a comment for clarity, otherwise it's kind of confusing.