Skip to content
This repository has been archived by the owner on Jun 16, 2023. It is now read-only.

feat: allow camera scene when audio permissions are denied #2048

Merged
merged 10 commits into from
Jan 18, 2019

Conversation

n1ru4l
Copy link
Collaborator

@n1ru4l n1ru4l commented Jan 11, 2019

@n1ru4l wrote:

Furthermore I think we should differentiate between the both cases:

  • Camera allowed and Audio not allowed
  • Camera allowed and Audio allowed

In the first case the camera view is still functional and the developer should decide whether the app user is forced to also allow audio permissions or is allowed to record without video.

What this PR does:

  • Prevent the camera view from not being rendered if camera permissions are granted but audio permissions are not granted.
  • Prevent crashes when trying to record audio without audio permissions being granted.
  • Show meanigful warnings for users that forgot to add permissions to Info.plist/AndroidManifest.xml

How to test this?

yarn add react-native-camera@https://github.com/n1ru4l/react-native-camera.git#d77c9c3caddf6ff0e900b1c9e70e3fd7d550e9e5

TODO:

Affected Issues:

Closes #2051
Closes #2047

@n1ru4l n1ru4l changed the title feat: allow recording when audio permissions are denied feat: allow camera scene when audio permissions are denied Jan 11, 2019
@n1ru4l n1ru4l changed the title feat: allow camera scene when audio permissions are denied [WIP] feat: allow camera scene when audio permissions are denied Jan 11, 2019
src/RNCamera.js Outdated Show resolved Hide resolved
@HighSoftWare96
Copy link

I've tested this PR. If I set the prop
captureAudio={false}
it gives me this unhandled exception:
image
but the camera does not work, otherwise without that the application crashes.

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Jan 15, 2019

@HighSoftWare96 What does the crashlog offer?

@HighSoftWare96
Copy link

How can I retrieve the crashlog?

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Jan 15, 2019

@HighSoftWare96 check the console output in XCode

@HighSoftWare96
Copy link

@n1ru4l
I'm not able to install this anymore... the packager cannot find the module "./handlePermissions", something is wrong?

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Jan 15, 2019

@HighSoftWare96 Seems like you should clear your react-native packager cache. Try starting the package with yarn react-native start --reset-cache

@HighSoftWare96
Copy link

@n1ru4l Right now I'm using npm but I've already tried with npm start -- --reset-cache 🙌

Copy link
Collaborator

@sibelius sibelius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove common handle permission function?

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Jan 16, 2019

@sibelius wrote

Why remove common handle permission function?

Because they now differ as the RNCamera implementation returns an object with two boolean properties instead of one boolean value

@sibelius
Copy link
Collaborator

we should remove RCTCamera soon

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Jan 17, 2019

@sibelius Do you know why on my Nexus 5X with Android 8.1.0 there is no Dialog being shown for Permissions when running the example app?

Also I do not need to add <uses-permission android:name="android.permission.CAMERA"/> to the AndroidManifest.xml (which is not in the example app anyway). However the camera view still works.

I only have to add <uses-permission android:name="android.permission.RECORD_AUDIO"/> otherwise the call in the requestPermissions function will resolve with NEVER_ASK_AGAIN. However after adding it, it will instantly resolve with GRANTED instead of showing a dialog to the user.

This happens in both development and release builds

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Jan 18, 2019

Ok I understood it partially. The AndroidManifest.xml located in react-native-camera/android/src/main/AndroidManifest.xml is "merged" with the one of the SampleApp upon building, which explains why I do not have to specify <uses-permission android:name="android.permission.CAMERA"/> in the AndroidManifest.xml of the consumer of react-native-camera.

However I still do not understand why I do not see a dialog...

@@ -369,4 +373,22 @@ public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) {
}
});
}

@ReactMethod
public void checkIfRecordAudioPermissionsAreDefined(final Promise promise) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a new method similar to the one on ios to check if the key is defined in the AndroidManifest.xml before asking for the permissions.

@asanz8
Copy link

asanz8 commented Jan 18, 2019

@n1ru4l Tested, it solved the problem without setting captureAudio property to false. I used a physical device (Android).

@luissmg
Copy link

luissmg commented Jan 18, 2019

@asanz8 Thanks for testing! I will not test it then @n1ru4l

@n1ru4l n1ru4l changed the title [WIP] feat: allow camera scene when audio permissions are denied feat: allow camera scene when audio permissions are denied Jan 18, 2019
@n1ru4l n1ru4l merged commit 22533ed into react-native-camera:master Jan 18, 2019
@n1ru4l n1ru4l deleted the feature-audio-permissions branch January 18, 2019 14:58
n1ru4l pushed a commit that referenced this pull request Jan 18, 2019
# [1.9.0](v1.8.1...v1.9.0) (2019-01-18)

### Features

* **android:** add videoBitrate option for recordAsync ([#2055](#2055)) [skip release] ([d93a6c7](d93a6c7))
* allow camera scene when audio permissions are denied ([#2048](#2048)), Fixes [#2047](#2047), Fixes [#2051](#2051) ([22533ed](22533ed))
@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Jan 18, 2019

🎉 This PR is included in version 1.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

n1ru4l added a commit that referenced this pull request Jan 25, 2019
…ild (#2070), Fixes #2069

It seems like Apple is rejecting apps that access a Info.plist usage description that is not available. This conflicts with enhanced developer warnings I implemented in #2048.

To fix that I changed the implementation to only check if the usage descriptions are available in `DEBUG` mode. I also added a note developer warnings, that the app might crash in release mode because using a feature that requires a usage description without having one provided will result in an application crash).
n1ru4l pushed a commit that referenced this pull request Jan 25, 2019
## [1.9.2](v1.9.1...v1.9.2) (2019-01-25)

### Bug Fixes

* **ios:** only check permission descriptions availability in debug build ([#2070](#2070)), Fixes [#2069](#2069) ([3f8b795](3f8b795)), closes [#2048](#2048)
PermissionsAndroid.PERMISSIONS.CAMERA,
params,
);
hasCameraPermissions = cameraPermissionResult === PermissionsAndroid.RESULTS.GRANTED;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! Why you use different "requestPermissions" method for Camera.js and RNCamera.js? Can you add || cameraPermissionResult === true like in Camera.js? Because react-native return boolean-type result for devices before Android 6.0

thx!

sunil-dev7 added a commit to sunil-dev7/react-native-camera that referenced this pull request Jun 11, 2021
sunil-dev7 added a commit to sunil-dev7/react-native-camera that referenced this pull request Jun 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants