Merge pull request #10425 from MarcelGerber/css-hints-color-badge

redmunds · redmunds · commit f1e23402aeba · 2015-01-24T09:48:30.000-08:00
Show colored swatch in CSS Code Hints
diff --git a/src/extensions/default/CSSCodeHints/main.js b/src/extensions/default/CSSCodeHints/main.js
@@ -175,16 +175,20 @@ define(function (require, exports, module) {
         return true;
     };
 
-    /*
+    /**
      * Returns a sorted and formatted list of hints with the query substring
      * highlighted.
      * 
      * @param {Array.<Object>} hints - the list of hints to format
      * @param {string} query - querystring used for highlighting matched
-     *      poritions of each hint
+     *      portions of each hint
      * @return {Array.jQuery} sorted Array of jQuery DOM elements to insert
      */
     function formatHints(hints, query) {
+        var hasColorSwatch = hints.some(function (token) {
+            return token.color;
+        });
+
         StringMatch.basicMatchSort(hints);
         return hints.map(function (token) {
             var $hintObj = $("<span>").addClass("brackets-css-hints");
@@ -204,6 +208,10 @@ define(function (require, exports, module) {
                 $hintObj.text(token.value);
             }
 
+            if (hasColorSwatch) {
+                $hintObj = ColorUtils.formatColorHint($hintObj, token.color);
+            }
+
             $hintObj.data("token", token);
 
             return $hintObj;
@@ -286,13 +294,19 @@ define(function (require, exports, module) {
                 
                 valueArray = valueArray.concat(namedFlows);
             } else if (type === "color") {
-                valueArray = valueArray.concat(ColorUtils.COLOR_NAMES);
+                valueArray = valueArray.concat(ColorUtils.COLOR_NAMES.map(function (color) {
+                    return { text: color, color: color };
+                }));
                 valueArray.push("transparent", "currentColor");
             }
             
-            result = $.map(valueArray, function (pvalue, pindex) {
-                var result = StringMatch.stringMatch(pvalue, valueNeedle, stringMatcherOptions);
+            result = $.map(valueArray, function (pvalue) {
+                var result = StringMatch.stringMatch(pvalue.text || pvalue, valueNeedle, stringMatcherOptions);
                 if (result) {
+                    if (pvalue.color) {
+                        result.color = pvalue.color;
+                    }
+
                     return result;
                 }
             });
diff --git a/src/extensions/default/CSSCodeHints/unittest-files/regions.css b/src/extensions/default/CSSCodeHints/unittest-files/regions.css
@@ -93,3 +93,13 @@ div.layout > div {
     flow-from: content;
     flow-from: element;
 }
+
+/* colors */
+.colorful {
+    color: ;
+    background-color: ;
+    border-left-color: deep;
+    border-color: rent;
+    height: ;
+    color: transparent;
+}
diff --git a/src/extensions/default/CSSCodeHints/unittests.js b/src/extensions/default/CSSCodeHints/unittests.js
@@ -80,11 +80,12 @@ define(function (require, exports, module) {
         }
         
         // Ask provider for hints at current cursor position; expect it to return some
-        function expectHints(provider, implicitChar) {
+        function expectHints(provider, implicitChar, returnWholeObj) {
             expect(provider.hasHints(testEditor, implicitChar)).toBe(true);
             var hintsObj = provider.getHints();
             expect(hintsObj).toBeTruthy();
-            return extractHintList(hintsObj.hints); // return just the array of hints
+            // return just the array of hints if returnWholeObj is falsy
+            return returnWholeObj ? hintsObj : extractHintList(hintsObj.hints);
         }
         
         // Ask provider for hints at current cursor position; expect it NOT to return any
@@ -664,6 +665,77 @@ define(function (require, exports, module) {
             });
         });
 
+        describe("Color names and swatches in a CSS file", function () {
+            beforeEach(function () {
+                setupTest(testContentCSS, "css");
+            });
+            
+            afterEach(function () {
+                tearDownTest();
+            });
+            
+            it("should list color names for color", function () {
+                testEditor.setCursorPos({ line: 98, ch: 11 }); // after color
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                verifyAttrHints(hintList, "aliceblue"); // first hint should be aliceblue
+            });
+            
+            it("should show color swatches for background-color", function () {
+                testEditor.setCursorPos({ line: 99, ch: 22 }); // after background-color
+                var hints = expectHints(CSSCodeHints.cssPropHintProvider, undefined, true).hints;
+                expect(hints[0].text()).toBe("aliceblue"); // first hint should be aliceblue
+                expect(hints[0].find(".color-swatch").length).toBe(1);
+                expect(hints[0].find(".color-swatch").css("backgroundColor")).toBe("rgb(240, 248, 255)");
+            });
+            
+            it("should filter out color names appropriately", function () {
+                testEditor.setCursorPos({ line: 100, ch: 27 }); // after border-left-color
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                verifyAttrHints(hintList, "deeppink"); // first hint should be deeppink
+                verifyAllValues(hintList, ["deeppink", "deepskyblue"]);
+            });
+            
+            it("should always include transparent and currentColor and they should not have a swatch, but class no-swatch-margin", function () {
+                testEditor.setCursorPos({ line: 101, ch: 22 }); // after border-color
+                var hints = expectHints(CSSCodeHints.cssPropHintProvider, undefined, true).hints,
+                    hintList = extractHintList(hints);
+                verifyAttrHints(hintList, "currentColor"); // first hint should be currentColor
+                verifyAllValues(hintList, ["currentColor", "darkmagenta", "transparent"]);
+                expect(hints[0].find(".color-swatch").length).toBe(0); // no swatch for currentColor
+                expect(hints[2].find(".color-swatch").length).toBe(0); // no swatch for transparent
+                expect(hints[0].hasClass("no-swatch-margin")).toBeTruthy(); // no-swatch-margin applied to currentColor
+                expect(hints[2].hasClass("no-swatch-margin")).toBeTruthy(); // no-swatch-margin applied to transparent
+            });
+
+            it("should remove class no-swatch-margin from transparent if it's the only one in the list", function () {
+                testEditor.setCursorPos({ line: 103, ch: 22 }); // after color
+                var hints = expectHints(CSSCodeHints.cssPropHintProvider, undefined, true).hints,
+                    hintList = extractHintList(hints);
+                verifyAllValues(hintList, ["transparent"]);
+                expect(hints[0].find(".color-swatch").length).toBe(0); // no swatch for transparent
+                expect(hints[0].hasClass("no-swatch-margin")).toBeFalsy(); // no-swatch-margin not applied to transparent
+            });
+            
+            it("should insert color names correctly", function () {
+                var expectedString  = "    border-left-color: deeppink;",
+                    line            = 100;
+
+                testEditor.setCursorPos({ line: line, ch: 27 }); // after border-left-color
+                expectHints(CSSCodeHints.cssPropHintProvider);
+                selectHint(CSSCodeHints.cssPropHintProvider, "deeppink");
+                expect(testDocument.getLine(line).length).toBe(expectedString.length);
+                expect(testDocument.getLine(line)).toBe(expectedString);
+                expectCursorAt({ line: line, ch: expectedString.length - 1 });
+            });
+            
+            
+            it("should not display color names for unrelated properties", function () {
+                testEditor.setCursorPos({ line: 102, ch: 12 }); // after height
+                var hintList = expectHints(CSSCodeHints.cssPropHintProvider);
+                expect(hintList.indexOf("aliceblue")).toBe(-1);
+            });
+        });
+
         describe("Should not invoke CSS hints on space key", function () {
             beforeEach(function () {
                 setupTest(testContentHTML, "html");
diff --git a/src/extensions/default/SVGCodeHints/main.js b/src/extensions/default/SVGCodeHints/main.js
@@ -102,6 +102,10 @@ define(function (require, exports, module) {
      * @return {Array.jQuery} sorted Array of jQuery DOM elements to insert
      */
     function formatHints(hints, query) {
+        var hasColorSwatch = hints.some(function (token) {
+            return token.color;
+        });
+
         StringMatch.basicMatchSort(hints);
         return hints.map(function (token) {
             var $hintObj = $("<span>").addClass("brackets-svg-hints");
@@ -121,6 +125,10 @@ define(function (require, exports, module) {
                 $hintObj.text(token.value);
             }
 
+            if (hasColorSwatch) {
+                $hintObj = ColorUtils.formatColorHint($hintObj, token.color);
+            }
+
             $hintObj.data("token", token);
 
             return $hintObj;
@@ -210,7 +218,9 @@ define(function (require, exports, module) {
                     isMultiple = attributeData[tagInfo.attrName].multiple;
 
                     if (attributeData[tagInfo.attrName].type === "color") {
-                        options = ColorUtils.COLOR_NAMES;
+                        options = ColorUtils.COLOR_NAMES.map(function (color) {
+                            return { text: color, color: color };
+                        });
                         options = options.concat(["currentColor", "transparent"]);
                     }
                 }
@@ -223,8 +233,12 @@ define(function (require, exports, module) {
                 query = XMLUtils.getValueQuery(tagInfo);
                 hints = $.map(options, function (option) {
                     if (tagInfo.exclusionList.indexOf(option) === -1) {
-                        var match = StringMatch.stringMatch(option, query, stringMatcherOptions);
+                        var match = StringMatch.stringMatch(option.text || option, query, stringMatcherOptions);
                         if (match) {
+                            if (option.color) {
+                                match.color = option.color;
+                            }
+
                             return match;
                         }
                     }
diff --git a/src/extensions/default/SVGCodeHints/unittests.js b/src/extensions/default/SVGCodeHints/unittests.js
@@ -39,8 +39,8 @@ define(function (require, exports, module) {
                         "<svg xmlns=\"https://www.w3.org/2000/svg\" xmlns:xlink=\"https://www.w3.org/1999/xlink\"\n" +
                         "     width=\"200\" height=\"200\" preserveAspectRatio=\"xMinYMin meet\">\n" +
                         "    <title>Brackets SVG Code Hints</title>\n" +
-                        "    <rect width=\"200\" height=\"200\" baseline-shift=\"baseline\" alignment-baseline=\"alphabetic\" stroke-width=\"1\"></rect>\n" +
-                        "    <rect width='160' height='160' x='20' y='20' baseline-shift='super' alignment-baseline='baseline' />\n" +
+                        "    <rect width=\"200\" height=\"200\" baseline-shift=\"baseline\" alignment-baseline=\"alphabetic\" stroke-width=\"1\" color=\"\"></rect>\n" +
+                        "    <rect width='160' height='160' x='20' y='20' baseline-shift='super' alignment-baseline='baseline' color='rent' fill='transparent' />\n" +
                         "    <g>\n" +
                         "        \n" +
                         "    </g>\n" +
@@ -205,7 +205,7 @@ define(function (require, exports, module) {
                 expectNoHints(SVGCodeHints.hintProvider);
                 
                 // Between <rect></rect>
-                testEditor.setCursorPos({line: 4, ch: 110});
+                testEditor.setCursorPos({line: 4, ch: 119});
                 expectNoHints(SVGCodeHints.hintProvider);
                 
                 // After <title>+space in <title> Brackets
@@ -216,21 +216,21 @@ define(function (require, exports, module) {
             
             it("should not hint after the closed tag", function () {
                 // After </rect>
-                testEditor.setCursorPos({line: 4, ch: 117});
+                testEditor.setCursorPos({line: 4, ch: 126});
                 expectNoHints(SVGCodeHints.hintProvider);
                 
                 // After space at </rect>+space
-                testDocument.replaceRange(" ", {line: 4, ch: 117});
-                testEditor.setCursorPos({line: 4, ch: 118});
+                testDocument.replaceRange(" ", {line: 4, ch: 126});
+                testEditor.setCursorPos({line: 4, ch: 127});
                 expectNoHints(SVGCodeHints.hintProvider);
                 
                 // After />
-                testEditor.setCursorPos({line: 5, ch: 104});
+                testEditor.setCursorPos({line: 5, ch: 136});
                 expectNoHints(SVGCodeHints.hintProvider);
                 
                 // After space in />+space
-                testDocument.replaceRange(" ", {line: 5, ch: 104});
-                testEditor.setCursorPos({line: 5, ch: 105});
+                testDocument.replaceRange(" ", {line: 5, ch: 136});
+                testEditor.setCursorPos({line: 5, ch: 137});
                 expectNoHints(SVGCodeHints.hintProvider);
                 
                 // After </g>
@@ -453,6 +453,38 @@ define(function (require, exports, module) {
             });
         });
         
+        describe("Color names and swatches", function () {
+            it("should show color swatches", function () {
+                // After color="
+                testEditor.setCursorPos({line: 4, ch: 117});
+                var hints = expectHints(SVGCodeHints.hintProvider);
+                verifyHints(hints, "aliceblue"); // first hint should be aliceblue
+                expect(hints[0].find(".color-swatch").length).toBe(1);
+                expect(hints[0].find(".color-swatch").css("backgroundColor")).toBe("rgb(240, 248, 255)");
+            });
+
+            it("should always include transparent and currentColor and they should not have a swatch, but class no-swatch-margin", function () {
+                // After color='rent
+                testEditor.setCursorPos({line: 5, ch: 113});
+                var hints = expectHints(SVGCodeHints.hintProvider);
+                verifyHints(hints, "currentColor"); // first hint should be currentColor
+                expect(hints[0].find(".color-swatch").length).toBe(0); // no swatch for currentColor
+                expect(hints[2].find(".color-swatch").length).toBe(0); // no swatch for transparent
+                expect(hints[0].hasClass("no-swatch-margin")).toBeTruthy(); // no-swatch-margin applied to currentColor
+                expect(hints[2].hasClass("no-swatch-margin")).toBeTruthy(); // no-swatch-margin applied to transparent
+            });
+
+            it("should remove class no-swatch-margin from transparent if it's the only one in the list", function () {
+                // After fill='transparent
+                testEditor.setCursorPos({line: 5, ch: 132});
+                var hints = expectHints(SVGCodeHints.hintProvider);
+                verifyHints(hints, "transparent");
+                expect(hints.length).toBe(1); // transparent should be the only hint
+                expect(hints[0].find(".color-swatch").length).toBe(0); // no swatch for transparent
+                expect(hints[0].hasClass("no-swatch-margin")).toBeFalsy(); // no-swatch-margin not applied to transparent
+            });
+        });
+
         describe("Tag Insertion", function () {
             it("should insert if query is empty", function () {
                 // After < inside <g>
diff --git a/src/styles/brackets_patterns_override.less b/src/styles/brackets_patterns_override.less
@@ -526,6 +526,25 @@ a:focus {
                         background-color: @dark-bc-bg-highlight;
                     }
                 }
+
+                .color-swatch {
+                    border-radius: 2px;
+                    width: 12px;
+                    height: 12px;
+                    float: left;
+                    margin: 2px -8px 0 4px;
+                    box-shadow: inset 0 0 2px rgba(0, 0, 0, 0.24);
+
+                    .dark & {
+                        box-shadow: inset 0 0 1px rgba(255, 255, 255, 0.24);
+                    }
+                }
+
+                // Used to align non-swatch items with the others. Only applied
+                // if there's at least one item with a swatch in the list
+                .no-swatch-margin {
+                    margin-left: 8px;
+                }
             }
         }
     }
diff --git a/src/utils/ColorUtils.js b/src/utils/ColorUtils.js
@@ -22,7 +22,7 @@
  */
 
 /*jslint vars: true, plusplus: true, devel: true, nomen: true, regexp: true, indent: 4, maxerr: 50 */
-/*global define */
+/*global define, $ */
 
 /**
  *  Utilities functions related to color matching
@@ -48,7 +48,27 @@ define(function (require, exports, module) {
     // use RegExp.source of the RegExp literal to avoid doubled backslashes
     var COLOR_REGEX = new RegExp(/#[a-f0-9]{6}\b|#[a-f0-9]{3}\b|\brgb\(\s*(?:[0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*(?:[0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*(?:[0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*\)|\brgb\(\s*(?:[0-9]{1,2}%|100%)\s*,\s*(?:[0-9]{1,2}%|100%)\s*,\s*(?:[0-9]{1,2}%|100%)\s*\)|\brgba\(\s*(?:[0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*(?:[0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*(?:[0-9]{1,2}|1[0-9]{2}|2[0-4][0-9]|25[0-5])\b\s*,\s*(?:1|1\.0|0|0?\.[0-9]{1,3})\s*\)|\brgba\(\s*(?:[0-9]{1,2}%|100%)\s*,\s*(?:[0-9]{1,2}%|100%)\s*,\s*(?:[0-9]{1,2}%|100%)\s*,\s*(?:1|1\.0|0|0?\.[0-9]{1,3})\s*\)|\bhsl\(\s*(?:[0-9]{1,3})\b\s*,\s*(?:[0-9]{1,2}|100)\b%\s*,\s*(?:[0-9]{1,2}|100)\b%\s*\)|\bhsla\(\s*(?:[0-9]{1,3})\b\s*,\s*(?:[0-9]{1,2}|100)\b%\s*,\s*(?:[0-9]{1,2}|100)\b%\s*,\s*(?:1|1\.0|0|0?\.[0-9]{1,3})\s*\)|\b/.source + COLOR_NAMES.join("\\b|\\b") + "\\b", "gi");
 
+    /*
+     * Adds a color swatch to code hints where this is supported.
+     * @param {!jQuery} $hintObj - list item where the swatch will be in
+     * @param {?string} color - color the swatch should have, or null to add extra left margin to
+     *      align with the other hints
+     * @return {jQuery} jQuery object with the correct class and/or style applied
+     */
+    function formatColorHint($hintObj, color) {
+        if (color) {
+            $hintObj.prepend($("<span>")
+                .addClass("color-swatch")
+                .css("backgroundColor", color));
+        } else {
+            $hintObj.addClass("no-swatch-margin");
+        }
+        return $hintObj;
+    }
+
+
     // Define public API
-    exports.COLOR_NAMES = COLOR_NAMES;
-    exports.COLOR_REGEX = COLOR_REGEX;
+    exports.COLOR_NAMES     = COLOR_NAMES;
+    exports.COLOR_REGEX     = COLOR_REGEX;
+    exports.formatColorHint = formatColorHint;
 });

Original file line number	Diff line number	Diff line change
`@@ -526,6 +526,25 @@ a:focus {`
`526`	`526`	`background-color: @dark-bc-bg-highlight;`
`527`	`527`	`}`
`528`	`528`	`}`
	`529`	`+`
	`530`	`+ .color-swatch {`
	`531`	`+ border-radius: 2px;`
	`532`	`+ width: 12px;`
	`533`	`+ height: 12px;`
	`534`	`+ float: left;`
	`535`	`+ margin: 2px -8px 0 4px;`
	`536`	`+ box-shadow: inset 0 0 2px rgba(0, 0, 0, 0.24);`
	`537`	`+`
	`538`	`+ .dark & {`
	`539`	`+ box-shadow: inset 0 0 1px rgba(255, 255, 255, 0.24);`
	`540`	`+ }`
	`541`	`+ }`
	`542`	`+`
	`543`	`+ // Used to align non-swatch items with the others. Only applied`
	`544`	`+ // if there's at least one item with a swatch in the list`
	`545`	`+ .no-swatch-margin {`
	`546`	`+ margin-left: 8px;`
	`547`	`+ }`
`529`	`548`	`}`
`530`	`549`	`}`
`531`	`550`	`}`