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

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Jan 7, 2019

fix #3392 for 5 traces.

Note 1: For bar and histogram cases marker.line.color is applied as the default color. If undefined then `marker.color' is used if defined.

Note 2: For scatter, scatter3d and scattergl cases line.color is applied as the default color. If undefined then marker.color' is used if defined. Also note that marker.line.coloris not used in this regard; and the trace line color and errorbar lines would usecolorway` in that scenario.

@plotly/plotly_js

@etpinard
Copy link
Contributor

etpinard commented Jan 7, 2019

Hmm. I don't think we should be considering this a bug fix. This changes a few of our baselines.

Personally, I think this PR belongs in the v2 push, but I'd be ok with calling this a feature set for the next minor v1.44.0.

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...

@alexcjohnson
Copy link
Collaborator

True, it contains a bit of a change, but mostly I’d call this fixing a bug. To me it’s not such a change that it should be considered breaking but I can understand not calling it just a patch.

@etpinard
Copy link
Contributor

etpinard commented Jan 8, 2019

To me it’s not such a change that it should be considered breaking but I can understand not calling it just a patch.

Sounds good, let's panned this one for 1.44.0.

@etpinard etpinard added this to the 1.44.0 milestone Jan 8, 2019
@etpinard
Copy link
Contributor

💃 nicely done @archmoj !

@archmoj archmoj merged commit 51b5c25 into master Jan 14, 2019
@archmoj archmoj deleted the fix3392-errorbars-inherit-color2 branch January 14, 2019 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error bars not inheriting color
3 participants