Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

#7723 - Adding Support For '0x' Notation Color Format #13154

Merged
merged 17 commits into from
Mar 14, 2017
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/command/KeyBindingManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ define(function (require, exports, module) {
var normalizedKey = normalizeKeyDescriptorString(key);

if (!normalizedKey) {
console.log("Fail to nomalize " + key);
console.log("Failed to normalize " + key);
} else if (_isKeyAssigned(normalizedKey)) {
var binding = _keyMap[normalizedKey],
command = CommandManager.get(binding.commandID),
Expand Down
77 changes: 64 additions & 13 deletions src/extensions/default/InlineColorEditor/ColorEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,32 @@ define(function (require, exports, module) {
*/
var STEP_MULTIPLIER = 5;

/**
* Convert 0x notation into hex6 format for tinycolor
* compatibility: ("0xFFAACC" => "#FFFFFF")
*/
function color0xToHex(color,convertToStr){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: space after comma color, convertToStr

var format0xToHexColor = color.replace("0x","#");
var hexColor = tinycolor(format0xToHexColor);
hexColor._format = "0x";

if(convertToStr){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more nit: if (convertToString) { (we really should enable http://eslint.org/docs/rules/space-after-keywords and similar in our config, our spaces are wildly inconsistent across the codebase 🤔 )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return hexColor.toString();
}
return hexColor;
}

function as0xString(color){
return color.toHexString().replace("#","0x");
}

function checkSetFormat(color,convertToStr){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: space after comma color, convertToStr

if ((/^0x/).test(color)) {
return color0xToHex(color,convertToStr);
}
return tinycolor(color);
}

/**
* Color picker control; may be used standalone or within an InlineColorEditor inline widget.
* @param {!jQuery} $parent DOM node into which to append the root of the color picker UI
Expand All @@ -64,8 +90,9 @@ define(function (require, exports, module) {
this._handleHueDrag = this._handleHueDrag.bind(this);
this._handleSelectionFieldDrag = this._handleSelectionFieldDrag.bind(this);

this._color = tinycolor(color);
this._originalColor = color;
this._color = checkSetFormat(color);

this._redoColor = null;
this._isUpperCase = PreferencesManager.get("uppercaseColors");
PreferencesManager.on("change", "uppercaseColors", function () {
Expand All @@ -77,6 +104,7 @@ define(function (require, exports, module) {
this.$rgbaButton = this.$element.find(".rgba");
this.$hexButton = this.$element.find(".hex");
this.$hslButton = this.$element.find(".hsla");
this.$0xButton = this.$element.find(".0x");
this.$currentColor = this.$element.find(".current-color");
this.$originalColor = this.$element.find(".original-color");
this.$selection = this.$element.find(".color-selection-field");
Expand All @@ -95,9 +123,8 @@ define(function (require, exports, module) {
// Attach event listeners to main UI elements
this._addListeners();

// Initially selected color
this.$originalColor.css("background-color", this._originalColor);
this._commitColor(color);
this.$originalColor.css("background-color", checkSetFormat(this._originalColor));
}

/**
Expand Down Expand Up @@ -137,6 +164,7 @@ define(function (require, exports, module) {
this._bindColorFormatToRadioButton("rgba");
this._bindColorFormatToRadioButton("hex");
this._bindColorFormatToRadioButton("hsla");
this._bindColorFormatToRadioButton("0x");

this._bindInputHandlers();

Expand All @@ -158,14 +186,14 @@ define(function (require, exports, module) {
* Update all UI elements to reflect the selected color (_color and _hsv). It is usually
* incorrect to call this directly; use _commitColor() or setColorAsHsv() instead.
*/
ColorEditor.prototype._synchronize = function () {
var colorValue = this.getColor().getOriginalInput(),
colorObject = tinycolor(colorValue),
hueColor = "hsl(" + this._hsv.h + ", 100%, 50%)";

ColorEditor.prototype._synchronize = function () {
var colorValue = this.getColor().getOriginalInput();
var colorObject = checkSetFormat(colorValue);
var hueColor = "hsl(" + this._hsv.h + ", 100%, 50%)";
this._updateColorTypeRadioButtons(colorObject.getFormat());
this.$colorValue.val(colorValue);
this.$currentColor.css("background-color", colorValue);
this.$currentColor.css("background-color", checkSetFormat(colorValue,true));
this.$selection.css("background-color", hueColor);
this.$hueBase.css("background-color", hueColor);

Expand Down Expand Up @@ -228,6 +256,9 @@ define(function (require, exports, module) {
case "hsl":
this.$buttonList.find(".hsla").parent().addClass("selected");
break;
case "0x":
this.$buttonList.find(".0x").parent().addClass("selected");
break;
}
};

Expand All @@ -237,8 +268,9 @@ define(function (require, exports, module) {
self = this;
handler = function (event) {
var newFormat = $(event.currentTarget).html().toLowerCase().replace("%", "p"),
newColor = self.getColor().toString(),
colorObject = tinycolor(newColor);
newColor = self.getColor().toString();

var colorObject = checkSetFormat(newColor);

switch (newFormat) {
case "hsla":
Expand All @@ -254,6 +286,11 @@ define(function (require, exports, module) {
newColor = colorObject.toHexString();
self._hsv.a = 1;
break;
case "0x":
newColor = as0xString(colorObject);
self._hsv.a = 1;
self._format = "0x";
break;
}

// We need to run this again whenever RGB/HSL/Hex conversions
Expand Down Expand Up @@ -302,6 +339,10 @@ define(function (require, exports, module) {
ColorEditor.prototype._normalizeColorString = function (color) {
var normalizedColor = color;

// Convert from 0x notation into hex format string
if (color.match(/^0x[0-9a-fA-F]{6}/)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can re-use the ensureHexFormat function I mentioned in the comment before here?

return color.replace("0x","#") || color;
}
// Convert 6-digit hex to 3-digit hex as TinyColor (#ffaacc -> #fac)
if (color.match(/^#[0-9a-fA-F]{6}/)) {
return tinycolor(color).toString();
Expand All @@ -317,7 +358,7 @@ define(function (require, exports, module) {
/** Handle changes in text field */
ColorEditor.prototype._handleTextFieldInput = function (losingFocus) {
var newColor = $.trim(this.$colorValue.val()),
newColorObj = tinycolor(newColor),
newColorObj = checkSetFormat(newColor),
newColorOk = newColorObj.isValid();

// TinyColor will auto correct an incomplete rgb or hsl value into a valid color value.
Expand Down Expand Up @@ -363,10 +404,12 @@ define(function (require, exports, module) {

// Create swatches
swatches.forEach(function (swatch) {
var swatchValue = swatch.value.replace("0x","#") || swatch.value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, wrap this in some sort of function

var stringFormat = (swatch.count > 1) ? Strings.COLOR_EDITOR_USED_COLOR_TIP_PLURAL : Strings.COLOR_EDITOR_USED_COLOR_TIP_SINGULAR,
usedColorTip = StringUtils.format(stringFormat, swatch.value, swatch.count);

self.$swatches.append("<li tabindex='0'><div class='swatch-bg'><div class='swatch' style='background-color: " +
swatch.value + ";' title='" + usedColorTip + "'></div></div> <span class='value'" + " title='" +
swatchValue + ";' title='" + usedColorTip + "'></div></div> <span class='value'" + " title='" +
usedColorTip + "'>" + swatch.value + "</span></li>");
});

Expand All @@ -376,6 +419,7 @@ define(function (require, exports, module) {
event.keyCode === KeyEvent.DOM_VK_ENTER ||
event.keyCode === KeyEvent.DOM_VK_SPACE) {
// Enter/Space is same as clicking on swatch

self._commitColor($(event.currentTarget).find(".value").html());
} else if (event.keyCode === KeyEvent.DOM_VK_TAB) {
// Tab on last swatch loops back to color square
Expand Down Expand Up @@ -427,6 +471,9 @@ define(function (require, exports, module) {
case "name":
colorVal = this._hsv.a < 1 ? newColor.toRgbString() : newColor.toHexString();
break;
case "0x":
colorVal = as0xString(newColor);
break;
}
colorVal = this._isUpperCase ? colorVal.toUpperCase() : colorVal;
this._commitColor(colorVal, false);
Expand All @@ -439,11 +486,15 @@ define(function (require, exports, module) {
* @param {boolean=} resetHsv Pass false ONLY if hsv set already been modified to match colorVal. Default: true.
*/
ColorEditor.prototype._commitColor = function (colorVal, resetHsv) {

if (resetHsv === undefined) {
resetHsv = true;
}
this._callback(colorVal);
this._color = tinycolor(colorVal);

var colorObj = checkSetFormat(colorVal);
colorObj._originalInput = colorVal;
this._color = colorObj;

if (resetHsv) {
this._hsv = this._color.toHsv();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
<li class="selected" title="{{COLOR_EDITOR_RGBA_BUTTON_TIP}}"><a href="#" tabindex="0" class="rgba">RGBa</a></li>
<li title="{{COLOR_EDITOR_HEX_BUTTON_TIP}}"><a href="#" tabindex="0" class="hex">Hex</a></li>
<li title="{{COLOR_EDITOR_HSLA_BUTTON_TIP}}"><a href="#" tabindex="0" class="hsla">HSLa</a></li>
<li title="{{COLOR_EDITOR_0X_BUTTON_TIP}}"><a href="#" tabindex="0" class="0x">0x</a></li>
</ul>
</footer>
</section>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ define(function (require, exports, module) {
var self = this;
if (colorString !== this._color) {
var range = this.getCurrentRange();

if (!range) {
return;
}
Expand Down
9 changes: 6 additions & 3 deletions src/extensions/default/QuickView/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -400,15 +400,18 @@ define(function (require, exports, module) {
}
} else if (pos.ch <= match.index + match[0].length) {
// build the css for previewing the gradient from the regex result
var previewCSS = gradientMatch.prefix + (gradientMatch.colorValue || match[0]);
var previewCSS = gradientMatch.prefix + (gradientMatch.colorValue || match[0]);

if ((/^0x/).test(previewCSS)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be a function like:

function ensureHexFormat(str) {
  return (/^0x/).test(str) ? str.replace("0x","#") : str;
}

adding constructs like these without explanation makes the code less readable to me

previewCSS = previewCSS.replace("0x","#");
};

// normalize the arguments to something that we can display to the user
// NOTE: we need both the div and the popover's _previewCSS member
// (used by unit tests) to match so normalize the css for both
previewCSS = normalizeGradientExpressionForQuickview(previewCSS);

var preview = "<div class='color-swatch' style='background:" + previewCSS + "'>" +
"</div>";
var preview = "<div class='color-swatch' style='background:" + previewCSS + "'>" + "</div>";
var startPos = {line: pos.line, ch: match.index},
endPos = {line: pos.line, ch: match.index + match[0].length},
startCoords = cm.charCoords(startPos),
Expand Down
1 change: 1 addition & 0 deletions src/nls/root/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,7 @@ define({
"COLOR_EDITOR_RGBA_BUTTON_TIP" : "RGBa Format",
"COLOR_EDITOR_HEX_BUTTON_TIP" : "Hex Format",
"COLOR_EDITOR_HSLA_BUTTON_TIP" : "HSLa Format",
"COLOR_EDITOR_0X_BUTTON_TIP" : "Hex (0x) Format",
"COLOR_EDITOR_USED_COLOR_TIP_SINGULAR" : "{0} (Used {1} time)",
"COLOR_EDITOR_USED_COLOR_TIP_PLURAL" : "{0} (Used {1} times)",

Expand Down
4 changes: 2 additions & 2 deletions src/utils/ColorUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ define(function (require, exports, module) {

/**
* Regular expression that matches reasonably well-formed colors in hex format (3 or 6 digits),
* rgb()/rgba() function format, hsl()/hsla() function format,
* rgb()/rgba() function format, hsl()/hsla() function format, 0x notation format
* or color name format according to CSS Color Module Level 3 (http://www.w3.org/TR/css3-color/)
* or "rebeccapurple" from CSS Color Module Level 4.
* @const @type {RegExp}
*/
// 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");
var COLOR_REGEX = new RegExp(/0x([a-f0-9]{6})\b|#[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.
Expand Down