-
Notifications
You must be signed in to change notification settings - Fork 5
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
[Geolocation] Provide default implementation for geolocation callback #177
base: main
Are you sure you want to change the base?
Conversation
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.
I haven't had chance to tophat yet
lib/src/test/java/com/shopify/checkoutsheetkit/DefaultCheckoutEventProcessorTest.kt
Outdated
Show resolved
Hide resolved
lib/src/test/java/com/shopify/checkoutsheetkit/DefaultCheckoutEventProcessorTest.kt
Outdated
Show resolved
Hide resolved
lib/src/test/java/com/shopify/checkoutsheetkit/DefaultCheckoutEventProcessorTest.kt
Outdated
Show resolved
Hide resolved
lib/src/test/java/com/shopify/checkoutsheetkit/DefaultCheckoutEventProcessorTest.kt
Outdated
Show resolved
Hide resolved
lib/src/test/java/com/shopify/checkoutsheetkit/DefaultCheckoutEventProcessorTest.kt
Outdated
Show resolved
Hide resolved
@@ -85,7 +85,8 @@ class MobileBuyEventProcessor( | |||
} | |||
|
|||
override fun onGeolocationPermissionsShowPrompt(origin: String, callback: GeolocationPermissions.Callback) { | |||
return (context as MainActivity).onGeolocationPermissionsShowPrompt(origin, callback) | |||
(context as MainActivity).requestGeolocationPermission() |
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.
Shall we remove this whole override and just use the default impl?
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.
This is required since the MainActivity is responsible for actually requesting the permissions. The default implementation checks if permission has been granted already but can't request permissions on behalf of the app.
If we were to request access on app start, then we wouldn't need to override this method at all. Some apps may do this when buyers log in for example or download and use the app for the first time.
a1f0372
to
16cc91a
Compare
16cc91a
to
94bdbda
Compare
Here's the plan for the React-Native side: Shopify/checkout-sheet-kit-react-native#148 |
What changes are you making?
Provides a default implementation for the
onGeolocationPermissionsShowPrompt
lifecycle method, which checks for both manifest and runtime permissions.This will simplify the implementation in that consumers will only need to care about requesting runtime permissions and calling
super.onGeolocationPermissionsShowPrompt
to let the library handle the rest.Before you merge
Important
Checklist for releasing a new version
build.gradle
fileTip
See the Contributing documentation for instructions on how to publish a new version of the library.