[webkit-reviews] review granted: [Bug 235422] Web Inspector: Element tooltips in overlays should use same encodable/decodable Label type as grid overlays : [Attachment 450827] Patch v1.1 - Address review comments so far, rebase
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Feb 10 14:19:16 PST 2022
Devin Rousso <drousso at apple.com> has granted Patrick Angle <pangle at apple.com>'s
request for review:
Bug 235422: Web Inspector: Element tooltips in overlays should use same
encodable/decodable Label type as grid overlays
https://bugs.webkit.org/show_bug.cgi?id=235422
Attachment 450827: Patch v1.1 - Address review comments so far, rebase
https://bugs.webkit.org/attachment.cgi?id=450827&action=review
--- Comment #7 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 450827
--> https://bugs.webkit.org/attachment.cgi?id=450827
Patch v1.1 - Address review comments so far, rebase
View in context: https://bugs.webkit.org/attachment.cgi?id=450827&action=review
r=mews, neat stuff! Thanks for uploading the other diff :)
> Source/WebCore/inspector/InspectorOverlayLabel.cpp:43
> +InspectorOverlayLabel::InspectorOverlayLabel(Vector<Content>&& contents,
const FloatPoint& location, const Color& backgroundColor, Arrow arrow)
NIT: I realize that it's a `struct` of two `enum`, but it's probably a good
idea to make all these `const Arrow&` in case a new member is added.
ditto below
> Source/WebCore/inspector/InspectorOverlayLabel.cpp:221
> + auto text = lines.at(i);
NIT: Any reason not to just `[i]` instead?
ditto below
More information about the webkit-reviews
mailing list