Skip to content

Add clustering to scattermapbox #5334

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

Merged

Conversation

elben10
Copy link
Contributor

@elben10 elben10 commented Dec 10, 2020

Supersedes #5147.

I've added a trace that allow one to use mapbox clustering layer.

Currently it doesn't support the fill, selection and text features.

TODO:

  • add jasmine test for hover
  • add jasmine tests to ensure Plotly.react can switch between modes i.e. with/without cluster
  • add second mock & baseline using new attributes e.g. step and opacity

@archmoj archmoj added status: in progress community community contribution feature something new labels Dec 11, 2020
@archmoj
Copy link
Contributor

archmoj commented Dec 11, 2020

Thanks @elben10 for opening this PR.
I updated the description and added various test to the TODO list mainly to ensure the correct functionality would be maintained in future.

@archmoj archmoj added this to the 1.59.0 milestone Jan 5, 2021
@archmoj
Copy link
Contributor

archmoj commented Jan 7, 2021

@elben10 Before adding tests please sync the master branch of your fork with plotly/plotly.js.
Then run git merge upstream/master into your branch.
Thank you!

@archmoj archmoj removed this from the Next major milestone Jan 7, 2021
@nicolaskruchten
Copy link
Contributor

A small update on timing: our team is working hard on releasing v2.0 of Plotly.js, which we anticipate will happen in early April. This PR would be a good candidate to land in the library in v2.1 or later, so with apologies for the delay, we will likely not be able to give much feedback on this PR for the next few weeks :)

@archmoj
Copy link
Contributor

archmoj commented Jun 26, 2021

There is a conflict in test/jasmine/tests/mock_test.js
@elben10 Wondering if you could possibly pull upstream/master and resolve the conflict by accepting incoming changes please?

@elben10 elben10 closed this Jul 11, 2021
@archmoj
Copy link
Contributor

archmoj commented Jul 12, 2021

@elben10 Are you planning to reopen this interesting PR after resolving that little conflict?
If needed I'd be happy to help : )

@elben10
Copy link
Contributor Author

elben10 commented Jul 14, 2021

Sure

@elben10 elben10 reopened this Jul 14, 2021
@archmoj archmoj changed the base branch from master to finalist-cluster-scattermapbox July 14, 2021 19:33
@archmoj
Copy link
Contributor

archmoj commented Jul 14, 2021

💃
Merging into finalist-cluster-scattermapbox branch.
Thanks @elben10 for the hard work 🏆 🥇
I'll reopen a PR to target master after resolving the conflicts and adjusting the tests.

@archmoj archmoj merged commit 12fd6ce into plotly:finalist-cluster-scattermapbox Jul 14, 2021
@elben10
Copy link
Contributor Author

elben10 commented Jul 15, 2021

Sounds great. Thanks for the guidance and support! 🥇 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants