[Geolocation] Add permissionTypeIOS option

[rt2zz]

This adds backgroundMode to the geolocation options. When options.backgroundMode === true for both gelocation.getCurrentPosition and gelocation.watchPosition:

1. Enforce NSLocationAlwaysUsageDescription is set in Info.plist
2. Requests requestAlwaysAuthorization as opposed to requestWhenInUseAuthorization

[ide]

This should probably be an enum instead of a boolean or at least renamed to be clearer.

[rt2zz]

@ide something like permissionType: ENUM('IN_USE', 'ALWAYS') ?

Does using ios'y terms make sense or are we trying to keep things platform agnostic?

[ide]

Android's permissioning is pretty different (coarse vs. fine updates rather than background vs. foreground) so I think we should optimize for iOS in this commit and clearly name the property something like permissionTypeIOS.

cc @frantic are you the right person to take a look at geolocation diffs (saw you were active on a few others)?

[frantic]

IIRC @sahrens built the original implementation. Re: this PR, I think we
should be explicit about the platform, especially when Android has a
different model
On Wed, Jun 3, 2015 at 5:19 PM James Ide notifications@github.com wrote:

Android's permissioning is pretty different (coarse vs. fine updates
rather than background vs. foreground) so I think we should optimize for
iOS in this commit and clearly name the property something like
permissionTypeIOS.

cc @frantic https://github.com/frantic are you the right person to take
a look at geolocation diffs (saw you were active on a few others)?

—
Reply to this email directly or view it on GitHub
#1506 (comment)
.

[rt2zz]

Ok, replaced backgroundMode with permissionTypeIOS which (if defined) must be one of ['ALWAYS', 'IN_USE']

I am not sure about the code style here, e.g. is it appropriate to use indexOf in an invariant argument as a surrogate for enum. Should we enum the permissionTypeIOS option again in objC land? Following react style is less obvious when working outside of components :(

Would it make sense to expose an explicity requestPermission method even though that does not conform for w3 geolocation spec?

[sahrens]

rm

[sahrens]

This seems ok to - @nicklockwood, you re-wrote this recently, any qualms?

[rt2zz]

Ah, thanks @sahrens

I changed the constants to be in line with react-native conventions, now: RCTPermissionTypeAlways, RCTPermissionTypeInUse, RCTPermissionKey. I also changed the permission type strings from all caps to camel case as that seems to be the convention, now: "always", "inUse"

[timdoes]

This is working great in the background. The only time it stops running is when the display is off. If I press the home button, it starts running again (even while locked) but as soon as the display goes black again, it stops running.

My code is the same as the React Native - Geolocation Example (with the added permissionTypeIOS: 'always').

Any ideas why this is happening? I will provide more details if needed.

Thanks! Love this!

[rt2zz]

@timdoes it may be an issue with your app setup. For example make sure the location background mode is enabled: https://developer.apple.com/library/ios/documentation/General/Reference/InfoPlistKeyReference/Articles/iPhoneOSKeys.html#//apple_ref/doc/plist/info/UIBackgroundModes

This diff only changes the permission type requested, the mechanism for retrieving the location is still the standard startUpdatingLocation method https://developer.apple.com/library/ios/documentation/CoreLocation/Reference/CLLocationManager_Class/index.html#//apple_ref/occ/instm/CLLocationManager/startUpdatingLocation

[timdoes]

My background mode declaration:

<key>UIBackgroundModes</key>
<array>
    <string>fetch</string>
    <string>location</string>
    <string>remote-notification</string>
</array>

Does this need to be used with a custom library class and bridged to React Native?

Sorry if this is a rookie question. Just thought this would add full background capabilities by just declaring the background mode and using the https://facebook.github.io/react-native/docs/geolocation.html example.

[rt2zz]

@timdoes just tested this out, and I see the same behaivor (no updates with display off). I do not have a definitive answer for you, apple does not make debugging this types of situations easy. You may consider playing around with allowDeferredLocationUpdatesUntilTraveled:timeout or signifigant location changes api or geofencing (none of which are currently supported by react-native gelocation)

[sahrens]

Actually going to pass this one to nick to finish review and import.

[machard]

any news on what's causing the events not to fire when screen is locked ?
I see the same behavior with https://github.com/machard/react-native-location.

It must be in the way the bridge eventDispatcher works because didUpdateLocations is called.

[satya164]

@nicklockwood @rt2zz Any updates on this?

[facebook-github-bot]

@rt2zz updated the pull request.

[facebook-github-bot]

@rt2zz updated the pull request.

[facebook-github-bot]

@rt2zz updated the pull request.

[rt2zz]

@satya164 not sure about the status, but fwiw I just rebased against master, so if you want to take this patch for a test drive feel free.

[satya164]

@rt2zz Everyone is on holiday. This PR might have to wait till next year :D

[facebook-github-bot]

@rt2zz updated the pull request.

[brentvatne]

ping @nicklockwood :)

[nicklockwood]

I'm not wholly happy with the implementation, but in the interests of moving forward, I'll accept this and then make some tweaks in a follow-up diff.

[nicklockwood]

@facebook-github-bot shipit

[facebook-github-bot]

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1453225971642005/int_phab to review.

[rt2zz]

Thanks @nicklockwood - let me know if there is anything needed from me to follow up / tweak.

[nicklockwood]

I'm closing this in favour of #5093, which solves the problem without exposing any proprietary props in Geolocation.js