-
Notifications
You must be signed in to change notification settings - Fork 7
Improve Photo Controller #526
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
base: develop
Are you sure you want to change the base?
Improve Photo Controller #526
Conversation
…n-standard file browsers. The app now also verifies the extensions of files selected this way.
|
Will take a look at this tomorrow. Thanks! |
| String imagePath = imageUri.getPath(); | ||
| //If the uri comes from the default document picker, we don't have to worry about the | ||
| // extension. Android will already have made sure it is an image. | ||
| if (!"com.android.providers.media.documents".equals(imageUri.getAuthority())) { |
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.
When I get the chance I need to check the minSdkVersion for the app. Previous to KitKat, this was "media/external/images/media"
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.
min sdk is ICS. Can you be sure this PR works for ICS and above?
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.
Yessir o7
…to make sure the image picker works with older versions of android.
| <string name="err_invalid_phone">Please enter a valid phone number</string> | ||
| <string name="err_address_blank">Please enter an address</string> | ||
| <string name="err_cant_load_photo">Unable to load photo</string> | ||
| <string name="err_cant_load_photo_bad_extension">Sorry\, we couldn\'t load your file because it isn\'t an image.</string> |
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 better but it's possible to choose a valid image and get this error message. Can we just make it say "Sorry, unable to load you image"
|
Overall this is good. Still not the proper way to handle lots of different image managers but this has better error handling than the code we have now. If you can fix that string, I think we're good to merge. Then hopefully someone can tackle the proper way of doing this with the media store/cursor. |
Improved the PhotoController to now accept files selected via non-standard file browsers. Also added file extension verification for files selected this way.
This is for issue #486
Let me know what you guys think!