[HOLD for payment 2024-06-28] [$250] [Search v1] iOS - Search - Background behind the transaction thread is blank when swiping to right

[izarutskaya]

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: v1.4.81-8
Reproducible in staging?: Y
Reproducible in production?: New feature
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Launch New Expensify app.
2. Go to Search.
3. Open any transaction.
4. Swipe the page to the right and do not release the finger.

Expected Result:

The background behind the transaction thread will show Search page.

Actual Result:

The background behind the transaction thread is blank.

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

Bug6509255_1718104435100.RPReplay_Final1718104258.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014f3e060c40c4a8f2
• Upwork Job ID: 1800531818313064614
• Last Price Increase: 2024-06-11
• Automatic offers:
• eh2077 | Reviewer | 102704810
• bernhardoj | Contributor | 102704811

Issue Owner

Current Issue Owner: @puneetlath

[melvin-bot]

Triggered auto assignment to @srikarparsi (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

Triggered auto assignment to @puneetlath (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.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[izarutskaya]

@puneetlath 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.

[izarutskaya]

We think this issue might be related to the #collect project.

[luacmartins]

I don't think this needs to be a blocker since the feature works and it's a minor UI issue.

[melvin-bot]

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

[melvin-bot]

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

[TheGithubDev]

Proposal

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

The background behind the transaction thread is blank when swiping to the right on iOS devices.

What's the root cause of that problem?

The root cause of this issue is due to the way the navigation stack or screen rendering is managed during the swipe gesture. When swiping to the right, the screen behind the transaction thread is not properly rendered or visible, resulting in a blank background.

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

Ensure that the background screen (Search page) is properly rendered and visible during the swipe gesture. This can be achieved by:

1. Ensure Proper Rendering of Background Screen:

• Make sure the Search page is retained in the view hierarchy and is properly rendered when the transaction thread is swiped to the right.

2. Adjust Navigation Stack Management:

• Ensure the navigation stack is managed in a way that the Search page remains visible behind the transaction thread during the swipe gesture.

Please note that the above explanation outlines only the technical approach I plan to take. I'll submit the finalized solution after getting accepted, as mentioned.

Thanks!

[bernhardoj]

Proposal

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

The background is blank when swiping to close the report RHP.

What is the root cause of that problem?

When we go to the search page, we add both search bottom tab and search central pane. (both have the same url)

App/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar/index.tsx

Lines 100 to 104 in 8d11d0b

	<Tooltip text={translate('common.search')}>
	<PressableWithFeedback
	onPress={() => {
	interceptAnonymousUser(() => Navigation.navigate(ROUTES.SEARCH.getRoute(CONST.TAB_SEARCH.ALL)));
	}}

In small screen, we only want to show the search bottom tab, so, to hide the central pane in small screen, we have a custom navigation state logic to hide the search central pane.

App/src/libs/Navigation/AppNavigator/createCustomStackNavigator/index.tsx

Lines 69 to 79 in 8d11d0b

	// On narrow layout, if we are on /search route we want to hide all central pane routes and show only the bottom tab navigator.
	if (isSmallScreenWidth && isLastRouteSearchRoute) {
	return {
	stateToRender: {
	...state,
	index: 0,
	routes: [firstRoute],
	},
	searchRoute: lastRoute,
	};
	}

One condition that we need to focus is isLastRouteSearchRoute. It checks whether the last route is the search central pane or not.

App/src/libs/Navigation/AppNavigator/createCustomStackNavigator/index.tsx

Line 65 in 8d11d0b

const isLastRouteSearchRoute = getTopmostCentralPaneRoute({routes: [lastRoute]} as State<RootStackParamList>)?.name === SCREENS.SEARCH.CENTRAL_PANE;

However, when we open the report/transaction RHP (or any page), the last route is not search central pane anymore, but the report RHP, thus the search central pane is not hidden anymore. When we swipe to right from the report RHP, the blank screen is the search central pane. Why blank?

It's because we return null if it's a small screen.

App/src/pages/Search/SearchPage.tsx

Lines 38 to 42 in 8d11d0b

	// On small screens this page is not displayed, the configuration is in the file: src/libs/Navigation/AppNavigator/createCustomStackNavigator/index.tsx
	// To avoid calling hooks in the Search component when this page isn't visible, we return null here.
	if (isSmallScreenWidth) {
	return null;
	}

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

Always filter out the search central pane from the navigation state to render.

if (isSmallScreenWidth) {
    const searchCentralPaneIndex = routes.findIndex(route => route.name === 'CentralPaneNavigator' && route.params && 'screen' in route.params && route.params.screen === SCREENS.SEARCH.CENTRAL_PANE);
    const filteredRoutes = [...routes.slice(0, searchCentralPaneIndex), ...routes.slice(searchCentralPaneIndex + 1)];
    return {
        stateToRender: {
            ...state,
            index: filteredRoutes.length - 1,
            routes: filteredRoutes,
        },
        searchRoute: routes[searchCentralPaneIndex],
    };
}

[luacmartins]

The RCA and solution make sense to me. Maybe we could use a .filter to make the code a bit cleaner though. Otherwise looks good.

[melvin-bot]

📣 @eh2077 🎉 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]

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

[bernhardoj]

PR is ready

cc: @eh2077

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

• Filter out search central pane in small screen #43628

If no regressions arise, payment will be issued on 2024-06-28. 🎊

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

• @bernhardoj requires payment automatic offer (Contributor)
• @eh2077 requires payment automatic offer (Reviewer)

[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:

• [@eh2077] The PR that introduced the bug has been identified. Link to the PR:
• [@eh2077] 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:
• [@eh2077] 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:
• [@eh2077] Determine if we should create a regression test for this bug.
• [@eh2077] 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.
• [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[puneetlath]

@bernhardoj has been paid. @eh2077 bump on the checklist so we can pay you.

[eh2077]

Checklist

• [@eh2077] The PR that introduced the bug has been identified. Link to the PR: I don't think there's a specific PR responsible for this issue. It's an edge case that wan't covered by app-navigation
• [@eh2077] 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: N/A
• [@eh2077] 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: N/A
• [@eh2077] Determine if we should create a regression test for this bug. Yes

Regression test

1. Open search page
2. Open any report
3. Verify the transaction screen opens
4. (iOS/mWeb iOS) Swipe right and don't release the finger (Android/Android mWeb) Press back button
5. Verify you can see the search page list under the transaction screen

Do we agree 👍 or 👎

cc @puneetlath

[puneetlath]

Regression test issue: https://github.com/Expensify/Expensify/issues/409354

All paid. Thanks everyone!