fix: App shows blank screen when user denies camera access #6969

phivh · 2021-12-31T09:34:52Z

Details

The blank screen appears with missed handle errors when validating identity. Added 3 error messages that need to handle on iOS and Android platforms.

Fixed Issues

$ #6841

Tests

Tested by denying camera access on iOS and Android.

QA Steps

Go to the app
Send money to someone
Click on Continue button
Start verify identity
Deny access to camera
Go back then repeat step 4.
Open settings
Go back the app then verify

Tested On

Screenshots

Web

N/A

Mobile Web

N/A

Desktop

N/A

iOS

ios.mp4

Android

android.mp4

github-actions · 2021-12-31T09:35:11Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

phivh · 2021-12-31T09:36:28Z

I have read the CLA Document and I hereby sign the CLA

mountiny · 2021-12-31T09:36:39Z

@phivh thank you! Could you please copy the line above as per the instructions and comment it here. That is a mandatory step for any contributor. Cheers!

parasharrajat · 2021-12-31T11:09:54Z

src/CONST.js

+            // eslint-disable-next-line max-len
+            USER_CAMERA_CONSENT_DENIED: 'Unexpected result Intent. It might be a result of incorrect integration, make sure you only pass Onfido intent to handleActivityResult. It might be due to unpredictable crash or error. Please report the problem to [email protected]. Intent: null \n resultCode: 0',


Let's wrap this in a new line and remove the Eslint comment.

There are some reasons that I would like to keep it as it is.

Linting required single quote, if we want to wrap the message we need to use backquote `` it will cause an error by lint rule. However, the condition to match this message variable should be 100% match when the error cause, if we wrap it into 2 lines, the condition couldn't work as expected.

From my point of view, this is a message under constant variable and not a block logic code so it should not a big deal.

Ok. Anyways, it is not a blocker.

src/components/Onfido/index.native.js

parasharrajat · 2021-12-31T11:12:23Z

src/languages/es.js

+        cameraRequestTitle: '"New Expensify" Would Like to Access the Camera',
+        cameraRequestMessage: 'Uh-oh, you have not granted us camera access. We need that to complete this verification',


Please ask for these translations on the Slack channel or on the issue.

mountiny

I agree with Rajat's changes. And i will try to confirm the coly of the messages.

mountiny · 2021-12-31T18:18:39Z

src/languages/en.js

@@ -522,6 +522,8 @@ export default {
        tryAgain: 'Try again',
        verifyIdentity: 'Verify identity',
        genericError: 'There was an error while processing this step. Please try again.',
+        cameraRequestTitle: '"New Expensify" Would Like to Access the Camera',


We will also need to ask Marketing team to confirm these translations. I will try to take care of it in the OG issue

Thanks for the follow-up. I will wait until getting these official messages.

@phivh Thank you for understanding, this might be a bit longer than usual given the holidays. Apologies for the delay.

@mountiny I see, no problem. This is also a chance for me to get more understanding of the process.

@phivh Could you actually use the key cameraPermissionsNotGranted we have couple lines above here?

App/src/languages/en.js

Line 514 in bec6d3a

cameraPermissionsNotGranted: 'Camera permissions not granted',

Replate this line with the aforementioned line and swap the translations keys where you have used it to cameraPermissionsNotGranted. And remove this standalone key. I have checked and it seems it was not used anywhere.

mountiny

I have let the test run. Thank you for your patience on this one. Amazing work so far on this issue 🙌

mountiny · 2022-01-02T16:02:24Z

src/languages/en.js

@@ -522,6 +522,8 @@ export default {
        tryAgain: 'Try again',
        verifyIdentity: 'Verify identity',
        genericError: 'There was an error while processing this step. Please try again.',
+        cameraRequestTitle: '"New Expensify" Would Like to Access the Camera',


@phivh Thank you for understanding, this might be a bit longer than usual given the holidays. Apologies for the delay.

mountiny

Requesting one change to the copy. I will follow soon with the Spanish translation.

parasharrajat · 2022-01-04T19:21:10Z

@mountiny we will also need Spanish translations

parasharrajat

Approving and leaving the translations to @mountiny

🎀 👀 🎀 C+ reviewed

mountiny · 2022-01-04T19:24:05Z

@parasharrajat waiting for the translations :)

mountiny

I have written down some requested changed to the translations. As a title, we would use a copy which was already in the translations page, but it was not used anywhere. So let's just use that key and not forget to change it in the place we need.

Thank you for your patience and for making these updates here @phivh

mountiny · 2022-01-04T19:43:12Z

src/languages/en.js

@@ -522,6 +522,8 @@ export default {
        tryAgain: 'Try again',
        verifyIdentity: 'Verify identity',
        genericError: 'There was an error while processing this step. Please try again.',
+        cameraRequestTitle: '"New Expensify" Would Like to Access the Camera',


@phivh Could you actually use the key cameraPermissionsNotGranted we have couple lines above here?

App/src/languages/en.js

Line 514 in bec6d3a

cameraPermissionsNotGranted: 'Camera permissions not granted',

Replate this line with the aforementioned line and swap the translations keys where you have used it to cameraPermissionsNotGranted. And remove this standalone key. I have checked and it seems it was not used anywhere.

mountiny · 2022-01-04T19:43:59Z

src/languages/en.js

@@ -522,6 +522,8 @@ export default {
        tryAgain: 'Try again',
        verifyIdentity: 'Verify identity',
        genericError: 'There was an error while processing this step. Please try again.',
+        cameraRequestTitle: '"New Expensify" Would Like to Access the Camera',
+        cameraRequestMessage: 'Uh-oh, you have not granted us camera access. We need that to complete this verification',


Let's use this copy preferably please.

Suggested change

cameraRequestMessage: 'Uh-oh, you have not granted us camera access. We need that to complete this verification',

cameraRequestMessage: 'You have not granted us camera access. We need access to complete verification.',

mountiny · 2022-01-04T19:44:25Z

src/languages/es.js

@@ -522,6 +522,8 @@ export default {
        tryAgain: 'Intentar otra vez',
        verifyIdentity: 'Verificar identidad',
        genericError: 'Hubo un error al procesar este paso. Inténtalo de nuevo.',
+        cameraRequestTitle: '"New Expensify" Would Like to Access the Camera',


Use the cameraPermissionsNotGranted key instead of this and translations should be couple lines above.

mountiny · 2022-01-04T19:45:34Z

src/languages/es.js

@@ -522,6 +522,8 @@ export default {
        tryAgain: 'Intentar otra vez',
        verifyIdentity: 'Verificar identidad',
        genericError: 'Hubo un error al procesar este paso. Inténtalo de nuevo.',
+        cameraRequestTitle: '"New Expensify" Would Like to Access the Camera',
+        cameraRequestMessage: 'Uh-oh, you have not granted us camera access. We need that to complete this verification',


Lastly, the translation here is following.

Suggested change

cameraRequestMessage: 'Uh-oh, you have not granted us camera access. We need that to complete this verification',

cameraRequestMessage: 'No has habilitado los permisos para acceder a la cámara. Necesitamos acceso para completar la verificaciôn.',

phivh · 2022-01-05T14:41:05Z

Hi @mountiny @parasharrajat
Thanks for your help!
I've updated the translations. Please help to take a look when you guys have a chance.

mountiny

Great @phivh! Can you just please move the cameraPermissionsNotGranted to the onfidoStep in the translations? Can you also merge main and test it for the last time and we can merge! 🎉 Thank you very much for your work and patience with the translations, this usually does not take so long. Christmas 🎄

mountiny · 2022-01-05T20:20:20Z

src/components/Onfido/index.native.js

@@ -56,7 +56,7 @@ class Onfido extends React.Component {
                    errorMessage,
                )) {
                    Alert.alert(
-                        this.props.translate('onfidoStep.cameraRequestTitle'),
+                        this.props.translate('cameraPermissionsNotGranted'),


Could we please move the cameraPermissionsNotGranted to the onfidoStep? It is not used anywhere else and it is uncommon to have the keys in the first level.

phivh · 2022-01-06T09:16:20Z

@mountiny I moved the key cameraPermissionsNotGranted under Onfido level and merged the main branch, everything working fine. Thanks for your suggestion.

mountiny

@phivh Thank you very much! I have thoroughly enjoyed working on this with you and I hope to work with you on plenty more!

OSBotify · 2022-01-06T11:55:10Z

@phivh, Great job getting your first Expensify/App pull request over the finish line! 🎉

I know there's a lot of information in our contributing guidelines, so here are some points to take note of 📝:

Now that your first PR has been merged, you can be hired for another issue. Once you've completed a few issues, you may be eligible to work on more than one job at a time.
Once your PR is deployed to our staging servers, it will undergo quality assurance (QA) testing. If we find that it doesn't work as expected or causes a regression, you'll be responsible for fixing it. Typically, we would revert this PR and give you another chance to create a similar PR without causing a regression.
Once your PR is deployed to production, we start a 7-day timer ⏰. After it has been on production for 7 days without causing any regressions, then we pay out the Upwork job. 💰

So it might take a while before you're paid for your work, but we typically post multiple new jobs every day, so there's plenty of opportunity. I hope you've had a positive experience contributing to this repo! 😊

OSBotify · 2022-01-06T11:55:36Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

phivh · 2022-01-06T12:09:27Z

Thanks @mountiny @parasharrajat
Nice to work with you guys.

OSBotify · 2022-01-10T19:45:43Z

🚀 Deployed to staging by @mountiny in version: 1.1.26-2 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2022-01-10T20:02:15Z

🚀 Deployed to staging by @mountiny in version: 1.1.27-0 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2022-01-12T05:04:41Z

🚀 Deployed to production by @roryabraham in version: 1.1.27-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

fix: App shows blank screen when user denies camera access

3f5dbee

phivh requested a review from a team as a code owner December 31, 2021 09:34

MelvinBot requested review from mountiny and parasharrajat and removed request for a team December 31, 2021 09:34

parasharrajat suggested changes Dec 31, 2021

View reviewed changes

mountiny reviewed Dec 31, 2021

View reviewed changes

cleanup

8d2b057

mountiny reviewed Jan 2, 2022

View reviewed changes

mountiny requested changes Jan 4, 2022

View reviewed changes

parasharrajat previously approved these changes Jan 4, 2022

View reviewed changes

mountiny self-requested a review January 4, 2022 19:35

mountiny requested changes Jan 4, 2022

View reviewed changes

Update copies

03fe3a6

phivh dismissed parasharrajat’s stale review via 03fe3a6 January 5, 2022 14:39

mountiny requested changes Jan 5, 2022

View reviewed changes

phivh added 2 commits January 6, 2022 15:37

Merge branch 'main' into fix/blank-screen-when-user-denies-camera-access

4b6e17b

Cleanup

2f5603d

mountiny self-requested a review January 6, 2022 11:53

mountiny approved these changes Jan 6, 2022

View reviewed changes

mountiny merged commit 5863d0f into Expensify:main Jan 6, 2022

OSBotify mentioned this pull request Jan 10, 2022

Deploy Checklist: New Expensify 2022-01-10 #7108

Closed

24 tasks

botify mentioned this pull request Jan 12, 2022

[HOLD for payment 2022-01-19] App shows blank screen when user denies camera access when validating identity - Reported by @adeel0202 #6841

Closed

		// eslint-disable-next-line max-len
		USER_CAMERA_CONSENT_DENIED: 'Unexpected result Intent. It might be a result of incorrect integration, make sure you only pass Onfido intent to handleActivityResult. It might be due to unpredictable crash or error. Please report the problem to [email protected]. Intent: null \n resultCode: 0',

		cameraRequestTitle: '"New Expensify" Would Like to Access the Camera',
		cameraRequestMessage: 'Uh-oh, you have not granted us camera access. We need that to complete this verification',

	cameraRequestMessage: 'Uh-oh, you have not granted us camera access. We need that to complete this verification',
	cameraRequestMessage: 'You have not granted us camera access. We need access to complete verification.',

	cameraRequestMessage: 'Uh-oh, you have not granted us camera access. We need that to complete this verification',
	cameraRequestMessage: 'No has habilitado los permisos para acceder a la cámara. Necesitamos acceso para completar la verificaciôn.',

fix: App shows blank screen when user denies camera access #6969

fix: App shows blank screen when user denies camera access #6969

Conversation

phivh commented Dec 31, 2021

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

github-actions bot commented Dec 31, 2021 • edited Loading

phivh commented Dec 31, 2021

mountiny commented Dec 31, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mountiny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mountiny Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

mountiny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mountiny left a comment

Choose a reason for hiding this comment

parasharrajat commented Jan 4, 2022

parasharrajat left a comment

Choose a reason for hiding this comment

mountiny commented Jan 4, 2022

mountiny left a comment

Choose a reason for hiding this comment

mountiny Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phivh commented Jan 5, 2022 • edited Loading

mountiny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phivh commented Jan 6, 2022

mountiny left a comment • edited Loading

Choose a reason for hiding this comment

OSBotify commented Jan 6, 2022

OSBotify commented Jan 6, 2022

phivh commented Jan 6, 2022

OSBotify commented Jan 10, 2022

OSBotify commented Jan 10, 2022

OSBotify commented Jan 12, 2022

github-actions bot commented Dec 31, 2021 •

edited

Loading

mountiny Jan 4, 2022 •

edited

Loading

mountiny Jan 4, 2022 •

edited

Loading

phivh commented Jan 5, 2022 •

edited

Loading

mountiny left a comment •

edited

Loading