Refactor to better match tinycolor2

Author: camdecoster

src/components/color/index.js

@@ -1,11 +1,18 @@
-'use strict';
-
+const _color = require('color').default;
 const isNumeric = require('fast-isnumeric');
-const isTypedArray = require('../../lib/array').isTypedArray;
-const color = require('color').default;
-
+const { isTypedArray } = require('../../lib/array');
 const { background, defaultLine, defaults, lightLine } = require('./attributes');
 
+// Safe wrapper: falls back to black instead of throwing on invalid input.
+// This matches the tinycolor2 behavior.
+const color = (cstr) => {
+    try {
+        return _color(cstr);
+    } catch (e) {
+        return _color('#000');
+    }
+};
+
 const rgb = (cstr) => {
     const { r, g, b } = color(cstr).rgb().object();
     return `rgb(${Math.round(r)}, ${Math.round(g)}, ${Math.round(b)})`;
@@ -79,15 +86,12 @@ const contrast = (cstr, lightAmount, darkAmount) => {
     if (c.alpha() !== 1) c = color(combine(cstr, background));
 
     // TODO: Should the API change such that lightAmount/darkAmount are passed in as decimal instead of percent number?
-    const newColor = color(
-        c.isDark()
-            ? lightAmount
-                ? c.lighten(lightAmount / 100)
-                : background
-            : darkAmount
-              ? c.darken(darkAmount / 100)
-              : defaultLine
-    );
+    let newColor;
+    if (c.isDark()) {
+        newColor = color(lightAmount ? c.lighten(lightAmount / 100) : background);
+    } else {
+        newColor = color(darkAmount ? c.darken(darkAmount / 100) : defaultLine);
+    }
 
     return newColor.rgb().string();
 };
@@ -175,39 +179,52 @@ const equals = (cstr1, cstr2) => cstr1 && cstr2 && color(cstr1).rgb().string() =
 
 const isValid = (cstr) => {
     try {
-        return cstr && !!color(cstr);
+        return cstr && !!_color(cstr);
     } catch {
         return false;
     }
 };
 
+// RGB-space brightening equivalent to tinycolor's brighten().
+// Adds a fixed amount to each RGB channel (unlike lighten which works in HSL space).
+const brighten = (cstr, amount) => {
+    amount = amount === 0 ? 0 : amount || 10;
+    const c = color(cstr).rgb().object();
+    const adj = Math.round(255 * (amount / 100));
+    return color({
+        r: Math.max(0, Math.min(255, c.r + adj)),
+        g: Math.max(0, Math.min(255, c.g + adj)),
+        b: Math.max(0, Math.min(255, c.b + adj))
+    })
+        .rgb()
+        .string();
+};
+
 const mix = (cstr1, cstr2, weight) =>
     color(cstr1)
         .mix(color(cstr2), weight / 100)
         .rgb()
         .string();
 
-const mostReadable = (baseColor, colorList = []) => {
+const mostReadable = (baseColor, colorList = ['#000', '#fff']) => {
     let bestColor;
     let bestContrast = -Infinity;
 
     for (const cstr of colorList) {
-        const contrast = color(baseColor).contrast(color(cstr));
-        if (contrast > bestContrast) {
-            bestContrast = contrast;
+        const contrastRatio = color(baseColor).contrast(color(cstr));
+        if (contrastRatio > bestContrast) {
+            bestContrast = contrastRatio;
             bestColor = color(cstr).rgb().string();
         }
     }
 
-    // Fall back to black/white if provided colors don't have proper contrast level
-    return bestColor && color(baseColor).level(color(bestColor))
-        ? bestColor
-        : mostReadable(baseColor, ['#000', '#fff']);
+    return bestColor;
 };
 
 module.exports = {
     addOpacity,
     background,
+    brighten,
     clean,
     color,
     combine,

src/traces/sankey/defaults.js

@@ -37,11 +37,12 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
 
     var colors = layout.colorway;
 
-    var defaultNodePalette = function(i) {return colors[i % colors.length];};
+    const defaultNodePalette = (i) => colors[i % colors.length];
 
-    coerceNode('color', nodeOut.label.map(function(d, i) {
-        return Color.addOpacity(defaultNodePalette(i), 0.8);
-    }));
+    coerceNode(
+        'color',
+        nodeOut.label.map((d, i) => Color.addOpacity(defaultNodePalette(i), 0.8))
+    );
     coerceNode('customdata');
 
     // link attributes
@@ -68,19 +69,22 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
 
     function makeDefaultHoverColor(_linkColor) {
         // hopefully the user-specified color is valid, but if not that can be caught elsewhere
-        if(!Color.isValid(_linkColor)) return _linkColor;
+        if (!Color.isValid(_linkColor)) return _linkColor;
 
         const c = Color.color(_linkColor);
         const alpha = c.alpha();
 
         return alpha <= 0.8
-            ? c.alpha(alpha + 0.2).rgb().string()
-            : (darkBG ? c.lighten(0.1) : c.darken(0.1)).rgb().string()
+            ? c
+                  .alpha(alpha + 0.2)
+                  .rgb()
+                  .string()
+            : (darkBG ? c.lighten(0.1) : c.darken(0.1)).rgb().string();
     }
 
-    coerceLink('hovercolor', Array.isArray(linkColor) ?
-        linkColor.map(makeDefaultHoverColor) :
-        makeDefaultHoverColor(linkColor)
+    coerceLink(
+        'hovercolor',
+        Array.isArray(linkColor) ? linkColor.map(makeDefaultHoverColor) : makeDefaultHoverColor(linkColor)
     );
 
     coerceLink('customdata');
@@ -96,10 +100,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     coerce('valueformat');
     coerce('valuesuffix');
 
-    var dfltArrangement;
-    if(nodeOut.x.length && nodeOut.y.length) {
-        dfltArrangement = 'freeform';
-    }
+    const dfltArrangement = nodeOut.x.length && nodeOut.y.length ? 'freeform' : undefined;
     coerce('arrangement', dfltArrangement);
 
     Lib.coerceFont(coerce, 'textfont', layout.font, { autoShadowDflt: true });

test/image/mocks/test_mock.json

@@ -0,0 +1,50 @@
+{
+  "data": [{ "type": "scatter", "x": [1, 2], "y": [1, 2] }],
+  "layout": {
+    "margin": { "t": 92 },
+    "shapes": [
+      {
+        "label": {
+          "text": "Frankenstein",
+          "textangle": 0,
+          "textposition": "middle center",
+          "xanchor": "center",
+          "yanchor": "middle"
+        },
+        "showlegend": false,
+        "type": "circle",
+        "x0": -25,
+        "x1": 25,
+        "xanchor": 1,
+        "xref": "x",
+        "xsizemode": "pixel",
+        "y0": 25,
+        "y1": 75,
+        "yanchor": 1,
+        "yref": "paper",
+        "ysizemode": "pixel"
+      },
+      {
+        "label": {
+          "text": "Rectangle",
+          "textangle": 0,
+          "textposition": "middle center",
+          "xanchor": "center",
+          "yanchor": "middle"
+        },
+        "showlegend": false,
+        "type": "rectangle",
+        "x0": -25,
+        "x1": 25,
+        "xanchor": 2,
+        "xref": "x",
+        "xsizemode": "pixel",
+        "y0": 25,
+        "y1": 75,
+        "yanchor": 1,
+        "yref": "paper",
+        "ysizemode": "pixel"
+      }
+    ]
+  }
+}

test/jasmine/tests/color_test.js

@@ -56,9 +56,8 @@ describe('Test color:', function() {
         });
 
         it('should count 0 as a fraction but not 1, except in alpha', function() {
-            // this is weird... but old tinycolor actually breaks
-            // if you pass in a 1, while in some cases a 1 here
-            // could be ambiguous - so we treat it as a real 1.
+            // a value of 1 here could be ambiguous (fraction vs integer)
+            // so we treat it as a real 1.
             var container = {
                 fractioncolor: 'rgb(0, 0.4, 0.8)',
                 regularcolor: 'rgb(1, 0.5, 0.5)',