Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.9k forks source link

[$250] mWeb/Chrome - IOU- Continue and green button not working in scan page #50485

Open lanitochka17 opened 1 month ago

lanitochka17 commented 1 month ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.46 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): Negasofonias@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to any chat or workspace
  2. Click ln + button and select submit expense or split bill
  3. In the scan page try clicking continue button or the big green button
  4. Observe that both buttons dont work

Expected Result:

Both buttons should open phones camera

Actual Result:

Non functional buttons

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/385493cf-12e6-495e-8d8f-3c77381d76b9

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021847268462588592344
  • Upwork Job ID: 1847268462588592344
  • Last Price Increase: 2024-10-18
  • Automatic offers:
    • dominictb | Contributor | 104509149
    • layacat | Contributor | 104696564
Issue OwnerCurrent Issue Owner: @dominictb
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

lanitochka17 commented 1 month ago

@dylanexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

layacat commented 1 month ago

Edited by proposal-police: This proposal was edited at 2024-10-09 04:04:45 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

mWeb/Chrome - IOU- Continue and green button not working in scan page

What is the root cause of that problem?

This issue often happens when user accidentally dismissed the request permission dialog (or blocked the camera permission). On the mWeb Chrome, once you blocked, you can't show the request permission dialog again.

Screenshot 2024-10-09 at 10 49 28

Now, when you don't have the permission, when you clicked the capture or Continue button, nothing happened because we early returned here: https://github.com/Expensify/App/blob/6556e9a31ad3433a2d9471d0ad1029919c549c55/src/pages/iou/request/step/IOURequestStepScan/index.tsx#L482-L485

What changes do you think we should make in order to solve the problem?

  1. We should let user know if they denied the permission and let them know how to resolve it, by changing the description text to explain when they press the button to capture photo here, ie:
    Camera access still hasn't been granted, please follow these instructions

And navigate user to our help site when user clicks on these instructions text link.

  1. Add a new infor section under our help site here to help user when they don't know why the Continue doesn't work: https://github.com/Expensify/App/blob/bfce2771d22cbecceee6cf3c064fe3f0627eb5d8/docs/articles/new-expensify/expenses-%26-payments/Create-an-expense.md?plain=1#L41

{% include info.html %}
If the Continue button doesn't open the camera on your mobile web browser, 
it might be because you denied camera permission. Learn how to enable camera permission in your browser [here](https://support.google.com/chrome/answer/2693767).
{% include end-info.html %}

What alternative solutions did you explore? (Optional)

Update the description of the text in here when user denied the camera permission (And disabled the continue/capture button) to let user know why they can't perform the action. https://github.com/Expensify/App/blob/6556e9a31ad3433a2d9471d0ad1029919c549c55/src/pages/iou/request/step/IOURequestStepScan/index.tsx#L582

For an example:


 <Text style={[styles.subTextFileUpload]}>{isPermissionDenied ? translate('receipt.userDeniedPermission') : translate('receipt.cameraAccess')}</Text> 

With receipt.cameraAccess is Camera access still hasn't been granted, please follow these instructions..

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

dylanexpensify commented 1 month ago

Reviewing shortly!

melvin-bot[bot] commented 1 month ago

@dylanexpensify Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 4 weeks ago

@dylanexpensify Still overdue 6 days?! Let's take care of this!

dylanexpensify commented 4 weeks ago

apologies, reviewing!

melvin-bot[bot] commented 3 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~021847268462588592344

melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @dominictb (External)

melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @RachCHopkins (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

dylanexpensify commented 3 weeks ago

Hey @RachCHopkins! I'm heading out on parental leave so reassigning this! TY! πŸ™‡β€β™‚οΈ

melvin-bot[bot] commented 3 weeks ago

πŸ“£ @dominictb πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

RachCHopkins commented 3 weeks ago

Sorry @dominictb, I don't think Dylan meant to unassign you as the C+ here.

melvin-bot[bot] commented 3 weeks ago

@RachCHopkins, @dominictb Whoops! This issue is 2 days overdue. Let's get this updated quick!

dominictb commented 3 weeks ago

Reviewing now.

dominictb commented 3 weeks ago

@layacat Thanks for the proposal. I agree with your RCA. As per my investigation:

Then the permission modal would never appear again.

Your main solution seems to be in the right direction. On Native apps, we would show an alert prompting user to go to device's Settings and manually update permission.

Screenshot 2024-10-22 at 20 17 24

However, for mobile browsers, sometimes we need to do that via the browser's "Site settings". AFAIK, there's no way to navigate to that settings directly from our site.

dominictb commented 3 weeks ago

So let's ask @Expensify/design for their opinions.

Problem

On mobile browsers, if camera access was denied once, it would never prompt again. That causes the "Continue" button in Scan page unresponsive:

We can't just open the device's settings to let user manually update the permission like we did on Native because that is not enough for browsers:

Solutions

I came up with 2 ideas:

  1. Display the detailed instruction to manually grant camera access like Onfido did:
Screenshot 2024-10-22 at 22 39 54
  1. Hide "Continue" button and show a message like this instead:

Or do you have any other suggestions?

dannymcclain commented 3 weeks ago

@Julesssss can you weigh in here? I feel like we already put a bunch of effort into solving this?

melvin-bot[bot] commented 3 weeks ago

@RachCHopkins @dominictb this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Julesssss commented 3 weeks ago

Yeah, this is similar to the mWeb location permission issues. Though in this case, it is triggered via a user action (instead of automatically), and it is critically necessary for the flow.

I think the best solution here would be to update the text on button press when permission still hasn't been granted. We show a shortened message explaining how to enable the permission. And the 'continue' button will then remain as it works today. Once they finally enabled permission the button will function without any other changes being necessary.

@dannymcclain LMK if that sounds reasonable to you and we can move forward with some messaging

dannymcclain commented 3 weeks ago

Sounds good to me.

Julesssss commented 3 weeks ago

Great, so I think we should only change the message once. By default the no-permission-granted message says ''Camera access is required to take pictures of receipts'.

The first time 'Continue' is pressed without permission ON WEB ONLY, we can change the message to something specific. I think ideally we would link to a help site page that explains the full options.

Maybe 'Camera access still hasn't been granted, please follow these instructions.'

dominictb commented 2 weeks ago

@layacat Would you mind updating your proposal based on https://github.com/Expensify/App/issues/50485#issuecomment-2431561580 and https://github.com/Expensify/App/issues/50485#issuecomment-2432298226?

melvin-bot[bot] commented 2 weeks ago

@Julesssss, @RachCHopkins, @dominictb Eep! 4 days overdue now. Issues have feelings too...

RachCHopkins commented 2 weeks ago

Waiting on updated proposal.

layacat commented 2 weeks ago

Updated proposal based on above suggestions πŸ˜„

dominictb commented 2 weeks ago

@layacat We only show the instructions message (i.e. Camera access still hasn't been granted...) as the last resort when the access prompt modal never displays again. How could we know that?

As per my own investigation, Chrome allows only one denial while Safari (v.18) allows up to 3 denials. Unless there're better ways, you might need to conduct thorough tests on different browsers and versions to confirm the correct behavior.

dominictb commented 2 weeks ago

@Julesssss I have been looking for an appropriate external help article and I only found this article for Chrome on Google official support site and none for Safari (there're plenty but none is on Apple/Safari official help site).

Do you think we need to create a dedicated page/note for that in our own help site (e.g., Under SmartScan a receipt section in this article). Or just any external article is fine?

dominictb commented 2 weeks ago

@layacat How is it going?

@Julesssss Can you quick check https://github.com/Expensify/App/issues/50485#issuecomment-2443832514?

I'm trying to resolve all my concerns before approving so we would save time during PR phase.

Julesssss commented 2 weeks ago

Do you think we need to create a dedicated page/note for that in our own help site (e.g., Under SmartScan a receipt section in this article). Or just any external article is fine?

Hey yeah, I think a Help Site article would be preferred here.

layacat commented 2 weeks ago

@layacat How is it going?

I think we should just display the text if the permission is denied and revert back to default text if permission is allowed. Wdyt?

dominictb commented 2 weeks ago

Since the "Continue" button would keep requesting camera permission. I think it's fine to just show the text when cameraPermissionState is denied regardless of the browser-specific maximum denials.

@layacat Would you mind updating your proposal in more details to include the above condition and the Help Site article mentioned in https://github.com/Expensify/App/issues/50485#issuecomment-2449505062?

After that we're good to go.

layacat commented 2 weeks ago

Sure. I will update it today!

layacat commented 1 week ago

Updated my proposal again to include the help site step.

dominictb commented 1 week ago

We might need copy confirmation and detailed troubleshoot steps in Help site article instead of just referencing the external link. We'll resolve those during PR phase.

@layacat proposal https://github.com/Expensify/App/issues/50485#issuecomment-2401239179 LGTM.

πŸŽ€ πŸ‘€ πŸŽ€ C+ Reviewed

melvin-bot[bot] commented 1 week ago

Current assignee @Julesssss is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

melvin-bot[bot] commented 1 week ago

πŸ“£ @layacat πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

layacat commented 1 week ago

I'm working on the PR!

melvin-bot[bot] commented 1 week ago

@Julesssss, @RachCHopkins, @dominictb, @layacat Whoops! This issue is 2 days overdue. Let's get this updated quick!

layacat commented 1 week ago

I can finally run the docs project! PR will be ready tomorrow!

melvin-bot[bot] commented 1 week ago

@Julesssss @RachCHopkins @dominictb @layacat this issue is now 4 weeks old, please consider:

Thanks!

Julesssss commented 1 week ago

Seems like the PR close to being put in review.