[Due for payment 2025-02-18] [$250] Search - Different behavior when using app and device back button in selection mode

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.10-2
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
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch New Expensify app
2. Submit an expense to any user
3. Go to Search
4. Long press on the expense
5. Tap Select
6. Tap Back button on the top left
7. Note that only the selection mode is dismissed and app remains on the same page
8. Tap Select
9. Go back using device navigation

Expected Result:

App will dimiss the selection mode and remains on the same page

Actual Result:

App returns to the previous page

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

Bug6549153_1721637282851.Screen_Recording_20240722_161640_New_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0189e7d6f2ae38bc4a
• Upwork Job ID: 1818838854171056546
• Last Price Increase: 2024-08-01
• Automatic offers:
• nkdengineer | Contributor | 103414449

Issue Owner

Current Issue Owner: @trjExpensify

[melvin-bot]

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

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

[Krishna2323]

Proposal

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

Search - Different behavior when using app and device back button in selection mode

What is the root cause of that problem?

• We aren't handling back button in SearchPageBottomTab.
  

  App/src/pages/Search/SearchPageBottomTab.tsx

  Line 54 in b4c5ac5

  <ScreenWrapper

• We need to stop the navigation if isMobileSelectionModeActive is true.

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

We should use BackHandler.addEventListener('hardwareBackPress' to handle device back button and stop the navigation if isMobileSelectionModeActive is true.

    useEffect(() => {
        const backAction = () => {
            if (isMobileSelectionModeActive) {
                setIsMobileSelectionModeActive(false);
                return true;
            }
            return false;
        };
        BackHandler.addEventListener('hardwareBackPress', backAction);
        return () => {
            BackHandler.removeEventListener('hardwareBackPress', backAction);
        };
    }, [isMobileSelectionModeActive]);

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

@adelekennedy 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

@adelekennedy 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[adelekennedy]

added to a project

[melvin-bot]

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

[melvin-bot]

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

[nkdengineer]

Proposal

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

App returns to the previous page

What is the root cause of that problem?

We already have a custom back handler for Android here but it doesn't cover the case when the selectionMode is enabled.

App/src/libs/Navigation/setupCustomAndroidBackHandler/index.android.ts

Line 34 in d69b203

if (bottomTabRoutes[bottomTabRoutes.length - 1].name === SCREENS.SEARCH.BOTTOM_TAB && focusedRoute?.name === SCREENS.SEARCH.CENTRAL_PANE) {

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

We should handle the back button for the selection mode case in SearchPageBottomTab.

App/src/pages/Search/SearchPageBottomTab.tsx

Lines 74 to 76 in d69b203

	onBackButtonPress={() => {
	clearSelectedTransactions();
	turnOffMobileSelectionMode();

While testing I found a problem that is if we add a hardwareBackPress listener and call the function as we do for the back button in the header here, this doesn't work. The problem is the selectedTransactions is not cleared before the selection mode is turned off and then this useEffect is triggered to turn on the selection mode again. It may be related to the state and lifeCycle of React here.

App/src/components/Search/index.tsx

Lines 112 to 114 in d69b203

	if (selectedKeys.length > 0 && !selectionMode?.isEnabled) {
	turnOnMobileSelectionMode();
	}

The solution here is we should only turn off the selection mode after the selectedTransactions is cleared. The details step here:

1. Add an extra field in SearchContext data like shouldTurnOffSelectionMode with the default value is false to detect whether we should turn off the selection mode if the selectedTransactions is empty.

App/src/components/Search/SearchContext.tsx

Line 13 in d69b203

const Context = React.createContext<SearchContext>(defaultSearchContext);

2. Accept an extra param in clearSelectedTransactions that will update shouldTurnOffSelectionMode

const clearSelectedTransactions = useCallback(
    (searchHash?: number, shouldTurnOffSelectionMode = false) => {
        if (searchHash === searchContextData.currentSearchHash) {
            return;
        }
        setSearchContextData((prevState) => ({
            ...prevState,
            shouldTurnOffSelectionMode,
            selectedTransactions: {},
        }));
    },
    [searchContextData.currentSearchHash],
);

App/src/components/Search/SearchContext.tsx

Line 35 in d69b203

const clearSelectedTransactions = useCallback(

and update shouldTurnOffSelectionMode to false when we set the selected transaction here to avoid the selection mode being turned off when we deselect the last selected option.

const setSelectedTransactions = useCallback((selectedTransactions: SelectedTransactions) => {
    setSearchContextData((prevState) => ({
        ...prevState,
        selectedTransactions,
        shouldTurnOffSelectionMode: false,
    }));
}, []);

App/src/components/Search/SearchContext.tsx

Line 28 in d69b203

const setSelectedTransactions = useCallback((selectedTransactions: SelectedTransactions) => {

3. Create a useEffect in here to turn off the selection mode if the seleactedTransactions is empty and shouldTurnOffSelectionMode is true

useEffect(() => {
    const selectedKeys = Object.keys(selectedTransactions).filter((key) => selectedTransactions[key]);
    if (selectedKeys.length === 0 && selectionMode?.isEnabled && shouldTurnOffSelectionMode) {
        turnOffMobileSelectionMode();
    }
}, [selectedTransactions, selectionMode?.isEnabled, shouldTurnOffSelectionMode]);

App/src/components/Search/index.tsx

Line 75 in d69b203

function Search({queryJSON, policyIDs, isCustomQuery}: SearchProps) {

OPTIONAL: We can also create a function in SearchContext to reset the shouldTurnOffSelectionMode and use this in the useEffect above after we turn off the selection mode

4. In SearchPageBottomTab, add an event listener for the back button of device and we will call clearSelectedTransactions with shouldTurnOffSelectionMode as true

const handleBackButtonPress = useCallback(() => {
    if (!selectionMode?.isEnabled) {
        return false;
    }
    if (selectionMode?.isEnabled) {
        clearSelectedTransactions(undefined, true);
        return true;
    }
}, [selectionMode, clearSelectedTransactions]);

useEffect(() => {
    const backHandler = BackHandler.addEventListener('hardwareBackPress', handleBackButtonPress);

    return () => backHandler.remove();
}, [handleBackButtonPress]);

We can also call clearSelectedTransactions(undefined, true) here

onBackButtonPress={() => { 
     clearSelectedTransactions(undefined, true);
}

App/src/pages/Search/SearchPageBottomTab.tsx

Lines 74 to 76 in d69b203

	onBackButtonPress={() => {
	clearSelectedTransactions();
	turnOffMobileSelectionMode();

The test branch here for more details: https://github.com/nkdengineer/App/tree/fix/45896

What alternative solutions did you explore? (Optional)

Result

Screen.Recording.2024-08-01.at.12.57.08.mov

[melvin-bot]

@eVoloshchak @adelekennedy 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!

[eVoloshchak]

I think we should proceed with @nkdengineer's proposal

🎀👀🎀 C+ reviewed!

[melvin-bot]

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

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

• Fix: different behavior when using app and device back button in selection mode #46937

If no regressions arise, payment will be issued on 2025-02-18. 🎊

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

• @eVoloshchak requires payment through NewDot Manual Requests
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

@eVoloshchak @adelekennedy @eVoloshchak 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]

[melvin-bot]

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

[adelekennedy]

@trjExpensify I will be ooo for payment on the 18th - adding you here!

[trjExpensify]

Sure thing! Let's get the checklist here, @eVoloshchak.

[trjExpensify]

Bump on the checklist @eVoloshchak to pay this out.

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: there isn't a PR that has introduced this, this is a default behavior we're changing to make it consistent across platforms (at least the ones we can)
• 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
• 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: this is a simple bug, I don't think an additional discussion is needed

Regression Test Proposal

iOS/Android native only

1. Submit an expense to any user
2. Go to Search
3. Long press on the expense
4. Tap Select
5. Tap Back button on the top left
6. Note that only the selection mode is dismissed and the app remains on the same page
7. Tap Select
8. Go back using device navigation
9. Verify that the App will dismiss the selection mode and remain on the same page

Do we agree 👍 or 👎

[Julesssss]

The test is valid, but I don't think we need a unique test for this case.

[trjExpensify]

Fair enough. Payment summary as follows:

• $250 to @eVoloshchak for the C+ review (go ahead and request)
• $250 to @nkdengineer for the PR (paid!)

Closing, thanks!

[JmillsExpensify]

$250 approved for @eVoloshchak