[HOLD for payment 2024-11-20] [$250] Scan - Infinite loading for scan option if it is selected after manual.

[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: 9.0.56-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5168073&group_by=cases:section_id&group_order=asc&group_id=309127
Issue reported by: Applause - Internal Team

Action Performed:

1. Click on FAB
2. Click on submit expense
3. Verify the manual option is selected
4. Select the Scan option

Expected Result:

"Take a photo, Camera access is required to take pictures of receipts" or a camera interface are displayed

Actual Result:

Infinite loading for scan option if it is selected after manual

Workaround:

Unknown

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6651242_1730388131736.Infinite_loading_for_scan_option_if_it_is_selected_after_manual.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021852097315271105119
• Upwork Job ID: 1852097315271105119
• Last Price Increase: 2024-10-31
• Automatic offers:
• QichenZhu | Contributor | 104755565

Issue Owner

Current Issue Owner: @joekaufmanexpensify

[melvin-bot]

Triggered auto assignment to @joekaufmanexpensify (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]

@joekaufmanexpensify 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

[lanitochka17]

We think that this bug might be related to #wave-collect - Release 1

[joekaufmanexpensify]

Reproduced.

2024-10-31_17-16-09.mp4

[melvin-bot]

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

[melvin-bot]

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

[TalhaAmjad0034]

Proposal

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

When switching from the "manual" option to the "scan" option in the app, the user encounters an infinite loading screen rather than seeing the camera interface or receiving a camera permissions prompt.

What is the root cause of that problem?

The problem arises because the isTabFocused state fails to update accurately during tab transitions. This inconsistency stems from delays in syncing isPageFocused and tabPositionAnimation, which cause isTabFocused to remain in an incorrect state. As a result, the app cannot reliably trigger the camera interface or permissions prompt, leading to an indefinite loading state.

App/src/hooks/useTabNavigatorFocus/index.ts

Lines 53 to 82 in bfce277

	useEffect(() => {
	if (!tabPositionAnimation) {
	return;
	}
	const listenerId = tabPositionAnimation.addListener(({value}: PositionAnimationListenerCallback) => {
	// Activate camera as soon the index is animating towards the `tabIndex`
	DomUtils.requestAnimationFrame(() => {
	setIsTabFocused(value > tabIndex - 1 && value < tabIndex + 1);
	});
	});
	// We need to get the position animation value on component initialization to determine
	// if the tab is focused or not. Since it's an Animated.Value the only synchronous way
	// to retrieve the value is to use a private method.
	// eslint-disable-next-line no-underscore-dangle
	const initialTabPositionValue = tabPositionAnimation.__getValue();
	if (typeof initialTabPositionValue === 'number') {
	DomUtils.requestAnimationFrame(() => {
	setIsTabFocused(initialTabPositionValue > tabIndex - 1 && initialTabPositionValue < tabIndex + 1);
	});
	}
	return () => {
	if (!tabPositionAnimation) {
	return;
	}
	tabPositionAnimation.removeListener(listenerId);
	};
	}, [tabIndex, tabPositionAnimation]);

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

To fix this, we need a robust synchronization between isPageFocused and tabPositionAnimation:

1. Separate tracking for isPageFocused using useEffect, so isTabFocused updates immediately when isPageFocused or tabPositionAnimation changes.
2. Introduce a check to update isTabFocused dynamically based on the current position range (tabIndex - 1 to tabIndex + 1) in tabPositionAnimation to ensure a reliable focus state when the user changes tabs.

This approach maintains state accuracy across tab changes, ensuring the scan tab loads the camera or permissions prompt as intended.

// Add a listener to track the animation's current position (value) and update isTabFocused accordingly.
        const listenerId = tabPositionAnimation.addListener(({value}: PositionAnimationListenerCallback) => {
            // Determine if the tab is within the visible range based on tabIndex and update focus status.
            const isInFocusRange = value >= tabIndex - 1 && value <= tabIndex + 1;
            const newFocusState = isInFocusRange && isPageFocused;

            // Request an animation frame to set isTabFocused for smoother updates.
            DomUtils.requestAnimationFrame(() => {
                setIsTabFocused(newFocusState);
            });
        });

        // Obtain the initial tab position for an accurate focus status on component load.
        const initialPosition = tabPositionAnimation.__getValue();
        const isInitiallyFocused = initialPosition > tabIndex - 1 && initialPosition < tabIndex + 1 && isPageFocused;

        // Set the initial focus state based on the current position and page focus status.
        setIsTabFocused(isInitiallyFocused);

        // Clean up the listener when the component unmounts or tabPositionAnimation changes.
        return () => {
            if (tabPositionAnimation) {
                tabPositionAnimation.removeListener(listenerId);
            }
        };
    }, [tabIndex, isPageFocused, tabPositionAnimation]);

    useEffect(() => {
        // If tabPositionAnimation is not available, update isTabFocused directly based on isPageFocused.
        if (!tabPositionAnimation) {
            setIsTabFocused(isPageFocused);
        } else {
            // Update focus state directly whenever isPageFocused changes, ensuring synchronization with tab animation.
            const updateFocusDirectly = () => {
                const value = tabPositionAnimation.__getValue();
                const isWithinTabIndex = value >= tabIndex - 1 && value <= tabIndex + 1;
                const shouldBeFocused = isWithinTabIndex && isPageFocused;

                setIsTabFocused(shouldBeFocused);
            };

            // Request an animation frame for a smoother update when isPageFocused changes.
            DomUtils.requestAnimationFrame(updateFocusDirectly);
        }
    }, [isPageFocused]);

What alternative solutions did you explore? (Optional)

One alternative solution considered was to use a custom animation trigger to monitor tab focus directly. However, this approach would have added complexity without fully addressing the root cause related to synchronization between isPageFocused and tabPositionAnimation.

Attached screen record of local version with above fix in useTabNavigatorFocus/index.ts

iphone-scan-test.mp4

[QichenZhu]

Edited by proposal-police: This proposal was edited at 2024-11-05 10:40:31 UTC.

Proposal

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

When open IOU with the Manual tab and switching to the Scan tab, the camera loads infinitely.

What is the root cause of that problem?

After PR #51020 was merged, which removed the use of animation state from TabSelector on web platforms, the listener of tabPositionAnimation is not called when switching tabs.

App/src/hooks/useTabNavigatorFocus/index.ts

Line 57 in bfce277

const listenerId = tabPositionAnimation.addListener(({value}: PositionAnimationListenerCallback) => {

So useTabNavigatorFocus doesn't work, and isTabActive is not changed, which mean it is false when you open the Manual tab, and it is still false after you switch to the Scan tab.

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

Line 95 in bfce277

const isTabActive = useTabNavigatorFocus({tabIndex});

The camera cannot get the permission state if isTabActive is false.

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

Lines 172 to 174 in bfce277

	if (!Browser.isMobile() || !isTabActive) {
	return;
	}

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

Line 189 in bfce277

setIsQueriedPermissionState(true);

So it shows a spinner.

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

Lines 616 to 617 in bfce277

	{((cameraPermissionState === 'prompt' && !isQueriedPermissionState) || (cameraPermissionState === 'granted' && isEmptyObject(videoConstraints))) && (
	<ActivityIndicator

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

Use the navigation state instead of the animation state as the source of truth to detect the active tab by replacing useTabNavigatorFocus with useIsFocused.

import {useIsFocused} from '@react-navigation/native';

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

Line 95 in bfce277

const isTabActive = useTabNavigatorFocus({tabIndex});

const isTabActive = useIsFocused();

App/src/pages/iou/request/step/IOURequestStepScan/NavigationAwareCamera/WebCamera.tsx

Line 11 in 6778419

const shouldShowCamera = useTabNavigatorFocus({

const shouldShowCamera = useIsFocused();

What alternative solutions did you explore? (Optional)

The mentioned PR addressed a very minor issue #50265, so maybe it could simply be reverted.

[ntdiary]

Under review.

I think the RCA in @QichenZhu's proposal is correct, but I don't think it's necessary to revert that PR for this issue. As for those alternative solutions, I've noticed several issues related to camera scan before, so need more time to verify to ensure there are no regressions. :)

[ntdiary]

@QichenZhu, regarding your alternative solutions, I think removing isTabActive is not very ideal. If the cameraPermissionState is granted, requestCameraPermission would be called when Scan mounting, which is unnecessary. However, I agree with your basic idea, we could use useIsFocused on web platform, or like in native, use useFocusEffect.
Can you please update your proposal so that we can move forward with it. Additionally, we also need to replace useTabNavigatorFocus with useIsFocused in WebCamera. :)

[QichenZhu]

Proposal

Updated

@ntdiary Thank you very much for your comments. I updated the main solution and moved the original main solution as a last option.

[ntdiary]

App/src/hooks/useTabNavigatorFocus/index.ts

Line 57 in 4603c82

const listenerId = tabPositionAnimation.addListener(({value}: PositionAnimationListenerCallback) => {

On web platforms, since tabPositionAnimation is actually an AnimatedDivision instance, so the listener won't be called.

I agree with @QichenZhu’s proposal and think replacing useTabNavigatorFocus with useIsFocused here is sufficient. After this fix, the mWeb behavior should still be similar to native app. :)

test.mp4

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[joekaufmanexpensify]

PR merged, awaiting deploy to staging

[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 9.0.60-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix web camera not loading #52083

If no regressions arise, payment will be issued on 2024-11-20. 🎊

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

• @ntdiary requires payment through NewDot Manual Requests
• @QichenZhu requires payment automatic offer (Contributor)

[melvin-bot]

@ntdiary @joekaufmanexpensify @ntdiary The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[joekaufmanexpensify]

@ntdiary could you handle checklist so we can prep for payment?

[ntdiary]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: https://github.com/Expensify/App/pull/51020/files#r1847832599

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue: https://github.com/Expensify/Expensify/issues/446317

Regression Test Proposal

Precondition:

Test:

1. Log in with any account.
2. Click FAB.
3. Choose Submit expense.
4. Make sure Manual tab is selected. If not, select it and navigate back, then repeat step 1 ~ 3.
5. Switch to Scan tab.
6. Verify that there won't be infinite loading, and Take a photo text or camera interface will appear.

Do we agree 👍 or 👎

[joekaufmanexpensify]

Regression test is here: https://github.com/Expensify/Expensify/issues/446317

[joekaufmanexpensify]

All set to pay. We need to pay:

• @QichenZhu $250 for PR via Upwork.
• @ntdiary $250 for C+ via NewDot.

[joekaufmanexpensify]

@QichenZhu $250 sent and contract ended!

[joekaufmanexpensify]

Upwork job closed.

[joekaufmanexpensify]

@ntdiary Please feel free to request at your earliest convenience. Closing for now as that is the only thing left!

[JmillsExpensify]

$250 approved for @ntdiary