[PAID] [$500] Web - Persistence of Tax Selection When Navigating to Other Pages and Returning

[kbecciv]

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: v1.4.59-3
Reproducible in staging?: y
Reproducible in production?: y
Issue reported by: Applause - Internal Team

Action Performed:

, Go to Settings > Profile > Workspace.
2, Open workspace and access "More Features" to enable taxes.
3, Select taxes from the list.
4, Navigate to another section.
5, Return to the previous section.

Expected Result:

Taxes should be deselected automatically upon navigating away and back.

Actual Result:

Selected taxes persist even after navigating to another section and returning, causing inconsistency between sections

Workaround:

n/a

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

Bug6436622_1712142766433.Screen_Recording_2024-04-03_at_3.57.46_AM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0167f336d9308b4e85
• Upwork Job ID: 1775671638527598592
• Last Price Increase: 2024-04-03
• Automatic offers:
• s77rt | Reviewer | 0
• Krishna2323 | Contributor | 0

[melvin-bot]

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

[kbecciv]

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

[kbecciv]

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

[Krishna2323]

Proposal

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

Web - Persistence of Tax Selection When Navigating to Other Pages and Returning

What is the root cause of that problem?

We don't have any logic to clean the selected options when route changes. Also it does happen on almost every policy feature page including the members page. The component doesn't get completely removed so the states are preserved.

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

We should add a useEffect which will run on route change and clear all the selection. Or instead of using route name we can use useIsFocused and clear the selected option when screen is not focused.

    const isFocused = useIsFocused();

    useEffect(() => {
        if (isFocused) {
            return;
        }
        setSelectedCategories({});
    }, [isFocused]);

    useEffect(
        () => () => {
            setSelectedTaxesIDs([]);
        },
        [],
    );

If we use isFocused then we can also remove the part where we clear selected options when navigating to option settings.

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

Lines 142 to 145 in ebc8de8

	const navigateToCategorySettings = (category: PolicyOption) => {
	setSelectedCategories({});
	Navigation.navigate(ROUTES.WORKSPACE_CATEGORY_SETTINGS.getRoute(route.params.policyID, category.keyForList));
	};

Result

[Krishna2323]

Proposal updated

• Added details

[allgandalf]

Actually
It's regression from

#38849

That PR didn't touch the Taxes folder @ZhenjaHorbach :)

[ZhenjaHorbach]

That PR didn't touch the Taxes folder @ZhenjaHorbach :)

I deleted comment )

[allgandalf]

Proposal

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

Persistence of Tax Selection When Navigating to Other Pages and Returning
Selected Taxes still remain when we shift tabs.

What is the root cause of that problem?

OpenPolicyTaxesPage is not called everytime we navigate to page the taxes page. To compare with categories, we call OpenPolicyCategoriesPage everytime we open the categories page, this overwrites the locally selected values and we get data from the database.

Now OpenPolicyTaxesPage is not called because when we compare from where we take the route prop (it's onyx value) from PolicyRoute:

App/src/pages/workspace/withPolicy.tsx

Lines 100 to 102 in 255f841

	type WithPolicyProps = WithPolicyOnyxProps & {
	route: PolicyRoute;
	};

But for categories we take the value of route from it's onyx value:

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

Line 51 in 4fcd8a1

policyCategories: OnyxEntry<OnyxTypes.PolicyCategories>;

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

We need to update the type such that we should fetch value from TaxRates

App/src/types/onyx/Policy.ts

Line 61 in 255f841

type TaxRates = Record<string, TaxRate>;

Result Video:

simplescreenrecorder-2024-04-03_23.11.18.mp4

Alternative

Or we can define a onyxentry for poicy, this way we can avoid any possible regression, so we can define a new type and add it in existing WorkspaceTaxesPageProps:

type TaxesPageOnyxProps = {
    policy: OnyxEntry<Policy>;
};

I would prefer my alternative solution over main one :), as it would avoid any further possible regression

[melvin-bot]

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

[melvin-bot]

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

[s77rt]

@Krishna2323 Thanks for the proposal. Your RCA is correct. The solution looks good to me.

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

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

[s77rt]

@GandalfGwaihir Thanks for the proposal. Your RCA is not correct. Calling the API has no effect on the selected options and the bug affects the categories page as well

Screen.Recording.2024-04-04.at.2.44.10.AM.mov

[allgandalf]

Thanks for the feedback @s77rt , will improve proposal next time,

Actually during testing i found out that we clear the option only when we visit places where we have list like switch between distance rates and categories, at that time we can see that categories get reset, idk if it is by design or it’s actually another regression:)

Also, won’t the option be reset if we open enabled/disabled option when an option is selected?(for using isFocused as proposed )This will lose focus from screen right, so which will unselect unintentionally

[hayata-suenaga]

@Krishna2323's proposal looks good. One question, can we use useFocusEffect from react-navigation instead?

  useFocusEffect(() => {
    return () => setSelectedCategories([]);
  });

  useFocusEffect(() => {
    return () => setSelectedTaxesIDs([]);
  });

[Krishna2323]

@hayata-suenaga yes, we can use useFocusEffect.

[trjExpensify]

Hm, I don't think we ever intended for the bulk selection to be remembered, so this isn't a bug. If you navigate away from the categories page, I think it's fine that the values are deselected.

[mountiny]

I agree with Tom, this was not designed this way and I dont think we want to build logic to support this. The selection should remain if you navigate to the RHP on the table list page, but not if you go to different page in the central pane.

We should close this

[Krishna2323]

If you navigate away from the categories page, I think it's fine that the values are deselected.

@mountiny @trjExpensify, the issue here is that the values are not being deselected when moving to other pages and coming back. Additionally, it is inconsistent, sometimes it clears the selected options, and sometimes it does not. We also clear the selection when moving to the option settings page.

persistence_of_tags.mp4

[mountiny]

For the first one, that was intended actually, I have been wrong before. We intentionally deselect the options when you navigate to the detail of the category/ tag

For the inconsistency, I agree we should make this consisstent and the selection should be removed when you navigate away

[trjExpensify]

Okay, agreed, so I think we can use this issue to do this:

• If you bulk select values in the table and navigate away from the page, deselect them all
• If you bulk select values in the table, and then click an individual value on the page to open the RHP, deselect them all

On that second one @mountiny, I think we should deselect them because the user has taken an action on the page to do something else at that point. 👍

[Krishna2323]

Raising the PR today.

[Krishna2323]

@s77rt PR is ready for review, all recordings have been added except for android native, we are facing build issues on android.

[Krishna2323]

@strepanier03, PR was deployed to production on 22nd April, this is ready for the payments process.

[jayeshmangwani]

@Krishna2323 Changes caused a regression,

1. Go to Workspaces > Go to any collect workspace
2. Go to Categories > Select a Few Categories
3. Now Press on any of the category
4. Previously Selected Categories are unselected

bug-workspace.mov

[s77rt]

@jayeshmangwani That's the expected behaviour

[jayeshmangwani]

Oh, sorry for the confusion; I thought it was a bug that previously selected categories were gone; please ignore it if thats expected behaviour.

[Krishna2323]

@strepanier03, friendly bump for payments.

[strepanier03]

Sorry for missing this, the failed automation caused this to fall through the cracks.

[strepanier03]

@Krishna2323 @s77rt - The job in Upwork had expired so I needed to make a new one and hire you both again. I just sent offers and will check again in a few hours.

[Krishna2323]

@strepanier03, offer accepted 🙃

[s77rt]

Accepted from side too

[strepanier03]

Great, thank you both, finishing up now.

In the future, since you can't update the labels or the title, feel free to ping me earlier for payment issues that fail automation. Failed automation = black hole for issues, sorry about that.

[strepanier03]

Contracts paid and closed. Thanks again, and apologies for the long delay.

[uzairusman1425]

Cause of the issue:

The issue you are facing is because the default behaviour of stack navigation, when you select certain checkboxes on your “Taxes” screen it is stored in the state of the component and when you move to a different screen the stack navigation pushes the “Taxes” screen on to the navigation stack and the newer screen gets mounted and displayed, and what happens to the “Taxes” screen is that in never gets unmounted and is saved with it’s current state in the navigation stack. And when you move back to your “Taxes” screen it pops the “Taxes” screen back from the navigation stack and as it never got unmounted it displays again with it’s previous state and same checkboxes will still be selected. That’s just the default behaviour to the stack navigation.

Ideal solution of the issue:

The best solution in my opinion is that we should use “useIsFocused” hook from “react-navigation” what it does is that when the screen is in focus(being displayed) it returns a value of “true” which you can use in “useEffect” hook from “react” to check when the screen is popped from the navigation stack and comes into the focus and we can then update the state of the checkboxes to their initial state.

[melvin-bot]

📣 @uzairusman1425! 📣
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>

[s77rt]

@uzairusman1425 Thanks for your interest! This issue is not open for proposals as it's already fixed. Please checkout issues with Help Wanted label where you can post a proposal. You should also check the contributing guide (linked in the comment above)

[mariapeever]

Proposal

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

Selected taxes remain selected after navigating to other pages from the tax selection page.

What is the root cause of that problem?

The selected tax ids state variable is not reset on navigating away from the tax selection page.

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

Solution 1 (local)

Use a reset state variable (useState) set to true when navigating away from the tax selection page. Call a reset function to reset the selected Tax IDs if reset is set to true in a useEffect hook with the reset state as a dependency (WorkspaceTaxesPage).

Solution 2 (global)

Add a callback function to the navigate method of Navigate (if a callback is required for other components/pages too) to reset the required state variables on navigate.

[melvin-bot]

📣 @mariapeever! 📣
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>

[mariapeever]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/mariapeever

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[s77rt]

@mariapeever Please check my comment above