DevTools: Source color picker experiment. Make color swatches clickable

third_party/WebKit/Source/devtools/front_end/sources/CSSSourceFrame.js

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 21 matching lines @@
  * @constructor
  * @extends {WebInspector.UISourceCodeFrame}
  * @param {!WebInspector.UISourceCode} uiSourceCode
  */
 WebInspector.CSSSourceFrame = function(uiSourceCode)
 {
     WebInspector.UISourceCodeFrame.call(this, uiSourceCode);
     this.textEditor.setAutocompleteDelegate(new WebInspector.CSSSourceFrame.AutocompleteDelegate());
     this._registerShortcuts();
     this._colorBookmarks = [];
+    this._swatchPopoverHelper = new WebInspector.SwatchPopoverHelper();
+    this._analyzeTextColors = true;

[lushnikov, 2016/07/19 21:12:16]

We usually call this kind of flags like muteXXX, e.g.:

this._muteColorProcessing = false;

[flandy, 2016/07/19 22:58:15]

Done.

 }
 
 /** @type {number} */
 WebInspector.CSSSourceFrame.UpdateTimeout = 200;
 
 WebInspector.CSSSourceFrame.prototype = {
     _registerShortcuts: function()
     {
         var shortcutKeys = WebInspector.ShortcutsScreen.SourcesPanelShortcuts;
         for (var i = 0; i < shortcutKeys.IncreaseCSSUnitByOne.length; ++i)
@@ skipping 93 matching lines @@
      * @param {?Array<!WebInspector.CSSSourceFrame.ColorPosition>} colorPositions
      */
     _putColorSwatchesInline: function(colorPositions)
     {
         this._clearColorBookmarks();
         if (!colorPositions)
             return;
 
         for (var i = 0; i < colorPositions.length; i++) {
             var colorPosition = colorPositions[i];
-
-            var swatch = WebInspector.ColorSwatch.create();
-            swatch.setColorText(colorPosition.color.asString(WebInspector.Color.Format.Original));
-            swatch.hideText(true);
-
-            var bookmark = this.textEditor.addBookmark(colorPosition.textRange.startLine, colorPosition.textRange.startColumn, swatch);
+            var swatch = new WebInspector.CSSSourceFrame.ColorSwatch(this, colorPosition);
+            var bookmark = this.textEditor.addBookmark(colorPosition.textRange.startLine, colorPosition.textRange.startColumn, swatch.getElement());
             this._colorBookmarks.push(bookmark);
         }
     },
 
     _clearColorBookmarks: function()
     {
         for (var i = 0; i < this._colorBookmarks.length; i++)
             this._colorBookmarks[i].clear();
         this._colorBookmarks = [];
     },
 
     /**
+     * @param {!WebInspector.ColorSwatch} swatch
+     */
+    _showSpectrum: function(swatch)
+    {
+        if (this._swatchPopoverHelper.isShowing()) {
+            this._swatchPopoverHelper.hide();
+            return;
+        }
+        var color = swatch.color();
+        var format = swatch.format();
+        if (format === WebInspector.Color.Format.Original)
+            format = color.format();
+
+        this._spectrum = new WebInspector.Spectrum();
+        this._spectrum.setColor(color, format);
+        this._spectrum.addEventListener(WebInspector.Spectrum.Events.SizeChanged, this._spectrumResized, this);

[lushnikov, 2016/07/19 21:12:16]

When does this event fire?

[flandy, 2016/07/19 22:58:15]

This event fires when changing the color palette of the spectrum or adding a
color to the custom color palette. The reposition deals with growing or
shrinking the size of the palette.

+        this._spectrum.addEventListener(WebInspector.Spectrum.Events.ColorChanged, this._spectrumChanged, this);
+        this._swatchPopoverHelper.show(this._spectrum, swatch.iconElement(), this._spectrumHidden.bind(this));
+    },
+
+    /**
+     * @param {!WebInspector.Event} event
+     */
+    _spectrumResized: function(event)
+    {
+        this._swatchPopoverHelper.reposition();
+    },
+
+    /**
+     * @param {!WebInspector.Event} event
+     */
+    _spectrumChanged: function(event)
+    {
+        this._analyzeTextColors = false;
+        var colorString = /** @type {string} */ (event.data);
+        this._currentSwatch.setColorText(colorString);
+        this._replaceColorText(this._currentSwatch.getColorPosition(), colorString);
+    },
+
+    _spectrumHidden: function()
+    {
+        this._analyzeTextColors = true;
+        this._spectrum.removeEventListener(WebInspector.Spectrum.Events.SizeChanged, this._spectrumResized, this);
+        this._spectrum.removeEventListener(WebInspector.Spectrum.Events.ColorChanged, this._spectrumChanged, this);
+        delete this._spectrum;

[lushnikov, 2016/07/19 21:12:16]

you should schedule updateColorSwatches here

[flandy, 2016/07/19 22:58:15]

Why? We already change the color and position of the existing color swatch when
the spectrum is changed. There should be no need to also update the swatches
when the spectrum is hidden. 

[lushnikov, 2016/07/20 00:49:23]

Imagine you have multiple color swatches in the line and edit the first one

[flandy, 2016/07/20 01:56:18]

Yes, good point! Done.

+    },
+
+    /**
+     * @param {!WebInspector.CSSSourceFrame.ColorPosition} colorPosition
+     * @param {string} colorString
+     */
+    _replaceColorText: function(colorPosition, colorString)
+    {
+        this._textEditor.editRange(colorPosition.textRange, colorString);
+        colorPosition.color = WebInspector.Color.parse(colorString);
+        colorPosition.textRange.endColumn = colorPosition.textRange.startColumn + colorString.length;
+    },
+
+    /**
      * @override
      */
     onTextEditorContentSet: function()
     {
         WebInspector.UISourceCodeFrame.prototype.onTextEditorContentSet.call(this);
-        if (Runtime.experiments.isEnabled("sourceColorPicker"))
+        if (Runtime.experiments.isEnabled("sourceColorPicker") && this._analyzeTextColors)

[lushnikov, 2016/07/19 21:12:16]

nit: I'd check for the flag first

[flandy, 2016/07/19 22:58:15]

Done.

             this._updateColorSwatches();
     },
 
     /**
      * @override
      * @param {!WebInspector.TextRange} oldRange
      * @param {!WebInspector.TextRange} newRange
      */
     onTextChanged: function(oldRange, newRange)
     {
         WebInspector.UISourceCodeFrame.prototype.onTextChanged.call(this, oldRange, newRange);
-        if (Runtime.experiments.isEnabled("sourceColorPicker")) {
+        if (Runtime.experiments.isEnabled("sourceColorPicker") && this._analyzeTextColors) {
             if (this._updateTimeout)
                 clearTimeout(this._updateTimeout);
             this._updateTimeout = setTimeout(this._updateColorSwatches.bind(this), WebInspector.CSSSourceFrame.UpdateTimeout);
         }
     },
 
+    /**
+     * @override
+     * @param {number} lineNumber
+     */
+    scrollChanged: function(lineNumber)
+    {
+        WebInspector.UISourceCodeFrame.prototype.scrollChanged.call(this, lineNumber);
+        if (this._swatchPopoverHelper.isShowing())
+            this._swatchPopoverHelper.hide();
+    },
+
     __proto__: WebInspector.UISourceCodeFrame.prototype
 }
 
 /**
  * @constructor
  * @param {!WebInspector.Color} color
  * @param {number} lineNumber
  * @param {number} startColumn
  * @param {number} textLength
  */
 WebInspector.CSSSourceFrame.ColorPosition = function(color, lineNumber, startColumn, textLength)
 {
     this.color = color;
     this.textRange = new WebInspector.TextRange(lineNumber, startColumn, lineNumber, startColumn + textLength);
 }
 
 /**
  * @constructor
+ * @param {!WebInspector.CSSSourceFrame} cssSourceFrame
+ * @param {!WebInspector.CSSSourceFrame.ColorPosition} colorPosition
+ */
+WebInspector.CSSSourceFrame.ColorSwatch = function(cssSourceFrame, colorPosition)
+{
+    this._cssSourceFrame = cssSourceFrame;
+    this._colorPosition = colorPosition;
+
+    this._swatch = WebInspector.ColorSwatch.create();
+    this._swatch.setColorText(colorPosition.color.asString(WebInspector.Color.Format.Original) || "white");
+    var shiftClickMessage = WebInspector.UIString("Shift + Click to change color format.");
+    this._swatch.iconElement().title = WebInspector.UIString("Open color picker. %s", shiftClickMessage);
+    this._swatch.setClickHandler(this._handleClick.bind(this));
+    this._swatch.hideText(true);
+}
+
+WebInspector.CSSSourceFrame.ColorSwatch.prototype = {
+    /**
+     * @param {!Event} event
+     */
+    _handleClick: function(event)
+    {
+        if (event.shiftKey) {

[lushnikov, 2016/07/19 21:12:16]

I'm not convinced we need the shift-click in the source frame swatch - mb remove
this functionality for now?

[flandy, 2016/07/19 22:58:15]

I think it would be nice to have, in order to mirror the functionality in the
styles sidebar pane. Removed for now.

+            this._swatch.toggleNextFormat();
+            this._cssSourceFrame._analyzeTextColors = false;
+            this._cssSourceFrame._replaceColorText(this._colorPosition, this._swatch.color().asString(this._swatch.format()) || "white");

[lushnikov, 2016/07/19 21:12:16]

where does "white" come from?

[flandy, 2016/07/19 22:58:15]

color.asString() may return null|string according to closure compiler.
cssSourceFrame._replaceColorText() expects a string, so this is to make the
compiler happy. 

I can change the annotation for _replaceColorText() to use {?string} and
introduce a check on that string

Or I think it may make more sense to add the get colorText() method on
WI.ColorSwatch and just call that.

+            this._cssSourceFrame._analyzeTextColors = true;
+        } else {
+            this._cssSourceFrame._currentSwatch = this;

[lushnikov, 2016/07/19 21:12:16]

let's not reach to internal fields of cssSourceFrame from here - you already
call it's delegate
method _showSpectrum, it should do everything

[flandy, 2016/07/19 22:58:15]

Done.

+            this._cssSourceFrame._showSpectrum(this._swatch);

[lushnikov, 2016/07/19 21:12:16]

I think it would be easier if you just pass the WI.CSSSourceFrame.ColorSwatch
instance
to the CSSSourceFrame:

this._cssSourceFrame._showSpectrum(this);

[flandy, 2016/07/19 22:58:15]

Done.

+        }
+        event.consume(true);
+    },
+
+    /**
+     * @return {!WebInspector.ColorSwatch}
+     */
+    getElement: function()

[lushnikov, 2016/07/19 21:12:16]

element: ...

(we use nouns for getters and verbs for setters)

[flandy, 2016/07/19 22:58:15]

Done.

+    {
+        return this._swatch;
+    },
+
+    /**
+     * @return {!WebInspector.CSSSourceFrame.ColorPosition}
+     */
+    getColorPosition: function()

[lushnikov, 2016/07/19 21:12:16]

colorPosition: ..

[flandy, 2016/07/19 22:58:15]

Done.

+    {
+        return this._colorPosition;
+    },
+
+    /**
+     * @param {string} colorString
+     */
+    setColorText: function(colorString)
+    {
+        this._swatch.setColorText(colorString);
+    }
+}
+
+/**
+ * @constructor
  * @implements {WebInspector.TextEditorAutocompleteDelegate}
  */
 WebInspector.CSSSourceFrame.AutocompleteDelegate = function()
 {
     this._simpleDelegate = new WebInspector.SimpleAutocompleteDelegate(".-$");
 }
 
 WebInspector.CSSSourceFrame._backtrackDepth = 10;
 
 WebInspector.CSSSourceFrame.AutocompleteDelegate.prototype = {
@@ skipping 73 matching lines @@
         var propertyToken = this._backtrackPropertyToken(editor, prefixRange.startLine, prefixRange.startColumn - 1);
         if (!propertyToken)
             return this._simpleDelegate.wordsWithPrefix(editor, prefixRange, substituteRange);
 
         var line = editor.line(prefixRange.startLine);
         var tokenContent = line.substring(propertyToken.startColumn, propertyToken.endColumn);
         var keywords = WebInspector.CSSMetadata.keywordsForProperty(tokenContent);
         return keywords.startsWith(prefix);
     },
 }