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

[Geolocation] Provide default implementation for geolocation callback #177

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

markmur
Copy link
Contributor

@markmur markmur commented Dec 17, 2024

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.

override fun onGeolocationPermissionsShowPrompt(origin: String, callback: GeolocationPermissions.Callback) {
    (context as MainActivity).requestGeolocationPermission()
     super.onGeolocationPermissionsShowPrompt(origin, callback)
}

Before you merge

Important


Checklist for releasing a new version

Tip

See the Contributing documentation for instructions on how to publish a new version of the library.

@markmur markmur self-assigned this Dec 17, 2024
@markmur markmur requested a review from a team as a code owner December 17, 2024 11:02
Copy link
Contributor

@kiftio kiftio left a 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

@@ -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()
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@markmur markmur force-pushed the markmur/smart-geo-default branch 2 times, most recently from a1f0372 to 16cc91a Compare December 17, 2024 13:58
@markmur markmur force-pushed the markmur/smart-geo-default branch from 16cc91a to 94bdbda Compare December 17, 2024 14:00
@markmur
Copy link
Contributor Author

markmur commented Dec 17, 2024

Here's the plan for the React-Native side: Shopify/checkout-sheet-kit-react-native#148

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.

2 participants