fixes #3273 - add Colorscale.crossTraceDefaults

etpinard · etpinard · commit c9d92d7942d6 · 2018-12-17T12:06:27.000-05:00
- 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'.
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() {

Original file line number	Diff line number	Diff line change
`@@ -449,6 +449,7 @@ plots.supplyDefaults = function(gd, opts) {`
`449`	`449`	`for(i = 0; i < crossTraceDefaultsFuncs.length; i++) {`
`450`	`450`	`crossTraceDefaultsFuncs[i](newFullData, newFullLayout);`
`451`	`451`	`}`
	`452`	`+ Registry.getComponentMethod('colorscale', 'crossTraceDefaults')(newFullData, newFullLayout);`
`452`	`453`
`453`	`454`	`// turn on flag to optimize large splom-only graphs`
`454`	`455`	`// mostly by omitting SVG layers during Cartesian.drawFramework`