[GitHub] cordova-plugin-camera pull request: Major refactor
Github user omefire commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-190840500 Oops, my bad. :) Reviewing that PR instead. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-190836981 @omefire I did close this one :wink: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user omefire commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-190836679 I see that you opened another PR: https://github.com/apache/cordova-plugin-camera/pull/185 Please, close this one then. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe closed the pull request at: https://github.com/apache/cordova-plugin-camera/pull/101 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user omefire commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-190409102 just started reviewing this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user adamscybot commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-190153265 With a project as popular as cordova is -- its so disappointing to see most of the "official" plugins are bug ridden and feel abandoned. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-155457722 Very disappointed to see that our efforts have been completely overlooked. This PR actually fixes a bunch of bugs that have since been fixed, and others that may not have. The point of doing a major refactor was to simplify the plugin and make more apparent the role of each option on the image processing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user athibaud commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-122946563 :wave: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-115352285 @shazron Any updates on this? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-109964558 The most probable is that it is a side effect, since I did not touch the implementation of `imageByScalingNotCroppingForSize` one bit :wink: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user shazron commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-109397387 This is amazing, thank you for this. This seems to fix: https://issues.apache.org/jira/browse/CB-5942 although I'm not sure how exactly (side effect?). I'll have a look at the code, and comment/integrate soon. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
GitHub user dieppe opened a pull request: https://github.com/apache/cordova-plugin-camera/pull/101 Major refactor Hi there, We needed access to the metadata which lead to asynchronous operations. We took the opportunity to refactor most of the plugin logic. Hopefully it's more readable and actually fixes some known bugs (e.g. we deal properly with NATIVE_URI wherever possible). This is a call for feedback and testing while I try to fix a known bug. The commit message is quite explicit, so here it is: --- The major thing added here is that metadata is correctly handled in most (all?) cases. I also think some bugs have been fixed, but I did not check the bug list ;) As a foreword, I want to say that I am far from being an expert in either Objective-C nor the iOS APIs (I only did some basic patches before). So please correct me if I say/did anything wrong. # Global flow As a general design decision, I tried to break all the workflow in small readable methods that either return something (I chose to use reference passing when returning more than one result which I find clearer than returning an NSDictionary where the result is specified nowhere), or use the convention completionBlock / resultBlock / failureBlock which it seems Apple APIs use. There are three main paths from the image retrieval to the result. 1. The user wants the original image from the PhotoLibrary or from the SavedPhotoAlbum with a destination type NATIVE_URI. 2. The user wants a modified version of the image, wherever it comes from. 3. The user wants the geolocation. # Option incompatibilities There are also some combinations of options that are disallowed because they donât make any sense: 1. if the user wants an image with a source type CAMERA, and a destination type NATIVE_URI, he has to save it to the photo album. An image that is not there does not have a NATIVE_URI. 2. if the user wants an image with a source type PHOTOLIBRARY or SAVEDPHOTOALBUM, needs editing (resize, orientation correction), and a destination type NATIVE_URI, he has to save it to the photo album. Otherwise, we would return the original image with no modification. As we can see, if we want a destination type of NATIVE_URI, there are only two possible paths: either we want an unmodified version of a library image, or we need to save to the photo album. # Metadata Regarding metadata, we do not need it in only one case, when the destination type is NATIVE_URI, the source is the library, and no edits have to be done. Otherwise, we need to either fetch if from the UIImagePickerControllerMediaMetadata key of the info dictionary provided by UIImagePickerController in case the source type is CAMERA, or from the ALAsset corresponding to the library image (see https://developer.apple.com/library/ios/documentation/UIKit/Reference/UIImagePickerControllerDelegate_Protocol/index.html#//apple_ref/doc/constant_group/Editing_Information_Keys). This actually introduces asynchronicity in the metadata retrieval, since we first need to fetch the ALAsset using UIImagePickerControllerReferenceURL. *We cannot use UIImage as it strips out most (all?) of the metadata*. Once we actually retrieved the metadata we need to modify it for every transformation of the image (orientation, resize, gps). # Saving to the Photo Album In the previous code, the saving was done after computing the CDVResult. This is not the case anymore as we need access to the saved ALAsset when we want to return a NATIVE_URI. The saving is now done after all image editing has occurred. # Asynchronous processing Since the metadata retrieval and the album saving is done asynchronously, the process pipeline itself is made asynchronous. We can use resultBlock and failureBlock to get the processed image with its corresponding metadata or to get the image Asset NATIVE_URI. # FIXMEs The documentation lacks some clarity for two things (see FIXME 1 & 2 in code): - Does the quality option apply to the library images as well. This would mean that we cannot use NATIVE_URI with the quality set, except if we save to the photo album. - Does the orientation correction apply to the library images as well. Same thing. I actually removed that options at first, but this would mean that we need to read the EXIF data in the js code and orient the image appropriately (except for NATIVE_URIs since the web view does that automagically, but orientation correction does not make sense here). # TODOs - usesGeolocation does not make sense for an image with a source type other that CAMERA (maybe on some fringe cases?) - fix a bug where allowsEdit + saveToPhotoAlbum + NATIVE_URI make the image save in the wrong orientation (despite the metadata having the correct or
[GitHub] cordova-plugin-camera pull request: major refactor : readability
GitHub user purplecabbage opened a pull request: https://github.com/apache/cordova-plugin-camera/pull/96 major refactor : readability aliased lots of the super long namespaces so we aren't fighting past with the 100 char line. You can merge this pull request into a Git repository by running: $ git pull https://github.com/purplecabbage/cordova-plugin-camera master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-plugin-camera/pull/96.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #96 commit 15f66bf5cd446fa4636a7132976140a7e5295712 Author: Jesse MacFadyen Date: 2015-05-19T04:38:34Z major refactor : readability --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
