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

CB-13421: (osx) Added macOS support #290

Merged
merged 1 commit into from
Nov 4, 2017
Merged

Conversation

JoseExposito
Copy link

@JoseExposito JoseExposito commented Oct 10, 2017

HI all,

This pull request adds macOS support to cordova-plugin-camera.

I'm not sure if this project accepts new features, is open to community collaborations or even if I should be submitting this PR to git.apache.org... Some guidance will be truly appreciated 😄

Here is a little demo of how it works:

cordova-plugin-camera-macos-low

The JS code looks like:

document.querySelector('#testCamera').addEventListener('click', function() {
    navigator.camera.getPicture(onCameraOk, onError, {
        sourceType: Camera.PictureSourceType.CAMERA,
        destinationType: Camera.DestinationType.DATA_URL,
        allowEdit: true,
        encodingType: Camera.EncodingType.JPEG,
        quality: 100,
        targetWidth:  500,
        targetHeight: 100,
    })
})

document.querySelector('#testFile').addEventListener('click', function() {
    navigator.camera.getPicture(onFileOk, onError, {
        sourceType: Camera.PictureSourceType.PHOTOLIBRARY,
        destinationType: Camera.DestinationType.FILE_URI,
        encodingType: Camera.EncodingType.PNG,
        mediaType: Camera.MediaType.ALLMEDIA,
        targetWidth:  500,
        targetHeight: 100,
    })
})

As you can see it support image scale, PNG, JPEG, quality...

I'm interested in collaborate to migrate more plugins to macOS, but first I wanted to contact with the team to know if:

(1) The Cordova team is interested in supporting macOS and/or other desktop platforms
(2) I need to introduce any further changes in this pull request
(3) You are open to migrate all the other core plugins to macOS
(4) You are interested in supporting multi window in desktop platforms

Known issues: When adding the plugin to a project:

$ cordova plugin add https://github.com/JoseExposito/cordova-plugin-camera.git

It is necessary to open platforms/osx/HelloCordova.xcodeproj and manually add the plugin to platforms/osx/HelloCordova/config.xml:

<?xml version='1.0' encoding='utf-8'?>
<widget id="io.cordova.hellocordova" version="1.0.0" xmlns="http://www.w3.org/ns/widgets" xmlns:cdv="http://cordova.apache.org/ns/1.0">
    [...]
    <feature name="Camera">
        <param name="osx-package" value="CDVCamera" />
    </feature>
    [...]
</widget>

I noticed this happens with other Cordova core plugins with official macOS support like cordova-plugin-device or cordova-plugin-file.

I guess this is an issue with the osx platform. Could somebody help me to report this issue to the right project/maintainer?

@JoseExposito JoseExposito changed the title [osx platform] Added macOS support CB-13421: (osx) Added macOS support Oct 10, 2017
@janpio
Copy link
Member

janpio commented Oct 10, 2017

Nice PR, as far as I understand it.

For your questions 1 to 4 you might want to email the dev list: https://cordova.apache.org/contact/ (My guess: Yes, No, Yes, Yes)

For the "known issue" I think you can open an additional issue at issues.cordova.io - this shouldn't be that way I think.

@JoseExposito
Copy link
Author

I created a JIRA with the macOS bug:
https://issues.apache.org/jira/browse/CB-13424

@JoseExposito
Copy link
Author

The plugin issue (https://issues.apache.org/jira/browse/CB-13424) is fixed in this pull request (apache/cordova-osx#43), it'll be nice to have both merged.

@stevengill stevengill closed this Nov 4, 2017
@stevengill stevengill reopened this Nov 4, 2017
@stevengill
Copy link
Contributor

@JoseExposito Thanks! This PR looks great. We are definitely open to having the other plugins add support for OSX.

@stevengill stevengill merged commit f761934 into apache:master Nov 4, 2017
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