[PAID] [$500] Scan - Inconsistency of flashlight/torch behavior in Scan tab

[lanitochka17]

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: 1.4.24-4
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Prerequisite: Camera use permission is accepted previously
Android App

1. Login to android Expensify App
2. Tap FAB button
3. Tap Request money
4. Tap on flashlight icon to enable (verify nothing happens)
5. Tap on green circle to take a shoot with camera (verify photo is taken with flashlight is ON)
   Mobile Web
6. Login to ND Expensify
7. Tap FAB button
8. Tap Request money
9. Tap on flashlight icon to enable (verify flashlight is ON)
10. Tap on flashlight icon to disable ((verify flashlight is OFF)

Expected Result:

Flashlight should function exactly same behaviour both mobile apps and mobile web
Flashlight icon should be shown in Scan tab within all devices

Actual Result:

Both Android and IOS apps works similarly; flashlight is only ON while taking a photo shoot
In Mobile Web, tapping on the flash icon turns the flashlight ON&OFF without camera taking a shoot

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6340357_1705077132365.Realme_GT2_Pro_App_Chrome.mp4

Bug6340357_1705077132330.No_flashlight_icon_on_mobile_web.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017684a08e378933c3
• Upwork Job ID: 1745866187913547776
• Last Price Increase: 2024-01-12
• Automatic offers:
• s77rt | Reviewer | 28108708
• tienifr | Contributor | 28108709

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[neonbhai]

Proposal

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

Inconsistency of flashlight/torch behavior in Scan tab

What is the root cause of that problem?

We don’t pass the torch param here

What changes do you think we should make in order

We should pass the torch prop like we do for web:

App/src/pages/iou/request/step/IOURequestStepScan/index.js

Line 221 in c8631be

torchOn={isFlashLightOn}

torchOn={flash}

To make sure we have torch available, we may use the onTorchAvailability prop [Ref: here] to set a custom state (to disable the torch button from press):

App/src/pages/iou/request/step/IOURequestStepScan/index.js

Line 222 in c8631be

onTorchAvailability={setIsTorchAvailable}

Alternatively

We may consider renaming flash variable to a more descriptive name like isFlashLightOn like we have on web

[bernhardoj]

Proposal

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

The torch for native only appears when taking a picture, but for mWeb, the torch shows immediately when turned on.

What is the root cause of that problem?

For native, the behavior is the flash is applied when taking the picture.

App/src/pages/iou/request/step/IOURequestStepScan/index.native.js

Lines 187 to 190 in c8631be

	camera.current
	.takePhoto({
	qualityPrioritization: 'speed',
	flash: flash ? 'on' : 'off',

But for web, we set up the torch like this:

App/src/pages/iou/request/step/IOURequestStepScan/NavigationAwareCamera/index.js

Lines 49 to 57 in c8631be

	useEffect(() => {
	if (!trackRef.current) {
	return;
	}
	trackRef.current.applyConstraints({
	advanced: [{torch: torchOn}],
	});
	}, [torchOn]);

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

For native, the camera can receive a torch prop that we can set to true.

torch={flash && device.hasTorch}

[tienifr]

Proposal

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

Both Android and IOS apps works similarly; flashlight is only ON while taking a photo shoot
In Mobile Web, tapping on the flash icon turns the flashlight ON&OFF without camera taking a shoot

What is the root cause of that problem?

In Android/iOS native, we only apply the torch when the user is taking photo here. So the flash only applies when talking photos.

Meanwhile for web, we'll turn on the flashlight as soon as the user press on it in the app. This is not correct because the flashlight should only be on when taking photos, similar to how native camera works. This is to avoid unintended inconvenience for other people and also save device battery life.

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

Similar to native Android, iOS, if the user enable flash, in web we should only apply the torch (by trackRef.current.applyConstraints({ ) right before taking the screenshot here, after that we should turn the torch off (also by trackRef.current.applyConstraints({ ). If the user doesn't enable flash/torch, then we don't need to do anything and can just take the screenshot normally.

What alternative solutions did you explore? (Optional)

NA

[s77rt]

@neonbhai Thanks for the proposal. I think we are looking to achieve the consistency here the other way around i.e. web should behave like native.

[s77rt]

@bernhardoj Thanks for the proposal. Same note as above ^

[neonbhai]

@s77rt hi, purely from a UX perspective, I will expect the flash button to turn on flash immediately (and if it doesn't do that, it seems broken). The user is very used to this happening from other camera interfaces.

Maybe we could have eyes from the design team on this?

User Story

User clicks receipt in dark environment:

1. User needs flash as they are clicking receipt in low light mode
2. User would want the flash to turn on so they can frame it
3. User clicks flash icon
4. The button just flickers
5. User cannot frame it, as the flash only turns on for a split second

[s77rt]

@tienifr Thanks for the proposal. I agree on the RCA. Regarding the solution, can we improve this and use the track to construct ImageCapture. This Web API have a takePhoto method that supports fillLightMode to choose if flash should be enabled.

PS: I think it would be better if we ship this feature upstream to react-webcam

[s77rt]

@neonbhai The mobile (native) behavior was implemented first. Then support was added to web. I think it makes sense to match the web to the native. If you think the native design is incorrect please raise that to Slack (and tag me and the design team).

[tienifr]

@tienifr Thanks for the proposal. I agree on the RCA. Regarding the solution, can we improve this and use the track to construct ImageCapture. This Web API have a takePhoto method that supports fillLightMode to choose if flash should be enabled.

@s77rt thanks for the suggestion! we can definitely explore that, I'll try it and update the proposal accordingly 👍

[tienifr]

Thanks for the proposal. I agree on the RCA. Regarding the solution, can we improve this and use the track to construct ImageCapture

@s77rt it seems that this Web API is only an experimental feature and is not supported in many major browsers like Safari, Firefox (reference), so I think we won't be able to use it in our app, except if we check the browser then decide which approach to use in the code, which I'm not sure is worth the trouble, and the old approach still has to be used for some browsers anyway.

[s77rt]

Raised this on Slack https://expensify.slack.com/archives/C01GTK53T8Q/p1705368355349939 for more 👀

[s77rt]

I think we should proceed here with @tienifr's proposal.

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

Triggered auto assignment to @luacmartins, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @tienifr 🎉 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 📖

[melvin-bot]

This issue has not been updated in over 15 days. @strepanier03, @s77rt, @luacmartins, @tienifr eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[s77rt]

@tienifr still working on this

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.56-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: Inconsistency of flashlight/torch behavior in Scan tab #34782

If no regressions arise, payment will be issued on 2024-04-03. 🎊

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment automatic offer (Reviewer)
• @tienifr requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@s77rt] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@strepanier03] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[strepanier03]

This is on hold for payment for another weekish so just clearing the overdue until then.

[s77rt]

• The PR that introduced the bug has been identified: Refactor the money request creation flow #28618
• The offending PR has been commented on: Refactor the money request creation flow #28618 (comment)
• A discussion in #expensify-bugs has been started: Not needed (unique bug)
• Determine if we should create a regression test for this bug: No, I don't think we need a test for this inconsistency

[strepanier03]

Looks like the automation didn't work here either. Handling payment stuff now.

[strepanier03]

All paid, closing!