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

Fix: Fail to request camera permission in Scan receipt screen #26231

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ const CONST = {
},
RECEIPT: {
ICON_SIZE: 164,
PERMISSION_AUTHORIZED: 'authorized',
PERMISSION_GRANTED: 'granted',
HAND_ICON_HEIGHT: 152,
HAND_ICON_WIDTH: 200,
SHUTTER_SIZE: 90,
Expand Down
12 changes: 12 additions & 0 deletions src/pages/iou/ReceiptSelector/CameraPermission/index.android.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import {check, PERMISSIONS, request} from 'react-native-permissions';

function requestCameraPermission() {
return request(PERMISSIONS.ANDROID.CAMERA);
}

// Android will never return blocked after a check, you have to request the permission to get the info.
function getCameraPermissionStatus() {
return check(PERMISSIONS.ANDROID.CAMERA);
}

export {requestCameraPermission, getCameraPermissionStatus};
11 changes: 11 additions & 0 deletions src/pages/iou/ReceiptSelector/CameraPermission/index.ios.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import {check, PERMISSIONS, request} from 'react-native-permissions';

function requestCameraPermission() {
return request(PERMISSIONS.IOS.CAMERA);
}

function getCameraPermissionStatus() {
return check(PERMISSIONS.IOS.CAMERA);
}

export {requestCameraPermission, getCameraPermissionStatus};
5 changes: 5 additions & 0 deletions src/pages/iou/ReceiptSelector/CameraPermission/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function requestCameraPermission() {}

function getCameraPermissionStatus() {}

export {requestCameraPermission, getCameraPermissionStatus};
25 changes: 15 additions & 10 deletions src/pages/iou/ReceiptSelector/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import PropTypes from 'prop-types';
import {launchImageLibrary} from 'react-native-image-picker';
import {withOnyx} from 'react-native-onyx';
import {useIsFocused} from '@react-navigation/native';
import {RESULTS} from 'react-native-permissions';
import PressableWithFeedback from '../../../components/Pressable/PressableWithFeedback';
import Icon from '../../../components/Icon';
import * as Expensicons from '../../../components/Icon/Expensicons';
Expand All @@ -21,6 +22,7 @@ import useLocalize from '../../../hooks/useLocalize';
import ONYXKEYS from '../../../ONYXKEYS';
import Log from '../../../libs/Log';
import participantPropTypes from '../../../components/participantPropTypes';
import * as CameraPermission from './CameraPermission';

const propTypes = {
/** React Navigation route */
Expand Down Expand Up @@ -99,7 +101,8 @@ function ReceiptSelector(props) {

const camera = useRef(null);
const [flash, setFlash] = useState(false);
const [permissions, setPermissions] = useState('authorized');
const [permissions, setPermissions] = useState('granted');
const isAndroidBlockedPermissionRef = useRef(false);
const appState = useRef(AppState.currentState);

const iouType = lodashGet(props.route, 'params.iouType', '');
Expand All @@ -113,7 +116,7 @@ function ReceiptSelector(props) {
useEffect(() => {
const subscription = AppState.addEventListener('change', (nextAppState) => {
if (appState.current.match(/inactive|background/) && nextAppState === 'active') {
Camera.getCameraPermissionStatus().then((permissionStatus) => {
CameraPermission.getCameraPermissionStatus().then((permissionStatus) => {
setPermissions(permissionStatus);
});
}
Expand Down Expand Up @@ -156,12 +159,14 @@ function ReceiptSelector(props) {
};

const askForPermissions = () => {
if (permissions === 'not-determined') {
Camera.requestCameraPermission().then((permissionStatus) => {
// Android will never return blocked after a check, you have to request the permission to get the info.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also mentioned in the doc: https://github.com/zoontek/react-native-permissions/blob/a836e114ce3a180b2b23916292c79841a267d828/README.md?plain=1#L670

There's no way we can check for the BLOCKED status without requesting the permission first, even with react-native's PermissionsAndroid.

Context: https://developer.android.com/training/permissions/requesting#workflow_for_requesting_permissions (point 4).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know this is a hack thus really looking forward to your feedback!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should put this in the description as comments. I think this is fine for the current change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean the PR description, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • We should put a proper description on why we're doing along with the link in the code comment.

if (permissions === RESULTS.BLOCKED || isAndroidBlockedPermissionRef.current) {
Linking.openSettings();
} else if (permissions === RESULTS.DENIED) {
Copy link
Contributor

@ntdiary ntdiary Oct 23, 2023

Choose a reason for hiding this comment

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

Coming from #27442. We improved the UX to avoid the permission button does nothing on first click

CameraPermission.requestCameraPermission().then((permissionStatus) => {
setPermissions(permissionStatus);
isAndroidBlockedPermissionRef.current = permissionStatus === RESULTS.BLOCKED;
});
} else {
Linking.openSettings();
}
};

Expand Down Expand Up @@ -219,13 +224,13 @@ function ReceiptSelector(props) {
});
}, [flash, iouType, props.iou, props.report, reportID, translate]);

Camera.getCameraPermissionStatus().then((permissionStatus) => {
CameraPermission.getCameraPermissionStatus().then((permissionStatus) => {
setPermissions(permissionStatus);
});

return (
<View style={styles.flex1}>
{permissions !== CONST.RECEIPT.PERMISSION_AUTHORIZED && (
{permissions !== RESULTS.GRANTED && (
<View style={[styles.cameraView, styles.permissionView]}>
<Hand
width={CONST.RECEIPT.HAND_ICON_WIDTH}
Expand All @@ -248,7 +253,7 @@ function ReceiptSelector(props) {
</PressableWithFeedback>
</View>
)}
{permissions === CONST.RECEIPT.PERMISSION_AUTHORIZED && device == null && (
{permissions === RESULTS.GRANTED && device == null && (
<View style={[styles.cameraView]}>
<ActivityIndicator
size={CONST.ACTIVITY_INDICATOR_SIZE.LARGE}
Expand All @@ -257,7 +262,7 @@ function ReceiptSelector(props) {
/>
</View>
)}
{permissions === CONST.RECEIPT.PERMISSION_AUTHORIZED && device != null && (
{permissions === RESULTS.GRANTED && device != null && (
<Camera
ref={camera}
device={device}
Expand Down