DevTools: Add bezier swatches to CSS Sources

DevTools: Add bezier swatches to CSS Sources

Bezier swatches will complement color swatches.

BUG=642782
Committed: https://crrev.com/02a61dc52a2ee12175d21c7d61d9906597a39fbb
Cr-Commit-Position: refs/heads/master@{#417177}

[flandy, 2016-08-31 19:47:23]

flandy@google.com changed reviewers:
+ dgozman@chromium.org, lushnikov@chromium.org

[flandy, 2016-08-31 19:47:24]

Please take a look. Dependent on https://codereview.chromium.org/2298903002/.

[lushnikov, 2016-08-31 20:25:40]

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js
(right):

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:125: if
(result.regexIndex !== 2 && result.regexIndex !== 3)
Let's rewrite this with Handlers:


var regexes = [..your regexes..];
var handlers = new Map();
handlers.set(WI.Color.Regex, handleColor.bind(this));
handlers.set(WI.Bezier.Regex, handleBezier.bind(this));

for (var lineNumber = startLine; lineNumber <= endLine; lineNumber++) {
    ....
}

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:137: if
(!WebInspector.Geometry.CubicBezier.parse(result.value))
let's extract this logic into a handler

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:144: if
(!WebInspector.Color.parse(result.value))
let's extract this logic into a handler as well

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:246:
_swatchIconClicked: function(swatch, bookmark, event)
let's make this function a click listener, and let it call
either _showBezierEditor or _showSpectrum

It would be easier to follow

[flandy, 2016-09-01 21:18:54]

Please take another look.

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js
(right):

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:125: if
(result.regexIndex !== 2 && result.regexIndex !== 3)
On 2016/08/31 20:25:39, lushnikov wrote:
> Let's rewrite this with Handlers:
> 
> 
> var regexes = [..your regexes..];
> var handlers = new Map();
> handlers.set(WI.Color.Regex, handleColor.bind(this));
> handlers.set(WI.Bezier.Regex, handleBezier.bind(this));
> 
> for (var lineNumber = startLine; lineNumber <= endLine; lineNumber++) {
>     ....
> }

Done.

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:137: if
(!WebInspector.Geometry.CubicBezier.parse(result.value))
On 2016/08/31 20:25:39, lushnikov wrote:
> let's extract this logic into a handler

Done.

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:144: if
(!WebInspector.Color.parse(result.value))
On 2016/08/31 20:25:39, lushnikov wrote:
> let's extract this logic into a handler as well

Done.

https://codereview.chromium.org/2297873004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:246:
_swatchIconClicked: function(swatch, bookmark, event)
On 2016/08/31 20:25:39, lushnikov wrote:
> let's make this function a click listener, and let it call
> either _showBezierEditor or _showSpectrum
> 
> It would be easier to follow

Done.

[lushnikov, 2016-09-01 21:43:40]

we also need a test for this!

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js
(right):

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:129: if
(result.regexIndex !== 2 && result.regexIndex !== 3)
instead of checking indexes, let's check if there's handler for the regex

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:135: ||
positionAfter < line.length && !delimiters.test(line.charAt(positionAfter)))
style: wrapping indent is for spaces

(see https://www.chromium.org/blink/coding-style for details)

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:167:
_handleColor: function(text)
_createColorSwatch:

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:183:
_handleBezier: function(text)
_createBezierSwatch:

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:205:
this._currentSwatchTextRange = swatchPosition.clone();
this._editedSwatchTextRange

[flandy, 2016-09-02 17:04:31]

Please take another look.

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js
(right):

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:129: if
(result.regexIndex !== 2 && result.regexIndex !== 3)
On 2016/09/01 21:43:40, lushnikov wrote:
> instead of checking indexes, let's check if there's handler for the regex

Done.

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:135: ||
positionAfter < line.length && !delimiters.test(line.charAt(positionAfter)))
On 2016/09/01 21:43:40, lushnikov wrote:
> style: wrapping indent is for spaces
> 
> (see https://www.chromium.org/blink/coding-style for details)

Done.

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:167:
_handleColor: function(text)
On 2016/09/01 21:43:40, lushnikov wrote:
> _createColorSwatch:

Done.

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:183:
_handleBezier: function(text)
On 2016/09/01 21:43:40, lushnikov wrote:
> _createBezierSwatch:

Done.

https://codereview.chromium.org/2297873004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:205:
this._currentSwatchTextRange = swatchPosition.clone();
On 2016/09/01 21:43:40, lushnikov wrote:
> this._editedSwatchTextRange

Done.

[lushnikov, 2016-09-06 16:48:53]

https://codereview.chromium.org/2297873004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/bezier-swatch-position.html
(right):

https://codereview.chromium.org/2297873004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/bezier-swatch-position.html:48:
function dumpSwatchPositions()
As you already use this method inside color-swatch-position.html, let's
generalize & extract this into sources-test.js

https://codereview.chromium.org/2297873004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js
(right):

https://codereview.chromium.org/2297873004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:278:
this._muteColorProcessing = true;
don't you want to set this flag in _showBezierEditor and _showSpectrum methods?

[flandy, 2016-09-07 00:11:56]

Please take another look.

https://codereview.chromium.org/2297873004/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/bezier-swatch-position.html
(right):

https://codereview.chromium.org/2297873004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/bezier-swatch-position.html:48:
function dumpSwatchPositions()
On 2016/09/06 16:48:52, lushnikov wrote:
> As you already use this method inside color-swatch-position.html, let's
> generalize & extract this into sources-test.js

Done.

https://codereview.chromium.org/2297873004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js
(right):

https://codereview.chromium.org/2297873004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js:278:
this._muteColorProcessing = true;
On 2016/09/06 16:48:52, lushnikov wrote:
> don't you want to set this flag in _showBezierEditor and _showSpectrum
methods?

We should set it in _swatchIconClicked then. Done and renamed to
_muteSwatchProcessing.

[lushnikov, 2016-09-07 21:15:25]

lgtm

[flandy, 2016-09-07 21:21:07]

The CQ bit was checked by flandy@google.com

[commit-bot: I haz the power, 2016-09-07 21:21:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 01:44:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-08 01:44:34]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[flandy, 2016-09-08 01:50:45]

The CQ bit was checked by flandy@google.com

[commit-bot: I haz the power, 2016-09-08 01:51:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 03:03:59]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-08 03:05:06]

Description was changed from

==========
DevTools: Add bezier swatches to CSS Sources

Bezier swatches will complement color swatches.

BUG=642782
==========

to

==========
DevTools: Add bezier swatches to CSS Sources

Bezier swatches will complement color swatches.

BUG=642782

Committed: https://crrev.com/02a61dc52a2ee12175d21c7d61d9906597a39fbb
Cr-Commit-Position: refs/heads/master@{#417177}
==========

[commit-bot: I haz the power, 2016-09-08 03:05:07]

Patchset 4 (id:??) landed as
https://crrev.com/02a61dc52a2ee12175d21c7d61d9906597a39fbb
Cr-Commit-Position: refs/heads/master@{#417177}