You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by dpolivy <gi...@git.apache.org> on 2014/10/01 00:01:07 UTC
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
GitHub user dpolivy opened a pull request:
https://github.com/apache/cordova-plugin-camera/pull/49
CB-7667 iOS8: Handle case where camera is not authorized
In iOS 7+, when the app does not have access to the camera, show a
prompt notifying the user so they're not puzzled by looking at a black
screen.
In iOS 8+, include a link on the dialog to open the Setting app to allow
the user to change their Camera privacy setting.
Fixes https://issues.apache.org/jira/browse/CB-7667
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dpolivy/cordova-plugin-camera cb7667
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugin-camera/pull/49.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 #49
----
commit cd3b637822e173b9ed9149fa648f45f8a46b5de9
Author: Dan Polivy <da...@cellartracker.com>
Date: 2014-09-30T21:28:24Z
CB-7667 iOS8: Handle case where camera is not authorized
In iOS 7+, when the app does not have access to the camera, show a
prompt notifying the user so they're not puzzled by looking at a black
screen.
In iOS 8+, include a link on the dialog to open the Setting app to allow
the user to change their Camera privacy setting.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by dpolivy <gi...@git.apache.org>.
Github user dpolivy commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-101119417
As this is iOS specific, I'm not sure it makes sense as a JavaScript parameter. Is there any provision for translating strings that are part of the Cordova project? Perhaps the translated versions can be added to plugin.xml, so they are automatically included in the native resources as necessary when the plugin is installed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by seronal <gi...@git.apache.org>.
Github user seronal commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-101083460
I don't know what is the official stance on this, but I don't think it's a good idea to hardcode an error message string (i.e. "Access to the camera has been prohibited; please enable it in the Settings app to continue."). This will be a problem in terms of localization. It should perhaps be passed within CameraOptions?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-101133525
I believe localization should never be up to the user of the component; the component itself should handle localization seamlessly.
This is how the Cordova Media Capture plugin does it:
1. https://github.com/apache/cordova-plugin-media-capture/tree/master/src/ios/CDVCapture.bundle
2. https://github.com/apache/cordova-plugin-media-capture/blob/f5514cabe19648a4c952bf05c09e088510895165/src/ios/CDVCapture.m#L39
To add more localizations, just add the appropriate .lproj folder with a Localizable.strings file -- send pull requests for adding each language as volunteers provide the strings.
In cordova-ios 4.0.x (coming in the future), these resource API functions will be available for plugin authors so they can easily localize their plugins: https://github.com/apache/cordova-ios/blob/ddadf8992bb4b34cd728ed9a21e880ad20e551db/CordovaLib/Classes/Public/CDVPlugin%2BResources.m#L24
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by seronal <gi...@git.apache.org>.
Github user seronal commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-101084822
Yes but not everyone uses native localization support. It's much easier to handle the localization on the javascript side, especially if you are targeting multiple platforms. This approach basically requires an extra plugin to create/move localization resources if you are using CLI based approach for development, not to mention having to create platform specific localized resources.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by dpolivy <gi...@git.apache.org>.
Github user dpolivy commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-69861487
I cleaned up the merge conflict, and I believe it's now working again. I had a bunch of issues upgrading my plugin (CDVCamera.m seemed to drop off the list of compiled source files?), but got those sorted out and it seems ok. Please do sanity test it on your side as well!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by dpolivy <gi...@git.apache.org>.
Github user dpolivy commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-100047309
@shazron Any possibility of a review & merge for the next plugin release?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by dpolivy <gi...@git.apache.org>.
Github user dpolivy commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-82518530
@shazron I just re-merged master and rebased the changes so it's a clean commit. Any chance of getting this pulled in to the next release?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-101083680
In the end it's not really hardcoded. If you notice, it is wrapped using NSLocalizedString. If you have a localized String resource file, it will load from the key "Access to the camera has been prohibited; please enable it in the Settings app to continue."
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by dpolivy <gi...@git.apache.org>.
Github user dpolivy commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-94880942
@stevengill @shazron Is it possible to get a review on this and some feedback on whether it is OK to merge?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-plugin-camera/pull/49
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-69424925
My apologies, the code has been refactored to be testable, so it doesn't merge cleanly anymore. It should be an easy rebase from your patch (I hope).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-7667 iOS8: Handle case wher...
Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/49#issuecomment-100305930
Thanks for the PR Dan -- I'm away, will take a look Monday.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org