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

Fix for pointForCoordinate and coordinateForPoint #2039

Merged
merged 2 commits into from
Mar 8, 2018

Conversation

wasedaigo
Copy link
Contributor

  • Fixed typo "coordinateFromPoint"
  • Moved coordinateForPoint/pointForCoordinate from AirMap to AirMapManager
  • Fixed how resolve/reject are handled

I have tested on iOS. Yet not confirmed on Android. If anybody can confirm this works on Android, I think this PR is good to go.

@rborn
Copy link
Collaborator

rborn commented Feb 22, 2018

LGTM @alvelig 🐽
PS - not tested on android yet

@rborn
Copy link
Collaborator

rborn commented Feb 22, 2018

@wasedaigo does it work on iOs for both Apple maps and Google maps ?
also how's compared to this: #1737

🤗

@wasedaigo
Copy link
Contributor Author

wasedaigo commented Feb 22, 2018

@rborn I haven't tested GoogleMap, but it seems it won't work. Sounds like we can integrate part of #1737 to make it work. What would be the best approach here? I can add the implementation, but I have no time to test. It is hard to test all combinations. As these methods do not work on GoogleMap/Android anyways, as long as it compiles we can merge this and move forward?

@rborn
Copy link
Collaborator

rborn commented Feb 22, 2018

@wasedaigo @alvelig I will try to test on android tomorrow and also if we can merge first #1737 and then this one.
If problems arise we might wanna integrate that PR in yours. OR if you know for sure that will be problems you could try to integrate it and update the PR. And I'll test.

Works for you?

@wasedaigo
Copy link
Contributor Author

wasedaigo commented Feb 22, 2018

actually #1737 should be closed, the implementation for GoogleMap is already there.
It is just this commit 9cb22b9 made a mistake on iOS AppleMap part

I think once you confirmed this works on Android, we are good.

@wasedaigo
Copy link
Contributor Author

@rborn is everything alright? I want to make sure I am not blocking you

@rborn
Copy link
Collaborator

rborn commented Feb 26, 2018

@wasedaigo sorry, didn't have the time to look into and the next days I'll be very busy. I'll try to take a look in the next evenings but I don't promise 😔

@rborn
Copy link
Collaborator

rborn commented Mar 8, 2018

@wasedaigo it works great on android ❤️

Sorry for the delay.

@rborn rborn merged commit 5f7bb26 into react-native-maps:master Mar 8, 2018
@henninghall
Copy link

I found some issues related to this pull request.

  1. coordinateForPoint with Google maps does not work on iOS
  2. coordinateForPoint resolves coordinate on format {lat, lng} on iOS with apple maps and {latitude,longitude} with google maps on android.

@rborn
Copy link
Collaborator

rborn commented Mar 13, 2018

@wasedaigo can you have a look at this ?

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.

3 participants