[webkit-reviews] review granted: [Bug 235454] Web Inspector: [Flexbox] Add instrumentation/protocol bits for flex layout containers : [Attachment 449695] Patch v1.0

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 21 15:31:56 PST 2022


Devin Rousso <drousso at apple.com> has granted Patrick Angle <pangle at apple.com>'s
request for review:
Bug 235454: Web Inspector: [Flexbox] Add instrumentation/protocol bits for flex
layout containers
https://bugs.webkit.org/show_bug.cgi?id=235454

Attachment 449695: Patch v1.0

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




--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 449695
  --> https://bugs.webkit.org/attachment.cgi?id=449695
Patch v1.0

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

r=me, neat :)

I'm guessing the UI and other things will be coming later?

> Source/WebCore/inspector/agents/InspectorCSSAgent.cpp:945
> +    if (is<RenderFlexibleBox>(renderer))
> +	   return Protocol::CSS::LayoutContextType::Flex;

NIT: I'd put this before `if (is<RenderGrid>(renderer))` so that it is
alphabetical and matches the order in the protocol :P

> Source/WebCore/rendering/RenderFlexibleBox.cpp:76
>      setChildrenInline(false); // All of our children must be block-level.
> +    InspectorInstrumentation::nodeLayoutContextChanged(element, this);

Style: I'd add a newline

> Source/WebCore/rendering/RenderFlexibleBox.cpp:83
>      setChildrenInline(false); // All of our children must be block-level.
> +    InspectorInstrumentation::nodeLayoutContextChanged(document, this);

ditto (:75)


More information about the webkit-reviews mailing list