[$250] Workspace- Device backspace and system back button have different function for Category/Tag

[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.77-3
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: 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): [email protected]
Issue reported by: Applause - Internal Team
Component: Workspace Settings

Action Performed:

1. Navigate to https://staging.new.expensify.com/
2. Log in to an any account
3. Navigate to WS (Create one if not any)
4. Navigate to Category /Tag list then select some Categories or Tags
5. Click system back button at the top left and notice the selection is dismissed
6. Select some categories/ tags again
7. Click the device back button and notice it navigate back to WS settings

Expected Result:

Device backspace and system back button have the same function

Actual Result:

Device backspace and system back button have different function for Category/Tag

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

Bug6699078_1734623400585.Screen_Recording_20241219_182636_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021873799168918490516
• Upwork Job ID: 1873799168918490516
• Last Price Increase: 2025-01-27
• Automatic offers:
• layacat | Contributor | 105922187

Issue Owner

Current Issue Owner: @hungvu193

[melvin-bot]

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

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-12-19 19:38:28 UTC.

Proposal

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

Workspace- Device backspace and system back button have different function for Category/Tag

What is the root cause of that problem?

This is because on pressing back button we only dismiss the selection mode instead of navigating back when the mobile selection mode is on here

App/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx

Lines 363 to 369 in 582c6f1

	onBackButtonPress={() => {
	if (selectionMode?.isEnabled) {
	setSelectedCategories({});
	turnOffMobileSelectionMode();
	return;
	}
	Navigation.goBack(backTo);

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

If we want to be consistent we can go back in all cases so change it to

onBackButtonPress={() => {
                        Navigation.goBack(backTo);
                    }}

Do the same for tag settings, report field, Distance Rate page, workspace perdiem page, taxes page.
We also have similar pattern in WorkspaceViewTagsPage, ReportFieldsListValuesPage, WorkspaceMembersPage, RoomMembersPage, ReportParticipantsPage, we can consider them too

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can add a test for the category and tag settings page by turning on mobile selection mode and asserting on back button press we are navigated back.

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[alexpensify]

There is no update yet; still on my list.

[alexpensify]

Still on my testing list

[melvin-bot]

@alexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

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

[melvin-bot]

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

[alexpensify]

@hungvu193 when you get a chance, can you review if this proposal will fix the issue? Thanks!

---

Heads up, I will be offline until Friday, January 3, 2025, and will not actively watch over this GitHub during that period.

If this GitHub requires an urgent update, please ask for help in the #expensify-open-source Slack Room. If the inquiry can wait, I'll review it when I return online.

[melvin-bot]

📣 @VektorTech! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[hungvu193]

Hi @VektorTech, please follow our proposal template here

[hungvu193]

@FitseTLT I believe we don't wanna remove that feature

If we want to be consistent we can go back in all cases so change it to

[layacat]

Proposal

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

Device backspace and system back button have different function for Category/Tag

What is the root cause of that problem?

On some android devices we still have the physical back button, and we didn't handle that case, which caused this issue:

App/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx

Lines 374 to 381 in 754817d

	onBackButtonPress={() => {
	if (selectionMode?.isEnabled) {
	setSelectedCategories({});
	turnOffMobileSelectionMode();
	return;
	}
	Navigation.goBack(backTo);
	}}

There are many other places in our app that we're using the same this technique. That's the root cause of this issue.

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

Add a hardwareBackPress listener to handle such cases for physical back button, below is the pseudocode:

    useFocusEffect(
        useCallback(() => {
            const onBackPress = () => {
                if (selectionMode?.isEnabled) {
                    setSelectedTags({});
                    turnOffMobileSelectionMode();
                    return true;
                }
                Navigation.goBack(backTo);
                return false;
            };
            BackHandler.addEventListener('hardwareBackPress', onBackPress);
        }, [backTo])
    );

We can also convert the above effect into a hook and reuse it everywhere else inside our App.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

N/A

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.

[hungvu193]

Thanks @layacat, I don't think hardwareBackPress supports mWeb.

[layacat]

I think this issue also occurs on Android and we should fix it. On mWeb, this is default web behavior, we can't prevent the popstate because it was handle by the browser.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[alexpensify]

I started a chat here in Slack:

https://expensify.slack.com/archives/C01GTK53T8Q/p1737687275614879

[hungvu193]

Thank you 🚀

[melvin-bot]

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

[hungvu193]

Not overdue. I think let's fix Android native. Wdyt?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[alexpensify]

Yeah, let's move forward with that plan! Will one of these proposals fix the issue on Android only? Thanks for checking!

[hungvu193]

Yes. Let's go with @layacat 's proposal

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[grgia]

Confirming with android users the expected behavior - https://expensify.slack.com/archives/C01GTK53T8Q/p1738148452178419?thread_ts=1737687275.614879&cid=C01GTK53T8Q

[melvin-bot]

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

I'll prepare an PR soon

[layacat]

Running into eslint error with changed files 😢. It will take a little bit more time

[layacat]

hey guys, do we need to address lint changed below?

Namespace imports from @libs are not allowed. Use named imports instead. Example: import { method } from "@libs/module"                no-restricted-syntax

There are so many of them and it doesn't relate to my current PR logic

[hungvu193]

hey guys, do we need to address lint changed below?

Namespace imports from @libs are not allowed. Use named imports instead. Example: import { method } from "@libs/module" no-restricted-syntax
There are so many of them and it doesn't relate to my current PR logic

Unfortunately, we can't merge the PR without addressing all the lints, we need to align with the rules, please address all the lints. Ty 🙇