[webkit-reviews] review granted: [Bug 203533] Web Inspector: Outline sRGB-safe areas on P3 color picker : [Attachment 384009] Patch for review

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 203533: Web Inspector: Outline sRGB-safe areas on P3 color picker
https://bugs.webkit.org/show_bug.cgi?id=203533

Attachment 384009: Patch for review

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




--- Comment #5 from Brian Burg <bburg at apple.com> ---
Comment on attachment 384009
  --> https://bugs.webkit.org/attachment.cgi?id=384009
Patch for review

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

r=me

> Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:232
> +	       this._srgbLabelElement.title = WI.UIString("This line marks the
edge of sRGB color space");

Please add a localization key that describes the larger context (i.e., label
for a guide within the color picker). As is, a localizer will have no idea how
to find this text in the UI.

> Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:256
> +		       // The point is within sRGB.

I think this comment is backwards. If the P3 value is clipped at all in sRGB,
then it's not within sRGB.

> Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:263
> +	   if (points.lastValue.y < this._dimension * 0.95) {

Nice attention to detail.