Created attachment 387581 [details] [Image] Bug ColorSquare shows the wrong color selected.
Created attachment 387582 [details] Patch
<rdar://problem/58548071>
Created attachment 387584 [details] Patch Retitled.
Comment on attachment 387584 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387584&action=review r=me > Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:109 > + let hsv = WI.Color.rgb2hsv(...tintedColor.normalizedRGB); NIT: we can destructure this for better code clarity: ``` let [hue, saturation] = WI.Color.rgb2hsv(...tintedColor.normalizedRGB); let x = hue / 100 * this._dimension; let y = (1 - (saturation / 100)) * this._dimension; ``` > Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:112 > + this._setCrosshairPosition(new WI.Point(x, y)); This is called again below. Please remove it, or add a comment explaining why it has to be called twice.
Comment on attachment 387584 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387584&action=review >> Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:109 >> + let hsv = WI.Color.rgb2hsv(...tintedColor.normalizedRGB); > > NIT: we can destructure this for better code clarity: > ``` > let [hue, saturation] = WI.Color.rgb2hsv(...tintedColor.normalizedRGB); > let x = hue / 100 * this._dimension; > let y = (1 - (saturation / 100)) * this._dimension; > ``` I'm not using the 1st array's item, hue. >> Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:112 >> + this._setCrosshairPosition(new WI.Point(x, y)); > > This is called again below. Please remove it, or add a comment explaining why it has to be called twice. This was a mistake. Thanks!
Created attachment 387987 [details] Patch
Comment on attachment 387987 [details] Patch Clearing flags on attachment: 387987 Committed r254726: <https://trac.webkit.org/changeset/254726>
All reviewed patches have been landed. Closing bug.