Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't mutate colorscale, cmin/cmax and zmin/zmax into user traces #3341

Merged
merged 12 commits into from
Dec 17, 2018
Merged
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