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 https://github.com/react-community/react-native-maps/issues/2203 #2207

Merged
merged 3 commits into from
Apr 24, 2018

Conversation

sytolk
Copy link

@sytolk sytolk commented Apr 18, 2018

Does any other open PR do the same thing?

no

What issue is this PR fixing?

#2203

How did you test this PR?

every location is have timestamp and it's now received on RN side.

@rborn
Copy link
Collaborator

rborn commented Apr 18, 2018

Please add the documentation too with platform specific 🤗
Can this be extended to iOS too? AppleMap and GoogleMap ?

Thank you

@sytolk
Copy link
Author

sytolk commented Apr 18, 2018

@rborn I think that Documentation for onUserLocationChange exist:
https://github.com/react-community/react-native-maps/blob/master/docs/mapview.md#events

Callback that is called when the underlying map figures our users current location (coordinate also includes isFromMockProvider value for Android API 18 and above). Make sure showsUserLocation is set to true and that the provider is "google"

this is only minor change that add timestamp parameter to location object received on RN side and I don't think that that there is need documentation changes

@rborn
Copy link
Collaborator

rborn commented Apr 18, 2018

@sytolk what about iOS ?

@sytolk
Copy link
Author

sytolk commented Apr 18, 2018

sorry but I cannot test onUserLocationChange on iOS.
I suppose that this is the line that is need timestamp to be set:
https://github.com/react-community/react-native-maps/blob/master/lib/ios/AirGoogleMaps/AIRGoogleMap.m#L451

https://developer.apple.com/documentation/corelocation/cllocation/1423589-timestamp
and maybe this row is missing for iOS ?
@"timestamp": @(location.timestamp),

but I'm Android developer and cannot testing it on iOS. Can I add this row in PR or you prefer to open new issue for iOS?

@rborn
Copy link
Collaborator

rborn commented Apr 24, 2018

LGTM @alvelig 🐽

@alvelig alvelig merged commit 71c74cd into react-native-maps:master Apr 24, 2018
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