[webkit-reviews] review granted: [Bug 221979] Web Inspector: Implement Grid Overlay "Show area names" drawing : [Attachment 420551] Patch v1.2 - Label Translucency and Clipping
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 16 16:08:04 PST 2021
Devin Rousso <drousso at apple.com> has granted Patrick Angle <pangle at apple.com>'s
request for review:
Bug 221979: Web Inspector: Implement Grid Overlay "Show area names" drawing
https://bugs.webkit.org/show_bug.cgi?id=221979
Attachment 420551: Patch v1.2 - Label Translucency and Clipping
https://bugs.webkit.org/attachment.cgi?id=420551&action=review
--- Comment #10 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 420551
--> https://bugs.webkit.org/attachment.cgi?id=420551
Patch v1.2 - Label Translucency and Clipping
View in context: https://bugs.webkit.org/attachment.cgi?id=420551&action=review
r=me (again )
> Source/WebCore/inspector/InspectorOverlay.cpp:1197
> + // Add an ellipsis to the label.
redundant IMO
> Source/WebCore/inspector/InspectorOverlay.cpp:1198
> + label.append("...");
add `_s` after the closing `"` (this converts the cstring to `ASCIILiteral`)
> Source/WebCore/inspector/InspectorOverlay.cpp:1199
> + while (textWidth + (padding * 2) > maximumWidth || label.length() <=
1) {
Shouldn't this be `&& label.length() >= 4`?
> Source/WebCore/inspector/InspectorOverlay.h:187
> + void drawLayoutLabel(GraphicsContext&, String, FloatPoint,
LabelArrowDirection, Color = Color::white, float = 0);
Style: please give a name to generic arguments like `Color backgroundColor =
Color::white` and `float maximumWidth = 0` so that someone just looking at the
header can figure out what the arguments are for without having to go
spelunking into the implementation :)
More information about the webkit-reviews
mailing list