-
Notifications
You must be signed in to change notification settings - Fork 1k
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
add preserveAspectRatio to CameraOptions while resizing an image #3309
Conversation
android/capacitor/src/main/java/com/getcapacitor/plugin/camera/ImageUtils.java
Show resolved
Hide resolved
@iphilgood To test we usually just add a test button in the example app in the 2.x branch: https://github.com/ionic-team/capacitor/tree/2.x/example With Capacitor 3 we'll be doing more unit testing. 😁 |
@dwieeb I'm gonna fix your reviews tomorrow 👍🏽 Thanks for having a look at it. Are you gonna add the test button or is it me? And do you know why my build failed? I ran the command on my local machine and except for some warnings I didn't see an error 🤔 Hahaha alright 😄 Some tests are always a good idea 😉 |
@iphilgood If you could add the button, that would be great! Just making sure there's a new button for using Sometimes the |
@dwieeb I've resolved your comments and added a new button to test my code (somehow the menu is not working in the example app. I had to set the RootPage accordingly) 😄 I think the other buttons still work as expected but it would be nice if someone else could have a look at it. |
@iphilgood There isn't a button for the menu but you can swipe from the left of screen because it's just a sidemenu. This works great on Android and iOS! Nice work! |
@dwieeb Hahahaha I'm sorry for that 😅 Now everything makes sense 😂 Thank you! |
Awesome work @iphilgood 🎉 |
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.
Added two comments.
Other than that, looks good to me.
android/capacitor/src/main/java/com/getcapacitor/plugin/Camera.java
Outdated
Show resolved
Hide resolved
Okay I've resolved all the comments. Thanks everybody 🤗 |
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've fixed a typo
Looks good to me
2.4.0 was just released with this! |
Awesome, thanks @iphilgood and @dwieeb 🎉 |
This PR is a re-opening of #3297 because of wrong base branch and a retry of #2050 and should fix ionic-team/capacitor-plugins#2 (also the discussion #1952). I've used the Android implementation of @sandstrom (shout-out) and also did an implementation for iOS.
There's now a
preserveAspectRatio
flag on CameraOptions which is set tofalse
by default.I've also fixed a bug where the app would crash while saving an image to the gallery on Android. This only happens on certain devices but I was able to reproduce it by creating 32 images until it crashed. A random UUID gets now appended to the file name to make sure no name clashes happen anymore.
@dwieeb is there anything I can do to provide some testing or how do you test this fix?