Skip to content
This repository was archived by the owner on Nov 8, 2023. It is now read-only.

Conversation

@denisraison
Copy link
Contributor

Hey guys,

As I commented here #146 I'm adding the usage of the updateDistance option for Android.
Now we should be able to receive locations update only if the user moved more them X meters on Android too.
🥇

@ghost ghost added the new PR label Jul 21, 2018
Copy link
Contributor

@etabakov etabakov left a comment

Choose a reason for hiding this comment

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

Thanks for your effort @denisraison! I noticed two minor issues before proceeding with this PR:

  • We've adopted the convention to use code blocks even in case of one-line conditional statements
  • In the location-monitor.d.ts file the updateDistance property of the Options interface is stated that the property is ignored for Android, which will no longer be the case after this PR.

0 : options.minimumUpdateTime || Math.min(updateTime, fastestTimeUpdate);
mLocationRequest.setFastestInterval(minUpdateTime);
if (options.updateDistance)
mLocationRequest.setSmallestDisplacement(options.updateDistance);
Copy link
Contributor

Choose a reason for hiding this comment

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

Our convention is to use code blocks even in case of one-line If statements.

Changed the description for the updateDistance property
@denisraison
Copy link
Contributor Author

Hey @etabakov,
I've just changed those minor issues and now, I think, should be alright.

Thank you! 😄

@etabakov
Copy link
Contributor

Everything looks great now. Congrats on your first contribution to @NativeScript's codebase! You just earned the "Contribution" badge.

contrib

@etabakov etabakov merged commit d3a35ab into NativeScript:master Jul 25, 2018
@ghost ghost removed the new PR label Jul 25, 2018
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