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

finalist: Errorbars inherit color from line or marker color #3408

Merged
merged 4 commits into from
Jan 14, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/traces/bar/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,13 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout

handleStyleDefaults(traceIn, traceOut, coerce, defaultColor, layout);

var lineColor = (traceOut.marker.line || {}).color;
var markerColor = (traceOut.marker || {}).color;

// override defaultColor for error bars with defaultLine
var errorBarsSupplyDefaults = Registry.getComponentMethod('errorbars', 'supplyDefaults');
errorBarsSupplyDefaults(traceIn, traceOut, Color.defaultLine, {axis: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, Color.defaultLine, {axis: 'x', inherit: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || Color.defaultLine, {axis: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || Color.defaultLine, {axis: 'x', inherit: 'y'});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think markerColor belongs here. That would be invisible against the bars themselves, right? I like lineColor though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes totally agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcjohnson there is also this PR for possibly only fixing this on scatter traces...


Lib.coerceSelectionMarkerOpacity(traceOut, coerce);
};
11 changes: 7 additions & 4 deletions src/traces/histogram/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,13 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout

handleStyleDefaults(traceIn, traceOut, coerce, defaultColor, layout);

Lib.coerceSelectionMarkerOpacity(traceOut, coerce);

var lineColor = (traceOut.marker.line || {}).color;
var markerColor = (traceOut.marker || {}).color;

// override defaultColor for error bars with defaultLine
var errorBarsSupplyDefaults = Registry.getComponentMethod('errorbars', 'supplyDefaults');
errorBarsSupplyDefaults(traceIn, traceOut, Color.defaultLine, {axis: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, Color.defaultLine, {axis: 'x', inherit: 'y'});

Lib.coerceSelectionMarkerOpacity(traceOut, coerce);
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || Color.defaultLine, {axis: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || Color.defaultLine, {axis: 'x', inherit: 'y'});
};
7 changes: 5 additions & 2 deletions src/traces/scatter/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,17 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
if(!subTypes.hasLines(traceOut)) handleLineShapeDefaults(traceIn, traceOut, coerce);
}

var lineColor = (traceOut.line || {}).color;
var markerColor = (traceOut.marker || {}).color;

if(traceOut.fill === 'tonext' || traceOut.fill === 'toself') {
dfltHoverOn.push('fills');
}
coerce('hoveron', dfltHoverOn.join('+') || 'points');
if(traceOut.hoveron !== 'fills') coerce('hovertemplate');
var errorBarsSupplyDefaults = Registry.getComponentMethod('errorbars', 'supplyDefaults');
errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'x', inherit: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || defaultColor, {axis: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || defaultColor, {axis: 'x', inherit: 'y'});

Lib.coerceSelectionMarkerOpacity(traceOut, coerce);
};
6 changes: 3 additions & 3 deletions src/traces/scatter3d/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
}

var errorBarsSupplyDefaults = Registry.getComponentMethod('errorbars', 'supplyDefaults');
errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'z'});
errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'y', inherit: 'z'});
errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'x', inherit: 'z'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || defaultColor, {axis: 'z'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || defaultColor, {axis: 'y', inherit: 'z'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || defaultColor, {axis: 'x', inherit: 'z'});
};

function handleXYZDefaults(traceIn, traceOut, coerce, layout) {
Expand Down
7 changes: 5 additions & 2 deletions src/traces/scattergl/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,17 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
handleTextDefaults(traceIn, traceOut, layout, coerce);
}

var lineColor = (traceOut.line || {}).color;
var markerColor = (traceOut.marker || {}).color;

coerce('fill');
if(traceOut.fill !== 'none') {
handleFillColorDefaults(traceIn, traceOut, defaultColor, coerce);
}

var errorBarsSupplyDefaults = Registry.getComponentMethod('errorbars', 'supplyDefaults');
errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, defaultColor, {axis: 'x', inherit: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || defaultColor, {axis: 'y'});
errorBarsSupplyDefaults(traceIn, traceOut, lineColor || markerColor || defaultColor, {axis: 'x', inherit: 'y'});

Lib.coerceSelectionMarkerOpacity(traceOut, coerce);
};
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/benchmarks.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/error_bar_bar_ids.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
76 changes: 76 additions & 0 deletions test/image/mocks/bar_errorbars_inherit_color.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
{
"data": [
{
"x": [0, 1, 2, 3, 4],
"y": [0, 100, 500, 600, 0],
"type": "bar",
"barmode": "group",
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [100, 200, 600, 700, 100],
"type": "bar",
"barmode": "group",
"marker": {
"line": {
"width": 2,
"color": "gray"
}
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [200, 300, 700, 800, 200],
"type": "bar",
"barmode": "group",
"marker": {
"color": "red"
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [300, 400, 800, 900, 300],
"type": "bar",
"barmode": "group",
"marker": {
"line": {
"color": "green"
}
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [400, 500, 900, 1000, 400],
"type": "bar",
"barmode": "group",
"marker": {
"line": {
"color": "blue"
},
"color": "black"
},
"error_y": {
"type": "sqrt"
}
}
],
"layout": {
"width": 600,
"heigh": 600,
"title": {
"text": "Bar chart error bars inherit color from marker.line or marker"
}
}
}
74 changes: 74 additions & 0 deletions test/image/mocks/gl2d_scattergl_errorbars_inherit_color.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
{
"data": [
{
"x": [0, 1, 2, 3, 4],
"y": [0, 100, 500, 600, 0],
"type": "scattergl",
"mode": "markers+lines",
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [100, 200, 600, 700, 100],
"type": "scattergl",
"mode": "markers+lines",
"marker": {
"line": {
"width": 2,
"color": "gray"
}
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [200, 300, 700, 800, 200],
"type": "scattergl",
"mode": "markers+lines",
"marker": {
"color": "red"
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [300, 400, 800, 900, 300],
"type": "scattergl",
"mode": "markers+lines",
"line": {
"color": "green"
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [400, 500, 900, 1000, 400],
"type": "scattergl",
"mode": "markers+lines",
"line": {
"color": "blue"
},
"marker": {
"color": "black"
},
"error_y": {
"type": "sqrt"
}
}
],
"layout": {
"width": 600,
"heigh": 600,
"title": {
"text": "Scatter-gl error bars inherit color from line or marker"
}
}
}
79 changes: 79 additions & 0 deletions test/image/mocks/gl3d_scatter3d_errorbars_inherit_color.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
{
"data": [
{
"x": [0, 1, 2, 3, 4],
"y": [-5, -10, -30, -35, -5],
"z": [0, 0, 0, 0, 0],
"type": "scatter3d",
"mode": "markers+lines",
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [-5, -10, -30, -35, -5],
"z": [1, 1, 1, 1, 1],
"type": "scatter3d",
"mode": "markers+lines",
"marker": {
"line": {
"width": 2,
"color": "gray"
}
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [-5, -10, -30, -35, -5],
"z": [2, 2, 2, 2, 2],
"type": "scatter3d",
"mode": "markers+lines",
"marker": {
"color": "red"
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [-5, -10, -30, -35, -5],
"z": [3, 3, 3, 3, 3],
"type": "scatter3d",
"mode": "markers+lines",
"line": {
"color": "green"
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [0, 1, 2, 3, 4],
"y": [-5, -10, -30, -35, -5],
"z": [4, 4, 4, 4, 4],
"type": "scatter3d",
"mode": "markers+lines",
"line": {
"color": "blue"
},
"marker": {
"color": "black"
},
"error_y": {
"type": "sqrt"
}
}
],
"layout": {
"width": 600,
"heigh": 600,
"title": {
"text": "Scatter3d error bars inherit color from line or marker"
}
}
}
71 changes: 71 additions & 0 deletions test/image/mocks/histogram_errorbars_inherit_color.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
{
"data": [
{
"x": [0, 1, 5, 6, 0],
"type": "histogram",
"barmode": "stack",
"error_y": {
"type": "sqrt"
}
},
{
"x": [1, 2, 6, 7, 1],
"type": "histogram",
"barmode": "stack",
"marker": {
"line": {
"width": 2,
"color": "gray"
}
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [2, 3, 7, 8, 2],
"type": "histogram",
"barmode": "stack",
"marker": {
"color": "red"
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [3, 4, 8, 9, 3],
"type": "histogram",
"barmode": "stack",
"marker": {
"line": {
"color": "green"
}
},
"error_y": {
"type": "sqrt"
}
},
{
"x": [4, 5, 9, 10, 4],
"type": "histogram",
"barmode": "stack",
"marker": {
"line": {
"color": "blue"
},
"color": "black"
},
"error_y": {
"type": "sqrt"
}
}
],
"layout": {
"width": 600,
"heigh": 600,
"title": {
"text": "Histogram error bars inherit color from marker.line or marker"
}
}
}
Loading