[webkit-reviews] review granted: [Bug 221763] Web Inspector: Grid list in Layout panel missing empty message : [Attachment 420517] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 16 12:16:45 PST 2021
Devin Rousso <drousso at apple.com> has granted Razvan Caliman
<rcaliman at apple.com>'s request for review:
Bug 221763: Web Inspector: Grid list in Layout panel missing empty message
https://bugs.webkit.org/show_bug.cgi?id=221763
Attachment 420517: Patch
https://bugs.webkit.org/attachment.cgi?id=420517&action=review
--- Comment #7 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 420517
--> https://bugs.webkit.org/attachment.cgi?id=420517
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=420517&action=review
r=me, nice fix :)
> Source/WebInspectorUI/UserInterface/Views/LayoutDetailsSidebarPanel.js:128
> + }
> + else {
Style: we put `else` on the same line as `}`
> Source/WebInspectorUI/UserInterface/Views/LayoutDetailsSidebarPanel.js:131
> + this._gridSection = new WI.CSSGridSection;
IMO you could/should just create this once in `initialLayout` to avoid having
extra allocations/deallocations. You could then always know that
`this._gridSection` is valid and not ever have to invalidate (`= null;`) it.
You are correct tho in calling `removeSubview` and `appendChild`/`addSubview`
because `WI.DetailsSectionRow` will `removeChildren()` when it shows/hides the
empty message, so make sure to keep those bits :)
More information about the webkit-reviews
mailing list