-
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
fixes issues related to location permission denial even when allowed inside in-app camera flow #5313
fixes issues related to location permission denial even when allowed inside in-app camera flow #5313
Conversation
…inside in-app camera flow
Some bug came up with #5299 merging, sorry for all the confusion caused. Just needed to add a return statement at one place. |
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 start testing, I will hopefully be able to merge within 12 hours. :-)
Unless I misunderstand, it seems like the issue described in #5273 is still present on my Android 14 Pixel 7 Pro as seen in the screencast below? screen-20230927-094208.mp4Namely, the in-app camera does not open the second time after tapping "Don't allow" when asked "Allow Commons to access this device's location?" by the Android OS. Note: I believe the notification permission dialog seen in the screencast is unrelated. |
Hi @nicolas-raoul this fix was in relation to @sivaraam 's comment here. I tested and the actual issue which was raised does exist, seems like its an issue with dexter library that is used for permission management as in this dexter issue. Dexter is completely ignoring location permissions flow for some reason. Working on it. |
Ah yes, I can confirm that the issue reported by Sivaraam in that comment is fixed indeed, thanks! 🙂 |
@nicolas-raoul Made the fixes for location flow when using camera. Removed the use of dexter library for location permissions and used native permission methods instead, please review. Thanks! |
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.
Looks fine, and works well, I tried many times, denying or removing location permission, and never observed the problem of in-app camera not opening.
Removing Dexter usage is sensible as that library is unmaintained.
Description (required)
Fixes #5273
What changes did you make and why?
Location permission issue fixed for in-app camera flow.
Tests performed (required)
Tested {prodDebug} on {pixel 5 emulator} with API level {30}.
Screenshots (for UI changes only)
https://github.com/commons-app/apps-android-commons/assets/53987325/961d4a5d-5a00-4cfd-9fcf-afe2cf873379
Need help? See https://support.google.com/android/answer/9075928
Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.