-
Notifications
You must be signed in to change notification settings - Fork 426
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
AC-788 Added crop image feature at registration screen #768
Conversation
@rishabh-997 thanks for working on this,got some failing tests on ci,you could checkout the ci logs |
There was some strange Kotlin error from the previous PR. Interestingly, they did not appear in the previous PR and suddenly appear in successive ones. |
Codecov Report
@@ Coverage Diff @@
## master #768 +/- ##
==========================================
- Coverage 11.81% 11.80% -0.01%
==========================================
Files 254 254
Lines 10918 10926 +8
Branches 1087 1091 +4
==========================================
Hits 1290 1290
- Misses 9540 9548 +8
Partials 88 88
Continue to review full report at Codecov.
|
openmrs-client/src/main/java/org/openmrs/mobile/activities/logs/LogsFragment.kt
Outdated
Show resolved
Hide resolved
@rishabh this error must have popped up due to some open Open Issue in kotlin java interop. where there is one more workaround that you can mark that field as @Jvmfield :) |
...lient/src/main/java/org/openmrs/mobile/activities/addeditpatient/AddEditPatientFragment.java
Show resolved
Hide resolved
openmrs-client/src/main/java/org/openmrs/mobile/activities/logs/LogsFragment.kt
Outdated
Show resolved
Hide resolved
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.
@rishabh-997 Just a few comments to take care of.
...lient/src/main/java/org/openmrs/mobile/activities/addeditpatient/AddEditPatientFragment.java
Show resolved
Hide resolved
...lient/src/main/java/org/openmrs/mobile/activities/addeditpatient/AddEditPatientFragment.java
Outdated
Show resolved
Hide resolved
openmrs-client/src/main/java/org/openmrs/mobile/activities/logs/LogsFragment.kt
Outdated
Show resolved
Hide resolved
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.
@rishabh-997 I'm not able to access UCrop's feature, see the recording at https://imgur.com/a/z3QEmqx . Tested on API level 21 and 30.
I think there's a problem with the outdated way of getting URI from a file using Uri.fromFile
, we should use AndroidX's way of getting it by using FileProvider.
...lient/src/main/java/org/openmrs/mobile/activities/addeditpatient/AddEditPatientFragment.java
Show resolved
Hide resolved
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.
LGTM @rishabh-997
Description of what I changed
Issue I worked on
JIRA Issue: https://issues.openmrs.org/browse/AC-788
Checklist: I completed these to help reviewers :)
(the number above, next to the 'Commits' tab is 1).
existing code that was well tested you do not have to add tests)