Skip to content

Commit e6d8d17

Browse files
authored
Merge pull request #754 from plotly/scatter3d-mlc-inherit
Fix scatter3d marker.line.color inheritance
2 parents 20a2af0 + db8ed5e commit e6d8d17

File tree

5 files changed

+82
-25
lines changed

5 files changed

+82
-25
lines changed

src/components/colorscale/calc.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ module.exports = function calc(trace, vals, containerStr, cLetter) {
2121
if(containerStr) {
2222
container = Lib.nestedProperty(trace, containerStr).get();
2323
inputContainer = Lib.nestedProperty(trace._input, containerStr).get();
24-
} else {
24+
}
25+
else {
2526
container = trace;
2627
inputContainer = trace._input;
2728
}

src/traces/scatter/colorscale_calc.js

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,15 @@ var calcColorscale = require('../../components/colorscale/calc');
1515
var subTypes = require('./subtypes');
1616

1717

18-
// common to 'scatter', 'scatter3d' and 'scattergeo'
1918
module.exports = function calcMarkerColorscale(trace) {
20-
21-
// auto-z and autocolorscale if applicable
22-
2319
if(subTypes.hasLines(trace) && hasColorscale(trace, 'line')) {
2420
calcColorscale(trace, trace.line.color, 'line', 'c');
2521
}
2622

2723
if(subTypes.hasMarkers(trace)) {
28-
2924
if(hasColorscale(trace, 'marker')) {
3025
calcColorscale(trace, trace.marker.color, 'marker', 'c');
3126
}
32-
3327
if(hasColorscale(trace, 'marker.line')) {
3428
calcColorscale(trace, trace.marker.line.color, 'marker.line', 'c');
3529
}

src/traces/scatter/line_defaults.js

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,18 @@ var hasColorscale = require('../../components/colorscale/has_colorscale');
1313
var colorscaleDefaults = require('../../components/colorscale/defaults');
1414

1515

16-
// common to 'scatter', 'scatter3d', 'scattergeo' and 'scattergl'
1716
module.exports = function lineDefaults(traceIn, traceOut, defaultColor, layout, coerce) {
18-
1917
var markerColor = (traceIn.marker || {}).color;
2018

2119
coerce('line.color', defaultColor);
20+
2221
if(hasColorscale(traceIn, 'line')) {
23-
colorscaleDefaults(
24-
traceIn, traceOut, layout, coerce, {prefix: 'line.', cLetter: 'c'}
25-
);
26-
} else {
27-
coerce('line.color', (Array.isArray(markerColor) ? false : markerColor) ||
28-
defaultColor);
22+
colorscaleDefaults(traceIn, traceOut, layout, coerce, {prefix: 'line.', cLetter: 'c'});
23+
}
24+
else {
25+
var lineColorDflt = (Array.isArray(markerColor) ? false : markerColor) || defaultColor;
26+
coerce('line.color', lineColorDflt);
2927
}
30-
3128

3229
coerce('line.width');
3330
coerce('line.dash');

src/traces/scatter/marker_defaults.js

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ var colorscaleDefaults = require('../../components/colorscale/defaults');
1616
var subTypes = require('./subtypes');
1717

1818

19-
// common to 'scatter', 'scatter3d', 'scattergeo' and 'scattergl'
2019
module.exports = function markerDefaults(traceIn, traceOut, defaultColor, layout, coerce) {
2120
var isBubble = subTypes.isBubble(traceIn),
22-
lineColor = !Array.isArray(traceIn.line) ? (traceIn.line || {}).color : undefined,
21+
lineColor = (traceIn.line || {}).color,
2322
defaultMLC;
2423

24+
// marker.color inherit from line.color (even if line.color is an array)
2525
if(lineColor) defaultColor = lineColor;
2626

2727
coerce('marker.symbol');
@@ -30,25 +30,22 @@ module.exports = function markerDefaults(traceIn, traceOut, defaultColor, layout
3030

3131
coerce('marker.color', defaultColor);
3232
if(hasColorscale(traceIn, 'marker')) {
33-
colorscaleDefaults(
34-
traceIn, traceOut, layout, coerce, {prefix: 'marker.', cLetter: 'c'}
35-
);
33+
colorscaleDefaults(traceIn, traceOut, layout, coerce, {prefix: 'marker.', cLetter: 'c'});
3634
}
3735

3836
// if there's a line with a different color than the marker, use
3937
// that line color as the default marker line color
38+
// (except when it's an array)
4039
// mostly this is for transparent markers to behave nicely
41-
if(lineColor && (traceOut.marker.color !== lineColor)) {
40+
if(lineColor && !Array.isArray(lineColor) && (traceOut.marker.color !== lineColor)) {
4241
defaultMLC = lineColor;
4342
}
4443
else if(isBubble) defaultMLC = Color.background;
4544
else defaultMLC = Color.defaultLine;
4645

4746
coerce('marker.line.color', defaultMLC);
4847
if(hasColorscale(traceIn, 'marker.line')) {
49-
colorscaleDefaults(
50-
traceIn, traceOut, layout, coerce, {prefix: 'marker.line.', cLetter: 'c'}
51-
);
48+
colorscaleDefaults(traceIn, traceOut, layout, coerce, {prefix: 'marker.line.', cLetter: 'c'});
5249
}
5350

5451
coerce('marker.line.width', isBubble ? 1 : 0);

test/jasmine/tests/scatter3d_test.js

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
var Scatter3D = require('@src/traces/scatter3d');
2+
var Lib = require('@src/lib');
3+
var Color = require('@src/components/color');
4+
5+
6+
describe('Scatter3D defaults', function() {
7+
'use strict';
8+
9+
var defaultColor = '#d3d3d3';
10+
11+
function _supply(traceIn) {
12+
var traceOut = { visible: true },
13+
layout = { _dataLength: 1 };
14+
15+
Scatter3D.supplyDefaults(traceIn, traceOut, defaultColor, layout);
16+
return traceOut;
17+
}
18+
19+
var base = {
20+
x: [1, 2, 3],
21+
y: [1, 2, 3],
22+
z: [1, 2, 1]
23+
};
24+
25+
it('should make marker.color inherit from line.color (scalar case)', function() {
26+
var out = _supply(Lib.extendFlat({}, base, {
27+
line: { color: 'red' }
28+
}));
29+
30+
expect(out.line.color).toEqual('red');
31+
expect(out.marker.color).toEqual('red');
32+
expect(out.marker.line.color).toBe(Color.defaultLine, 'but not marker.line.color');
33+
});
34+
35+
it('should make marker.color inherit from line.color (array case)', function() {
36+
var color = [1, 2, 3];
37+
38+
var out = _supply(Lib.extendFlat({}, base, {
39+
line: { color: color }
40+
}));
41+
42+
expect(out.line.color).toBe(color);
43+
expect(out.marker.color).toBe(color);
44+
expect(out.marker.line.color).toBe(Color.defaultLine, 'but not marker.line.color');
45+
});
46+
47+
it('should make line.color inherit from marker.color if scalar)', function() {
48+
var out = _supply(Lib.extendFlat({}, base, {
49+
marker: { color: 'red' }
50+
}));
51+
52+
expect(out.line.color).toEqual('red');
53+
expect(out.marker.color).toEqual('red');
54+
expect(out.marker.line.color).toBe(Color.defaultLine);
55+
});
56+
57+
it('should not make line.color inherit from marker.color if array', function() {
58+
var color = [1, 2, 3];
59+
60+
var out = _supply(Lib.extendFlat({}, base, {
61+
marker: { color: color }
62+
}));
63+
64+
expect(out.line.color).toBe(defaultColor);
65+
expect(out.marker.color).toBe(color);
66+
expect(out.marker.line.color).toBe(Color.defaultLine);
67+
});
68+
});

0 commit comments

Comments
 (0)