Skip to content

expose map type callback #275

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
merged 1 commit into from
Nov 24, 2016

Conversation

mattydoincode
Copy link
Contributor

@mattydoincode mattydoincode commented Nov 7, 2016

This PR would expose the google maps event that says "hey, the user changed from satellite view to roads view" or vice versa. This is something I need to detect and change the icon styles based on so am looking to expose it. Any input is greatly appreciated!

@mattydoincode
Copy link
Contributor Author

mattydoincode commented Nov 7, 2016

Doesn't seem like CI is related to my PR...

^^ I stand corrected

@istarkov
Copy link
Collaborator

istarkov commented Nov 7, 2016

Really?
653:63 error Trailing spaces not allowed no-trailing-spaces

@mattydoincode
Copy link
Contributor Author

LOL when i looked at it that message hadn't loaded yet and I got super confused. Will update!!

@istarkov
Copy link
Collaborator

istarkov commented Nov 7, 2016

Having onBlabblaChange calback in interface I always try to find blabbla prop on Component public interface.
Here we have onMapTypeIdChange but mapTypeId is not present in props, for me it's confusing a little.
Not sure what to do, or to add mapTypeId as a controllable property, or to somehow change the event name. IMO first is overhead, and I can't suggest a good name for the second.
Your opinion?

@mattydoincode
Copy link
Contributor Author

That's a great point, I would say considering it's something that a user might want to set.. for example "oh, i want to show a satellite map here" we probably want it to be controlled. This seems reasonable to me.
https://developers.google.com/maps/documentation/javascript/reference#MapTypeId. The real truth though, is isn't it wrapped up in the options object? Like a lot of the things you can do with this library are hidden behind that sort of big options object that you can fill with styles, or whatever you want, so in a sense it's kinda handled. Thoughts?

@istarkov
Copy link
Collaborator

istarkov commented Nov 7, 2016

Please add 1-2 lines about in documentation, and let's merge then.

Let's think about after merge, as now I have no ideas ;-)

@istarkov istarkov merged commit 7be0f10 into google-map-react:master Nov 24, 2016
@istarkov
Copy link
Collaborator

google-map-react@0.21.4

@lock
Copy link

lock bot commented Dec 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants