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

Scattermapbox line trailing gaps #1421

Merged
merged 4 commits into from
Feb 27, 2017
Merged

Conversation

etpinard
Copy link
Contributor

fixes #1418

easy. cc @rreusser

@etpinard etpinard added status: reviewable bug something broken labels Feb 24, 2017
@etpinard etpinard added this to the 1.24.0 milestone Feb 24, 2017
var lineCoords = [
[[10, 20], [20, 20], [30, 10]],
[[20, 10], [10, 20]]
];
Copy link
Contributor Author

@etpinard etpinard Feb 24, 2017

Choose a reason for hiding this comment

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

Previously, this was:

var lineCoords = [
  [[10, 20], [20, 20], [30, 10]],
  [[20, 10], [10, 20]].
  []
];

which lead to an invalid GeoJSON and made mapbox-gl fail.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. I thought this would be a pretty easy fix. Just to confirm, I assume it will also handle [10, '20', 30, 20, null, 20, 10, null, null] without problems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume it will also handle [10, '20', 30, 20, null, 20, 10, null, null] without problems?

Yes it does: 1a22beb

@@ -365,6 +365,23 @@ describe('scattermapbox convert', function() {
expect(actualText).toEqual(['A', 'B', 'C', 'F', '']);
});

it('for lines traces with trailing gaps', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment as #1414 (comment) - is this an intentional pattern now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

24d7c47 - better?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, much better to my eye - thanks!

@alexcjohnson
Copy link
Collaborator

💃

@etpinard etpinard merged commit f685823 into master Feb 27, 2017
@etpinard etpinard deleted the scattermapbox-line-trailing-gaps branch February 27, 2017 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

scattermapbox lines break with trailing null
3 participants