-
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
[Bug]: Improve the media location access dialog strings #5276
Comments
Hello, I've also observed this bug. Would it be possible for me to give it a try? |
@sivaraam Just for double check: Do you mean we should display a dialogue block before requesting for media location access permission? For now, that dialogue block will be displayed if the user declines the request. |
@nicolas-raoul , I found another potential bug when I was working on this issue (please see below). If you can confirm this bug, then we can possibly open a new issue. Summary: Title: Location Bug Description: If a user declines the request for location access, the app will crash when the user tries to upload a picture. I guess this is due to the reason that the app doesn't have location access and can't implement a location function. If so, the app should request the user for location access again instead of crashing directly. Steps to reproduce Expected behaviour Actual behaviour Device name Android version Commons app version Device logs Would you like to work on the issue? |
@1911-revo I can not reproduce the crash on my Pixel7 strangely. Would you mind installing from the latest main branch, trying again, and sending a screencast if it happens again? Thanks a lot! |
Modify the sequence of displaying dialogues
@nicolas-raoul , I just found a potential bug based on this current pull request: |
@sivaraam Any opinion on the proposed pull request? :-) |
Apologies for the rather delayed response @1911-revo. I'm kind of unable to reproduce the issue described here at the moment. So, I'm not able to give a suggestion on what the suggested flow should be. Are you able to get the "Media location" access dialog now?
This crash should hopefully be fixed now I suppose. |
@sivaraam, As mentioned here, the ACCESS_MEDIA_LOCATION is a runtime permission, but the permission request states that access to media and photos is requested, instead of asking for access to media location. Upon accepting this permission, sometimes only the ACCESS_MEDIA_LOCATION is given, but not the READ_EXTERNAL_STORAGE or WRITE_EXTERNAL_STORAGE, thus this behaviour was observed by you. I think it would be better if we show the dialog for asking for External storage permissions, but we will ask for both permissions during runtime. Also, this permission is asked just as the app is opened, which is against the Android guidelines (which state that we should ask for permissions only when the user is interacting with a feature that requires that permission). |
@nicolas-raoul @sivaraam, what are your thoughts on the above comment? Do you think I should implement the above flow? |
Hi Shaswat! From your comment, I could roughly understand that you're proposing to avoid asking for the permission as soon as the app is opened. I'm not very sure what flow you're actually proposing as a solution. Could you elaborate on the same? |
Summary
We show a dialog requesting for media location access permission so that we would be able to access the location from images. The title and description of that dialog are as follows:
Title: Media location access denied
Description: We may not be able to automatically obtain location data from pictures you upload. Please add the appropriate location for each picture before submitting
This is completely misleading since hitting "Ok" on that dialog is shown right before we request for the permission. The strings are appropriate to be shown only if we shown them right after knowing that we're denied that permission.
We need to tweak the code such that we show an appropriate string before requesting for the media location permission. We could show the above string if we're denied that permission for some reason.
Steps to reproduce
master
branchExpected behaviour
We should show an dialog stating why we need the media location access permission. If we're denied the permission, we should show the existing description shown above.
Actual behaviour
We're showing a dialog with the existing description above before requesting access to the media location permission.
Device name
OnePlus Nord
Android version
Android 12
Commons app version
master @ Sep 04
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: