| Summary: | REGRESSION(r251487): Web Inspector: selected color in color picker has wrong lightness | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||||||
| Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | commit-queue, hi, inspector-bugzilla-changes, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 387582 [details]
Patch
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. |
Created attachment 387581 [details] [Image] Bug ColorSquare shows the wrong color selected.