Skip to content

Seperate drawMap from zoomToPosition and add configuration for clusters#9

Merged
sandraor merged 3 commits intosandraor:masterfrom
alexclarkofficial:master
Jan 14, 2015
Merged

Seperate drawMap from zoomToPosition and add configuration for clusters#9
sandraor merged 3 commits intosandraor:masterfrom
alexclarkofficial:master

Conversation

@alexclarkofficial
Copy link

In reference to: emberjs#1904

@wifelette
Copy link

Thumbs up on this from @wycats (in theory, we just looked at the code, haven't pulled it and tested 😛) / cc @sandraor

@alexclarkofficial alexclarkofficial changed the title Seperate drawMap from zoomToPosition Seperate drawMap from zoomToPosition and add configuration for clusters Jan 12, 2015
@alexclarkofficial
Copy link
Author

Do we have the PNG's we would like to use for the clusters? This is the most basic implementation, but I'd be happy to do the CSS to make it look right.

@alexclarkofficial
Copy link
Author

Oh, also I just figured out how to set the break points for cluster size. If we'd like to change cluster images at different sizes let me know (I think it's currently set to 2 and 10).

@wifelette
Copy link

RE: image for clusters: best answered by @sandraor

@sandraor
Copy link
Owner

Thanks, @alexclarkofficial! Here are the two cluster images. The first one is designed for 1 digit, the second one for 2 digits, both with white text. The smallest breakpoint (2) seems too aggressive, so can we try it at 4?

map-cluster-1_x2
map-cluster-1
map-cluster-2_x2
map-cluster-2

@alexclarkofficial
Copy link
Author

Thanks @sandraor! I went with the smaller image for each cluster size that you provided. I tried smallest breakpoints of both 3 and 4. You can check out the results below. Let me know what you think and any other remarks about styling and I'll make the commit.

Globe with smallest breakpoint of 3:
http://cl.ly/image/1o3T061Y0z0w

Globe with smallest breakpoint of 4:
http://cl.ly/image/3s2e2I0n2s1Y

Europe with smallest breakpoint of 3:
http://cl.ly/image/2X2a0a161B1p

Europe with smallest breakpoint of 4:
http://cl.ly/image/2L242p0h2m0L

@wifelette
Copy link

I'm way too excited to get this merged in 😀

Copy link

Choose a reason for hiding this comment

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

Something seems weird about the indentation style here.

Copy link
Author

Choose a reason for hiding this comment

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

That is weird - it will be fixed in my next commit.

@alexclarkofficial alexclarkofficial mentioned this pull request Jan 13, 2015
@wifelette
Copy link

Making sure we're all on the same page here: @alexclarkofficial is fixing a couple indentation changes, and then this is good to be merged. He'll comment here (and/or here too: emberjs#1904) to let @sandraor know when #9 is ready to be merged in.

@alexclarkofficial
Copy link
Author

Thanks @wifelette! I was hoping to get @sandraor's design approval and final comment on the smallest breakpoint for clusters - but other than that I can make the commit anytime.

@sandraor
Copy link
Owner

I think the breakpoint at 4 looks best. Thanks for testing them out!

Also, I just noticed that github didn't keep or display the names of the cluster images I uploaded, so I'm sorry, what I said about them didn't make much sense! :( I think only the larger image needs to be changed:

This is for the small breakpoint (map-cluster-1.png): https://cloud.githubusercontent.com/assets/2864198/5715531/599d9ba8-9aa8-11e4-9537-45c10012ac8c.png
This is the retina version (map-cluster-1_x2.png):
https://cloud.githubusercontent.com/assets/2864198/5715530/598a88a6-9aa8-11e4-902a-476fcf2e8bf9.png

This is for the large breakpoint (map-cluster-2.png): https://cloud.githubusercontent.com/assets/2864198/5715533/59a443c2-9aa8-11e4-92fc-edd75c434146.png
This is the retina version (map-cluster-2_x2.png): https://cloud.githubusercontent.com/assets/2864198/5715532/59a2efa4-9aa8-11e4-8091-d15466a6d62f.png

Then I think it's ready to go! :D

@alexclarkofficial
Copy link
Author

Changes have been made, but unfortunately I couldn't figure out how to include the URLs for the retina images. The doc's showed a boolean for enabling retina images, but no details on how to include them.
http://google-maps-utility-library-v3.googlecode.com/svn/trunk/markerclustererplus/docs/reference.html#ClusterIconInfo

sandraor added a commit that referenced this pull request Jan 14, 2015
Seperate drawMap from zoomToPosition and add configuration for clusters
@sandraor sandraor merged commit 616e84c into sandraor:master Jan 14, 2015
@sandraor
Copy link
Owner

No worries — I just included them in case they were needed.

This looks great! Thank you! :)

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.

4 participants