-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat(ios): expose new APIs to use location AccuracyAuthorization #11813
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
* Licensed under the terms of the Apache Public License | ||
* Please see the LICENSE included with this distribution for details. | ||
*/ | ||
/* globals OS_VERSION_MAJOR */ | ||
/* eslint-env mocha */ | ||
/* eslint no-unused-expressions: "off" */ | ||
'use strict'; | ||
|
@@ -13,6 +14,7 @@ var should = require('./utilities/assertions'); | |
// Skip on Windows 10 Mobile device family due to prompt, | ||
// however we might be able to run some tests? | ||
describe.windowsBroken('Titanium.Geolocation', function () { | ||
|
||
it('.apiName', function () { | ||
should(Ti.Geolocation).have.readOnlyProperty('apiName').which.is.a.String(); | ||
should(Ti.Geolocation.apiName).be.eql('Ti.Geolocation'); | ||
|
@@ -244,7 +246,52 @@ describe.windowsBroken('Titanium.Geolocation', function () { | |
should(Ti.Geolocation.trackSignificantLocationChange).be.true(); | ||
}); | ||
|
||
it.ios('.ACCURACY_REDUCED', function () { | ||
if (OS_VERSION_MAJOR >= 14) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we define these special filters like |
||
should(Ti.Geolocation).have.constant('ACCURACY_REDUCED').which.is.a.Number(); | ||
} | ||
}); | ||
|
||
it.ios('.ACCURACY_AUTHORIZATION_FULL', function () { | ||
if (OS_VERSION_MAJOR >= 14) { | ||
should(Ti.Geolocation).have.constant('ACCURACY_AUTHORIZATION_FULL').which.is.a.Number(); | ||
} | ||
}); | ||
|
||
it.ios('.ACCURACY_AUTHORIZATION_REDUCED', function () { | ||
if (OS_VERSION_MAJOR >= 14) { | ||
should(Ti.Geolocation).have.constant('ACCURACY_AUTHORIZATION_REDUCED').which.is.a.Number(); | ||
} | ||
}); | ||
|
||
it.ios('.locationAccuracyAuthorization', function () { | ||
vijaysingh-axway marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (OS_VERSION_MAJOR >= 14) { | ||
should(Ti.Geolocation).have.a.property('locationAccuracyAuthorization').which.is.a.Number(); | ||
} | ||
}); | ||
|
||
// Methods | ||
it.ios('#requestTemporaryFullAccuracyAuthorization()', function (finish) { | ||
this.timeout(6e4); // 60 sec | ||
if (OS_VERSION_MAJOR < 14) { | ||
return finish(); | ||
} | ||
|
||
should(Ti.Geolocation.requestTemporaryFullAccuracyAuthorization).be.a.Function(); | ||
Ti.Geolocation.requestTemporaryFullAccuracyAuthorization('purposekey', function (e) { | ||
try { | ||
// It will always give error because 'purposekey' is not in tiapp.xml. | ||
should(e).have.property('success').which.is.a.Boolean(); | ||
should(e.success).be.false(); | ||
should(e).have.property('code').which.is.a.Number(); | ||
should(e.code).be.eql(1); | ||
should(e).have.property('error').which.is.a.String(); | ||
finish(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typically I'd move this outside the try/catch block, because if an |
||
} catch (err) { | ||
return finish(err); | ||
} | ||
}); | ||
}); | ||
|
||
it('#forwardGeocoder()', function (finish) { | ||
this.timeout(6e4); // 60 sec | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have to create a new method? From a cross-platform standpoint, it might be nice to re-use our existing requestLocationPermissions() and hasLocationPermissions() methods and add the new "purpose" string key as a parameter... or even better add support for a settings dictionary to these methods with a new "purpose" field.
On Android, location permission can be granted temporarily by the end-user too. We can still leverage the
hasLocationPermission()
method to check if it's still granted. And theTi.Location.accuracy
property defines the accuracy of the permission we need to look for. On Android, theACCESS_FINE_LOCATION
sounds like the equivalent of the "accurate" permission on iOS (which means read from the GPS).https://developer.android.com/preview/privacy/permissions#one-time
So, can we do something like this?
Question:
On iOS 14, is the "purpose" key required? Are people going to have location accessing problem if they don't update their location permission handling code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jquick-axway This new API is for requesting full accuracy if user requires it in app. And it get expires. It is not mandatory. e.g your app do not need full accurate location, you should simply need location access . let's say at some screen you need full accuracy to show use'r location, you have to go for it. See here for more details
I think changes in Android 11 is same as of 'allow once' permission given in iOS 13. It is location permission's life not accuracy. See here.
'purpose' key is only required if one is going to access location with full accuracy. It will not break existing implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So my only beef with this is if the app developer sets
Ti.Location.accuracy
toACCURACY_HIGH
, then I thinkhasLocationPermissions()
should returnfalse
if it has high accuracy has expired, because that's what the app desires. That's why I'm question the necessity of these new APIs... that and I think we're making this more complicated for app developers than it needs to be. A portable API that works between all iOS versions and Android would be the ideal.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If user has granted location permission
hasLocationPermissions()
will betrue
. ACURRACY_HIGH (kCLLocationAccuracyBest) is desirable not guaranteed.In iOS 14+ when app request for location permission there comes a popup (see attached Image1) which have a button mentioning 'Precise: On'. If user sets it off, app will not get location data with full accuracy. If at some point it is required to get 'full accuracy', with this new API app can ask for permission to get full accuracy in location data with a valid reason (see image2 ). And it will be temporary.
Image1-
image2 -