-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Requesting location permission popup opens app's settings instead of location settings #5255
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
Comments
A PR comment related to this issue. |
Is this issue still open? I'd like to be assigned to work on it. |
@paco-arana You are very welcome to submit a pull request. :-) |
@rohit9625 That may be due to some recent changes related to migrating to OSM maps. Would you be able to analyse the rrot cause for the same? |
@sivaraam @rohit9625 for me the toast displays a different message. But when I click the target icon again (after giving permission) it correctly prompts me to turn on the location and take me to the location settings only. |
@ShashwatKedia you are right. |
@rohit9625 I think it's not resolved because, ideally, on giving permission for location, the app should automatically ask to turn on location (if not on) and show the map accordingly. This is not being implemented, as the user has to click the target button again for the app to ask to turn on the location. |
@ShashwatKedia you are right. It should work like as you told. |
@rohit9625 I found that inside the locationPermissionGranted() function, if the lastKnownLocation is null, then we only display a toast to user, instead of showing dialog to turn on the location. This can be implemented by calling showLocationOffDialog() when location is null. |
No one else is working on this issue. You could feel free to work on it 🙂 |
Hey guys, WhatsApp.Video.2024-01-14.at.21.42.11_4aa5ed18.mp4Also, on the logcat an error returned as a response from Wikimedia API. Saying this Does anyone know about the code from where this request was sent? |
@rohit9625 Please use the app's prodDebug flavor. See https://github.com/commons-app/commons-app-documentation/blob/master/android/build-variants/Build-Variants.md |
@rohit9625, were you able to reproduce the issue, or are you working on this? |
Yes I had reproduced the issue and currently working on it. |
Adding the
mobizen_20240123_165603.-.Trim.mp4
mobizen_20240123_165804.-.Trim.mp4Important
mobizen_20240123_171837.mp4The app crashes when tapped on the target button if location services are off. I want some assistance in resolving the double permission popup problem. |
This issue is already been solved by this PR #5418, so that is no longer an issue. Another issue I notice is that even after getting the permission for location, the Nearby fragment doesn't ask to turn on the location, unless tapped on the target icon (which is also the case in Explore fragment, being discussed in this issue).
Let me test and check the code once, I'll get back to you in some time (either here or on Zulip) |
After a lot of debugging, I found that this was because recenterMap() was calling the showLocationOffDialog() (which is supposed to happen on target icon click), but the ActivityResultLauncher was also calling showLocationOffDialog(), which shouldn't have happened, as ActivityResultLauncher itself shouldn't be called after permission is provided. So possible solutions are removing the call to showLocationOffDialog() from recenterMap(), but it poses it's own problems. Popping up of 2 dialogs is also caused just when permission is given in the Explore tab itself, not just when the target icon is clicked. I also found new issues while debugging such as Permission being requested twice simultaneously in the Explore tab (you can reproduce by denying permission once in the Explore tab, to see it pop up again) and some others (and their solutions). I suggest closing this issue, as we have come too far away from the main agenda of this issue, which was the opening of the app's setting instead of location settings (which seems to be already fixed). I can open a new issue to resolve all possible bugs and incorrect behaviour in the nearby and explore tabs, to clear these issues once and for all. @nicolas-raoul Are we allowed to open such an issue, which requests to solve all possible bugs in an fragment |
Hey @rohit9625, I can surely assist you with it, can you please ping me on Zulip |
I had noticed all the stuff you mention. And I agree with you that this issue is about something else which is resolved already. |
There are many more, actually. Upload wizard has a map, again some bugs appear there. If you're planning to fix all the issues, #5256 might be worth having a look. All fragments behave differently, you'll have to intensively test all of them unless we write more maintainable code. As far as this issue is concerned, I no longer have access to that device, so can't confirm. Not reproducible on an emulator, at least. Sivaraam faced the same issue, but I think this got resolved after bumping the targetSdk. Since this is no longer reproducible, I'll close it for now. Anyone who faces this issue in future, please feel free to open it again. Thanks!😄 |
Yes. I could confirm I too don't face this issue anymore. I'm properly taken to the Location setting of the device now. |
Summary
A "Requesting location permission" popup is used at various places in the app (like Explore fragment map and also in PR #5249) that asks users to turn location on for all apps in the Settings. The Settings button, however, takes the user to the app's settings instead of location settings where users can turn their GPS on.
Steps to reproduce
Expected behaviour
Tapping on Settings should take the users to location settings page where they can enable GPS.
Actual behaviour
The app's settings page is displayed instead of location settings. Since the user has already allowed location access in step 3, displaying this page isn't helpful.
Side note: The same button opens location settings page correctly on some devices.
Device name
Moto g62
Android version
12
Commons app version
master and prodDebug
Device logs
No response
Screen-shots
No response
Would you like to work on the issue?
None
The text was updated successfully, but these errors were encountered: