The hover effect animation is not functioning properly on the Bank Account in workspace settings

[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: 1.4.36-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Navigate to Settings > Workspace > Bank Account
2. Click on it; the modal opens on the right side
3. The hover effect animation should be centered on the Bank Account section

Expected Result:

The hover effect animation should be centered on the Bank Account section

Actual Result:

The hover effect animation is not centered on the Bank Account section

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

Bug6367956_1707149094676.BA.mp4

View all open jobs on GitHub

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

[melvin-bot]

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

[mountiny]

Taking over, related to ideal nav.

[esh-g]

Proposal

Please re-state the problem we are trying to fix

The hover effect animation is not functioning properly on the Bank Account in workspace settings

What is the root cause of the problem?

We are checking if the Menuitem is pressed based on whether the item route name is present in the route, but for bankAccount we are not passing any routeName

App/src/pages/workspace/WorkspaceInitialPage.js

Line 214 in 43d1576

focused={activeRoute && activeRoute.startsWith(item.routeName)}

What changes should be made to fix this?

1. We should pass routeName property for bankAccount item in WorkspaceInitialPage
   

   App/src/pages/workspace/WorkspaceInitialPage.js

   Lines 139 to 147 in 43d1576

   	{
   	translationKey: 'workspace.common.bankAccount',
   	icon: Expensicons.Bank,
   	action: () =>
   	policy.outputCurrency === CONST.CURRENCY.USD
   	? singleExecution(waitForNavigate(() => ReimbursementAccount.navigateToBankAccountRoute(policy.id, Navigation.getActiveRouteWithoutParams())))()
   	: setIsCurrencyModalOpen(true),
   	brickRoadIndicator: !_.isEmpty(props.reimbursementAccount.errors) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '',
   	},

   
   We should pass it as SCREENS.REIMBURSEMENT_ACCOUNT.

2. We are only getting the activeRoute for CentralPaneNavigator whereas the bank account page is in RHP. Therefore we should modify the BottomTabNavigator to pass both active routes for both RHP and CentralPane here:
   

   App/src/libs/Navigation/AppNavigator/Navigators/BottomTabNavigator.tsx

   Lines 21 to 24 in 43d1576

   	function BottomTabNavigator() {
   	const activeRoute = useNavigationState(getTopmostCentralPaneRoute);
   	return (
   	<ActiveRouteContext.Provider value={activeRoute?.name ?? ''}>

   
   This means the activeRoute will be changed from just string to:

type ActiveRouteType = {
  centralPaneActiveRoute: string;
  rightModalActiveRoute: string;
}

3. Instead of just checking for centralPane we can add the check for RHP here:
   

   App/src/pages/workspace/WorkspaceInitialPage.js

   Line 214 in 43d1576

   focused={activeRoute && activeRoute.startsWith(item.routeName)}

   
   by creating this function to check if focused and pass it to focused prop:

const isFocusedRoute = (routeName) => {
        if (activeRoute.centralPaneActiveRoute.startsWith(routeName) && activeRoute.rightModalActiveRoute === NAVIGATORS.CENTRAL_PANE_NAVIGATOR) {
            return true;
        } else if (activeRoute.rightModalActiveRoute.startsWith(routeName)) {
            return true;
        }
        return false;
    };

Result

Screen.Recording.2024-02-05.at.11.41.47.PM.mov

[mountiny]

@trjExpensify Curious for your opinion on this one, I feel like we should eventually update the bank account to have its own page in the main pane same as all the other settings pages.

If we would do that, this change would not be necessary

[trjExpensify]

Yeah, interesting one. We could move the Bank account page to the main pane like we have the others. Clicking the rows in the card open the modal in the RHP.

That said, with Simplified Collect: Workflows when we implement the new re-designed workspace editor, it'll actually be RHP again:

So perhaps the decision to do nothing was the right one for the time being. It's a release1 initiative, so we'd be living with this for a month or so. CC: @luacmartins @JmillsExpensify

[mountiny]

@trjExpensify alright I see we will not have any case when the Bank account would be in the LHP. then I can see us doing nothing here

[esh-g]

Does that mean the bank account page will remain in RHP or LHP? We also have the case where we have a flow from central navigator to the bank account page in RHP, so this can't be moved to central pane at least

Screen.Recording.2024-02-06.at.1.04.30.AM.mov

[mountiny]

That means there will be no Bank Account option in LHP

[trjExpensify]

We also have the case where we have a flow from central navigator to the bank account page in RHP, so this can't be moved to central pane at least

Nah, all of those weird redirects will go away with the redesigned workspace editor as those pages get replaced.

[trjExpensify]

@trjExpensify alright I see we will not have any case when the Bank account would be in the LHP.

Nah, there won't be a Bank account menu item in the LHN if that's the question. Further, the Connect bank account > push row in the main pane of Workflows will open the Connect bank account page in the RHP.

[mountiny]

making this monthly and will confirm once we update this flow

[mountiny]

This button will be removed from the LHP so I am going to close this issue