fixes #3273 - add Colorscale.crossTraceDefaults

- to 'relink' zmin/zmax, cmin/cmax and auto colorscale, these values are computed in 'calc' in need to be relinked in 'supply-defaults' in order to work properly after edits that don't go through 'calc'.
plotly · etpinard · Dec 17, 2018 · Dec 13, 2018 · Dec 13, 2018 · Dec 14, 2018
commit c9d92d7942d6eae4eaf7e204e43c5dbf90441369
diff --git a/src/components/colorscale/calc.js b/src/components/colorscale/calc.js
@@ -15,39 +15,10 @@ module.exports = function calc(gd, trace, opts) {
    var vals = opts.vals;
    var containerStr = opts.containerStr;
    var cLetter = opts.cLetter;
-    var container = trace;
-    var inputContainer = trace._input;
-    var fullInputContainer = trace._fullInput;

-    // set by traces with groupby transforms
-    var updateStyle = trace.updateStyle;
-
-    function doUpdate(attr, inputVal, fullVal) {
-        if(fullVal === undefined) fullVal = inputVal;
-
-        if(updateStyle) {
-            updateStyle(trace._input, containerStr ? (containerStr + '.' + attr) : attr, inputVal);
-        }
-        else {
-            inputContainer[attr] = inputVal;
-        }
-
-        container[attr] = fullVal;
-        if(fullInputContainer && (trace !== trace._fullInput)) {
-            if(updateStyle) {
-                updateStyle(trace._fullInput, containerStr ? (containerStr + '.' + attr) : attr, fullVal);
-            }
-            else {
-                fullInputContainer[attr] = fullVal;
-            }
-        }
-    }
-
-    if(containerStr) {
-        container = Lib.nestedProperty(container, containerStr).get();
-        inputContainer = Lib.nestedProperty(inputContainer, containerStr).get();
-        fullInputContainer = Lib.nestedProperty(fullInputContainer, containerStr).get() || {};
-    }
+    var container = containerStr ?
+        Lib.nestedProperty(trace, containerStr).get() :
+        trace;

    var autoAttr = cLetter + 'auto';
    var minAttr = cLetter + 'min';
@@ -70,32 +41,24 @@ module.exports = function calc(gd, trace, opts) {
        max += 0.5;
    }

-    doUpdate(minAttr, min);
-    doUpdate(maxAttr, max);
+    container['_' + minAttr] = container[minAttr] = min;
+    container['_' + maxAttr] = container[maxAttr] = max;

+    // TODO ?!?
    /*
     * If auto was explicitly false but min or max was missing,
     * we filled in the missing piece here but later the trace does
     * not look auto.
     * Otherwise make sure the trace still looks auto as far as later
     * changes are concerned.
     */
-    doUpdate(autoAttr, (auto !== false || (min === undefined && max === undefined)));
+    // doUpdate(autoAttr, (auto !== false || (min === undefined && max === undefined)));

    if(container.autocolorscale) {
        if(min * max < 0) scl = fullLayout.colorscale.diverging;
        else if(min >= 0) scl = fullLayout.colorscale.sequential;
-        else scl = gd._fullLayout.colorscale.sequentialminus;
-
-        // reversescale is handled at the containerOut level
-        doUpdate('colorscale', scl);
+        else scl = fullLayout.colorscale.sequentialminus;

-        // We pushed a colorscale back to input, which will change the default autocolorscale next time
-        // to avoid spurious redraws from Plotly.react, update resulting autocolorscale now
-        // This is a conscious decision so that changing the data later does not unexpectedly
-        // give you a new colorscale
-        if(!inputContainer.autocolorscale) {
-            doUpdate('autocolorscale', false);
-        }
+        container._colorscale = container.colorscale = scl;
    }
 };
diff --git a/src/components/colorscale/cross_trace_defaults.js b/src/components/colorscale/cross_trace_defaults.js
@@ -0,0 +1,70 @@
+/**
+* Copyright 2012-2018, Plotly, Inc.
+* All rights reserved.
+*
+* This source code is licensed under the MIT license found in the
+* LICENSE file in the root directory of this source tree.
+*/
+
+'use strict';
+
+var Lib = require('../../lib');
+var hasColorscale = require('./helpers').hasColorscale;
+
+module.exports = function crossTraceDefaults(fullData) {
+    function replace(cont, k) {
+        var val = cont['_' + k];
+        if(val !== undefined) {
+            cont[k] = val;
+        }
+    }
+
+    function relinkColorAtts(trace, cAttrs) {
+        var cont = cAttrs.container ?
+            Lib.nestedProperty(trace, cAttrs.container).get() :
+            trace;
+
+        if(cont) {
+            var isAuto = cont.zauto || cont.cauto;
+            var minAttr = cAttrs.min;
+            var maxAttr = cAttrs.max;
+
+            if(isAuto || cont[minAttr] === undefined) {
+                replace(cont, minAttr);
+            }
+            if(isAuto || cont[maxAttr] === undefined) {
+                replace(cont, maxAttr);
+            }
+            if(cont.autocolorscale) {
+                replace(cont, 'colorscale');
+            }
+        }
+    }
+
+    for(var i = 0; i < fullData.length; i++) {
+        var trace = fullData[i];
+        var _module = trace._module;
+
+        if(_module.colorbar) {
+            relinkColorAtts(trace, _module.colorbar);
+        }
+
+        // TODO could generalize _module.colorscale and use it here?
+
+        if(hasColorscale(trace, 'marker.line')) {
+            relinkColorAtts(trace, {
+                container: 'marker.line',
+                min: 'cmin',
+                max: 'cmax'
+            });
+        }
+
+        if(hasColorscale(trace, 'line')) {
+            relinkColorAtts(trace, {
+                container: 'line',
+                min: 'cmin',
+                max: 'cmax'
+            });
+        }
+    }
+};
diff --git a/src/components/colorscale/index.js b/src/components/colorscale/index.js
@@ -20,6 +20,7 @@ module.exports = {

    supplyLayoutDefaults: require('./layout_defaults'),
    handleDefaults: require('./defaults'),
+    crossTraceDefaults: require('./cross_trace_defaults'),

    calc: require('./calc'),


diff --git a/src/plots/plots.js b/src/plots/plots.js
@@ -449,6 +449,7 @@ plots.supplyDefaults = function(gd, opts) {
    for(i = 0; i < crossTraceDefaultsFuncs.length; i++) {
        crossTraceDefaultsFuncs[i](newFullData, newFullLayout);
    }
+    Registry.getComponentMethod('colorscale', 'crossTraceDefaults')(newFullData, newFullLayout);

    // turn on flag to optimize large splom-only graphs
    // mostly by omitting SVG layers during Cartesian.drawFramework

diff --git a/test/jasmine/tests/colorscale_test.js b/test/jasmine/tests/colorscale_test.js
@@ -406,21 +406,6 @@ describe('Test colorscale:', function() {
            expect(trace.colorscale).toEqual(colorscale);
        });

-        it('should set autocolorscale to false if it wasn\'t explicitly set true in input', function() {
-            trace = {
-                type: 'heatmap',
-                z: [[0, -1.5], [-2, -10]],
-                zmin: -10,
-                zmax: 0,
-                autocolorscale: true,
-                _input: {}
-            };
-            gd = _supply(trace);
-            calcColorscale(gd, trace, {vals: trace.z, containerStr: '', cLetter: 'z'});
-            expect(trace.autocolorscale).toBe(false);
-            expect(trace.colorscale[5]).toEqual([1, 'rgb(220,220,220)']);
-        });
-
        it('should be Blues when the only numerical z <= -0.5', function() {
            trace = {
                type: 'heatmap',

diff --git a/test/jasmine/tests/contour_test.js b/test/jasmine/tests/contour_test.js
@@ -309,14 +309,7 @@ describe('contour calc', function() {

                ['start', 'end', 'size'].forEach(function(attr) {
                    expect(out.contours[attr]).toBe(spec[attr], [contoursIn, spec.inputNcontours, attr]);
-                    // all these get copied back to the input trace
-                    expect(out._input.contours[attr]).toBe(spec[attr], [contoursIn, spec.inputNcontours, attr]);
                });
-
-                expect(out._input.autocontour).toBe(true);
-                expect(out._input.zauto).toBe(true);
-                expect(out._input.zmin).toBe(0);
-                expect(out._input.zmax).toBe(5);
            });
        });
    });

diff --git a/test/jasmine/tests/mesh3d_test.js b/test/jasmine/tests/mesh3d_test.js
@@ -14,11 +14,11 @@ describe('Test mesh3d restyle', function() {
            var fullTrace = gd._fullData[0];

            expect(trace.cauto).toBe(user[0], 'user cauto');
-            expect(trace.cmin).toEqual(user[1], 'user cmin');
-            expect(trace.cmax).toEqual(user[2], 'user cmax');
+            expect(trace.cmin).toBe(user[1], 'user cmin');
+            expect(trace.cmax).toBe(user[2], 'user cmax');
            expect(fullTrace.cauto).toBe(full[0], 'full cauto');
-            expect(fullTrace.cmin).toEqual(full[1], 'full cmin');
-            expect(fullTrace.cmax).toEqual(full[2], 'full cmax');
+            expect(fullTrace.cmin).toBe(full[1], 'full cmin');
+            expect(fullTrace.cmax).toBe(full[2], 'full cmax');
        }

        Plotly.plot(gd, [{
@@ -32,12 +32,12 @@ describe('Test mesh3d restyle', function() {
            intensity: [0, 0.33, 0.66, 3]
        }])
        .then(function() {
-            _assert([true, 0, 3], [true, 0, 3]);
+            _assert([undefined, undefined, undefined], [true, 0, 3]);

            return Plotly.restyle(gd, 'cmin', 0);
        })
        .then(function() {
-            _assert([false, 0, 3], [false, 0, 3]);
+            _assert([false, 0, undefined], [false, 0, 3]);

            return Plotly.restyle(gd, 'cmax', 10);
        })
@@ -47,7 +47,7 @@ describe('Test mesh3d restyle', function() {
            return Plotly.restyle(gd, 'cauto', true);
        })
        .then(function() {
-            _assert([true, 0, 3], [true, 0, 3]);
+            _assert([true, 0, 10], [true, 0, 3]);

            return Plotly.purge(gd);
        })

diff --git a/test/jasmine/tests/parcoords_test.js b/test/jasmine/tests/parcoords_test.js
@@ -284,6 +284,8 @@ describe('parcoords initialization tests', function() {
                cauto: true,
                cmin: 21,
                cmax: 63,
+                _cmin: 21,
+                _cmax: 63,
                autocolorscale: false,
                reversescale: false,
                showscale: false

diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js
@@ -1052,18 +1052,17 @@ describe('Test plot api', function() {

        it('should redo auto z/contour when editing z array', function(done) {
            Plotly.plot(gd, [{type: 'contour', z: [[1, 2], [3, 4]]}]).then(function() {
-                expect(gd.data[0].zauto).toBe(true, gd.data[0]);
-                expect(gd.data[0].zmin).toBe(1);
-                expect(gd.data[0].zmax).toBe(4);
-
+                expect(gd._fullData[0].zauto).toBe(true);
+                expect(gd._fullData[0].zmin).toBe(1);
+                expect(gd._fullData[0].zmax).toBe(4);
                expect(gd.data[0].autocontour).toBe(true);
                expect(gd.data[0].contours).toEqual({start: 1.5, end: 3.5, size: 0.5});

                return Plotly.restyle(gd, {'z[0][0]': 10});
            }).then(function() {
-                expect(gd.data[0].zmin).toBe(2);
-                expect(gd.data[0].zmax).toBe(10);
-
+                expect(gd._fullData[0].zauto).toBe(true);
+                expect(gd._fullData[0].zmin).toBe(2);
+                expect(gd._fullData[0].zmax).toBe(10);
                expect(gd.data[0].contours).toEqual({start: 3, end: 9, size: 1});
            })
            .catch(failTest)
@@ -1111,10 +1110,10 @@ describe('Test plot api', function() {
            var zmax1 = 10;

            function check(auto, msg) {
-                expect(gd.data[0].zmin).negateIf(auto).toBe(zmin0, msg);
-                expect(gd.data[0].zauto).toBe(auto, msg);
-                expect(gd.data[1].zmax).negateIf(auto).toBe(zmax1, msg);
-                expect(gd.data[1].zauto).toBe(auto, msg);
+                expect(gd._fullData[0].zmin).negateIf(auto).toBe(zmin0, msg);
+                expect(gd._fullData[0].zauto).toBe(auto, msg);
+                expect(gd._fullData[1].zmax).negateIf(auto).toBe(zmax1, msg);
+                expect(gd._fullData[1].zauto).toBe(auto, msg);
            }

            Plotly.plot(gd, [
@@ -1144,32 +1143,32 @@ describe('Test plot api', function() {
        });

        it('turns off cauto (autocolorscale) when you edit cmin or cmax (colorscale)', function(done) {
-            var autocscale = require('@src/components/colorscale/scales').scales.Reds;
+            var scales = require('@src/components/colorscale/scales').scales;

+            var autocscale = scales.Reds;
+            var mcscl0 = 'Rainbow';
+            var mlcscl1 = 'Greens';
            var mcmin0 = 3;
-            var mcscl0 = 'rainbow';
            var mlcmax1 = 6;
-            var mlcscl1 = 'greens';

            function check(auto, autocolorscale, msg) {
-                expect(gd.data[0].marker.cauto).toBe(auto, msg);
-                expect(gd.data[0].marker.cmin).negateIf(auto).toBe(mcmin0);
+                expect(gd._fullData[0].marker.cauto).toBe(auto, msg);
+                expect(gd._fullData[0].marker.cmin).negateIf(auto).toBe(mcmin0);
                expect(gd._fullData[0].marker.autocolorscale).toBe(autocolorscale, msg);
-                expect(gd.data[0].marker.colorscale).toEqual(auto ? autocscale : mcscl0);
-                expect(gd.data[1].marker.line.cauto).toBe(auto, msg);
-                expect(gd.data[1].marker.line.cmax).negateIf(auto).toBe(mlcmax1);
+                expect(gd._fullData[0].marker.colorscale).toEqual(auto ? autocscale : scales[mcscl0]);
+
+                expect(gd._fullData[1].marker.line.cauto).toBe(auto, msg);
+                expect(gd._fullData[1].marker.line.cmax).negateIf(auto).toBe(mlcmax1);
                expect(gd._fullData[1].marker.line.autocolorscale).toBe(autocolorscale, msg);
-                expect(gd.data[1].marker.line.colorscale).toEqual(auto ? autocscale : mlcscl1);
+                expect(gd._fullData[1].marker.line.colorscale).toEqual(auto ? autocscale : scales[mlcscl1]);
            }

            Plotly.plot(gd, [
                {y: [1, 2], mode: 'markers', marker: {color: [1, 10]}},
                {y: [2, 1], mode: 'markers', marker: {line: {width: 2, color: [3, 4]}}}
            ])
            .then(function() {
-                // autocolorscale is actually true after supplyDefaults, but during calc it's set
-                // to false when we push the resulting colorscale back to the input container
-                check(true, false, 'initial');
+                check(true, true, 'initial');
                return Plotly.restyle(gd, {'marker.cmin': mcmin0, 'marker.colorscale': mcscl0}, null, [0]);
            })
            .then(function() {