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

Rollback image picker update that causes app to crash when clicking upload component #158

Closed
wants to merge 1 commit into from

Conversation

mrsarm
Copy link
Contributor

@mrsarm mrsarm commented Feb 1, 2021

Downgrade com.github.Mariovc:ImagePicker library 1.2.2 → 1.2.0 as it was before when working, to allow user to use the file chooser again.

Issue: #127

Notes

  • The ImagePicker library was updated from the version 1.2.0 to 1.2.2 two years ago on this commit: 7538d16 2 years ago, for sure it worked for some devices or older versions of Android 🤷‍♂️ on that moment. In fact the older version set again didn't work with a virtual Android 10 device, but it worked in my Android 10 and Android 11 devices (Webview flavor). I also tested it with an Android 6 device (XWalk) and it worked with the v1.2.0 set again. But we should test it with more devices and OS versions, like Android 4.4 if we continue supporting it.
  • The last version of the library is the one that was set (1.2.2), and the project don't have a newer version since 2018, and in 2019 was officially deprecated by the author, so maybe it is a good time to replace it with a more up-to-date library.
  • The author suggest a good replacement that it is also officially recommended by Google, but it supports Android 5 and above, so if we want to keep supporting Android 4.4 is not an option. Although I'm not sure yet wheter this replacement also allows to pick images from local storage or only supports the camera, in which case is also not a good fit.

…n upload field

Downgrade `com.github.Mariovc:ImagePicker` library ~1.2.2~ → 1.2.0
@mrsarm mrsarm requested review from garethbowen and craig-landry and removed request for garethbowen February 1, 2021 19:01
Copy link
Member

@craig-landry craig-landry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see back in 7538d16 this was updated, but I don't see any reason. Just staying current is reason enough, but it would be nice to know if there was something else involved. @mrsarm what are the changes in the library between 1.2.0 and 1.2.2? First it seems odd that a breaking change would happen on a minor revision change, but anyway, we don't seem to have much choice if things are broken. My only concern would be if there's some major security issue involved. Please verify that there was no such issue and if so, let's do the downgrade as proposed here, and then in the future when we can drop Android 4.4, do the recommend fix you'd noted and switch libraries.

I'm approving this for now under the assumption that there is no major issue in the version diff, but if you do see something like that, please note it here and we'll discuss further.

@garethbowen
Copy link
Member

I've created an issue for removing the deprecated library once we drop support for Android 4.x

@mrsarm
Copy link
Contributor Author

mrsarm commented Feb 3, 2021

Created tag v0.7.4-alpha.1 to release an alpha version with the patch. Once the CI build finish the APKs should be available to test.

In the https://gamma-cht.dev.medicmobile.org environment I left the report "Photos on Android - Demo Form" configured for testing.

@garethbowen
Copy link
Member

I've removed the PR from the project and added the Issue instead.

@craig-landry
Copy link
Member

I've removed the PR from the project and added the Issue instead.

Ah, thanks for the correction!

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.

3 participants