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

Bug fix for Scatter3D lines+markers with line color array error #3316

Closed
wants to merge 3 commits into from

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Dec 10, 2018

@@ -33,7 +33,12 @@ module.exports = function calc(gd, trace, opts) {
updateStyle(trace._input, containerStr ? (containerStr + '.' + attr) : attr, inputVal);
}
else {
inputContainer[attr] = inputVal;
if(trace._input.type === 'scatter3d') {
trace._input[attr] = inputVal;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. Looks to me like containerStr about isn't getting set properly? What's its value here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The inputContainer was undefined if I recall correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right , but what about containerStr?

From this block above

if(containerStr) {
container = Lib.nestedProperty(container, containerStr).get();
inputContainer = Lib.nestedProperty(inputContainer, containerStr).get();
fullInputContainer = Lib.nestedProperty(fullInputContainer, containerStr).get() || {};
}

this leads be to believe that containerStr isn't getting set properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First it is equal to line; but then when it is set to marker we get an error.

Copy link
Contributor

@etpinard etpinard Dec 11, 2018

Choose a reason for hiding this comment

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

but then when it is set to marker we get an error.

Right, and in this case Colorscale.calc should not get called in the first place. Something must be not working correctly here:

if(subTypes.hasMarkers(trace)) {
if(hasColorscale(trace, 'marker')) {
calcColorscale(gd, trace, {
vals: trace.marker.color,
containerStr: 'marker',
cLetter: 'c'
});
}

@etpinard
Copy link
Contributor

@archmoj PR #3341 fixes this bug. More specifically commit c9d92d7 removes all the inputContainer logic in Colorscale.calc and commit e8973fa adds a new mock 🔒ing down the fix.

Closing!

@etpinard etpinard closed this Dec 17, 2018
@etpinard etpinard deleted the issue-2873 branch December 17, 2018 19:11
@archmoj
Copy link
Contributor Author

archmoj commented Dec 17, 2018

Excellent!

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.

None yet

2 participants