Skip to content

Commit

Permalink
Merge pull request #3341 from plotly/colorscale-no-mutate
Browse files Browse the repository at this point in the history
Don't mutate colorscale, cmin/cmax and zmin/zmax into user traces
  • Loading branch information
etpinard committed Dec 17, 2018
2 parents d205d68 + 596bb47 commit e398676
Show file tree
Hide file tree
Showing 57 changed files with 950 additions and 559 deletions.
8 changes: 6 additions & 2 deletions src/components/colorbar/connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
* LICENSE file in the root directory of this source tree.
*/


'use strict';

var drawColorbar = require('./draw');
var flipScale = require('../colorscale/helpers').flipScale;

/**
* connectColorbar: create a colorbar from a trace, using its module to
Expand Down Expand Up @@ -49,7 +49,11 @@ module.exports = function connectColorbar(gd, cd, moduleOpts) {

var cb = cd[0].t.cb = drawColorbar(gd, cbId);

cb.fillgradient(container.colorscale)
var scl = container.reversescale ?
flipScale(container.colorscale) :
container.colorscale;

cb.fillgradient(scl)
.zrange([container[moduleOpts.min], container[moduleOpts.max]])
.options(container.colorbar)();
};
4 changes: 2 additions & 2 deletions src/components/colorscale/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

'use strict';

var palettes = require('./scales.js');
var palettes = require('./scales.js').scales;
var paletteStr = Object.keys(palettes);

function code(s) {
Expand Down Expand Up @@ -197,7 +197,7 @@ module.exports = function colorScaleAttrs(context, opts) {
valType: 'boolean',
role: 'style',
dflt: false,
editType: 'calc',
editType: 'plot',
description: [
'Reverses the color mapping if true.',
effectDesc,
Expand Down
65 changes: 7 additions & 58 deletions src/components/colorscale/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,52 +6,19 @@
* LICENSE file in the root directory of this source tree.
*/


'use strict';

var Lib = require('../../lib');

var flipScale = require('./flip_scale');


module.exports = function calc(gd, trace, opts) {
var fullLayout = gd._fullLayout;
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';
Expand All @@ -74,32 +41,14 @@ module.exports = function calc(gd, trace, opts) {
max += 0.5;
}

doUpdate(minAttr, min);
doUpdate(maxAttr, max);

/*
* 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)));
container['_' + minAttr] = container[minAttr] = min;
container['_' + maxAttr] = container[maxAttr] = max;

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, container.reversescale ? flipScale(scl) : 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;
}
};
70 changes: 70 additions & 0 deletions src/components/colorscale/cross_trace_defaults.js
Original file line number Diff line number Diff line change
@@ -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'
});
}
}
};
14 changes: 0 additions & 14 deletions src/components/colorscale/default_scale.js

This file was deleted.

47 changes: 22 additions & 25 deletions src/components/colorscale/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,53 +6,50 @@
* LICENSE file in the root directory of this source tree.
*/


'use strict';

var isNumeric = require('fast-isnumeric');

var Lib = require('../../lib');

var hasColorbar = require('../colorbar/has_colorbar');
var colorbarDefaults = require('../colorbar/defaults');
var isValidScale = require('./is_valid_scale');
var flipScale = require('./flip_scale');

var isValidScale = require('./scales').isValid;

module.exports = function colorScaleDefaults(traceIn, traceOut, layout, coerce, opts) {
var prefix = opts.prefix,
cLetter = opts.cLetter,
containerStr = prefix.slice(0, prefix.length - 1),
containerIn = prefix ?
Lib.nestedProperty(traceIn, containerStr).get() || {} :
traceIn,
containerOut = prefix ?
Lib.nestedProperty(traceOut, containerStr).get() || {} :
traceOut,
minIn = containerIn[cLetter + 'min'],
maxIn = containerIn[cLetter + 'max'],
sclIn = containerIn.colorscale;
function npMaybe(cont, prefix) {
var containerStr = prefix.slice(0, prefix.length - 1);
return prefix ?
Lib.nestedProperty(cont, containerStr).get() || {} :
cont;
}

module.exports = function colorScaleDefaults(traceIn, traceOut, layout, coerce, opts) {
var prefix = opts.prefix;
var cLetter = opts.cLetter;
var containerIn = npMaybe(traceIn, prefix);
var containerOut = npMaybe(traceOut, prefix);
var template = npMaybe(traceOut._template || {}, prefix) || {};

var minIn = containerIn[cLetter + 'min'];
var maxIn = containerIn[cLetter + 'max'];
var validMinMax = isNumeric(minIn) && isNumeric(maxIn) && (minIn < maxIn);
coerce(prefix + cLetter + 'auto', !validMinMax);
coerce(prefix + cLetter + 'min');
coerce(prefix + cLetter + 'max');

// handles both the trace case (autocolorscale is false by default) and
// the marker and marker.line case (autocolorscale is true by default)
var sclIn = containerIn.colorscale;
var sclTemplate = template.colorscale;
var autoColorscaleDflt;
if(sclIn !== undefined) autoColorscaleDflt = !isValidScale(sclIn);
if(sclTemplate !== undefined) autoColorscaleDflt = !isValidScale(sclTemplate);
coerce(prefix + 'autocolorscale', autoColorscaleDflt);
var sclOut = coerce(prefix + 'colorscale');

// reversescale is handled at the containerOut level
var reverseScale = coerce(prefix + 'reversescale');
if(reverseScale) containerOut.colorscale = flipScale(sclOut);

// ... until Scatter.colorbar can handle marker line colorbars
if(prefix === 'marker.line.') return;
coerce(prefix + 'colorscale');
coerce(prefix + 'reversescale');

if(!opts.noScale) {
if(!opts.noScale && prefix !== 'marker.line.') {
// handles both the trace case where the dflt is listed in attributes and
// the marker case where the dflt is determined by hasColorbar
var showScaleDflt;
Expand Down
35 changes: 0 additions & 35 deletions src/components/colorscale/extract_scale.js

This file was deleted.

23 changes: 0 additions & 23 deletions src/components/colorscale/flip_scale.js

This file was deleted.

38 changes: 0 additions & 38 deletions src/components/colorscale/get_scale.js

This file was deleted.

Loading

0 comments on commit e398676

Please sign in to comment.