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

#15 Support showing ClusteredPoint at centroid of all contained points instead of at initial point. #16

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ruckc
Copy link

@ruckc ruckc commented Jul 13, 2013

No description provided.

@ruckc
Copy link
Author

ruckc commented Jul 13, 2013

The only issue with this implementation is when splitting a clusteredPoint (size 3) into two clusteredPoints, the 'size 2' point jumps (non-animated). This seems to happen when any clusteredPoint splits off a clusteredPoint (size 1).

@carltonwhitehead
Copy link
Contributor

I considered taking this approach early in development, but I decided not to implement it because it results in drawing markers where no points actually exist. I can understand that is mostly a matter a preference though, so I would be willing to accept this pull request if you add an option to enable/disable this behavior in the Options class.

EDIT: and please comment the setting as a beta feature until the animation issue can be resolved.

@carltonwhitehead
Copy link
Contributor

@ruckc sorry I didn't get back to you earlier about this. I like where you are going with this, but I'm concerned about the static ClusterPoint reference to Options. Also, (this is my fault), its constructors are getting unwieldy, so I don't want to ask you to refactor that. At some point, I'm going to refactor the ClusterPoint class, most likely using a Builder pattern, and I will merge your pull into that. I'll keep all of the public APIs the same though, so it shouldn't cause any conflicts with your branch when we eventually merge. Thanks for your contribution!

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

Successfully merging this pull request may close these issues.

2 participants