Auto-select the Expensify Card tab when first creating a workspace

[kevinksullivan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Join the freePlan beta
2. Create a New Workspace via the + icon in NewDot
3. Name the Workspace and arrive at the workspace menu with no tab auto-selected

Expected Result:

The Expensify Card tab of the Workspace should be Auto-selected when a user has just created one for the first time.

Actual Result:

No tab is selected when a user first sees the Workspace, and they arrive at the menu.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Notes/Photos/Videos:
Notice no tab is selected when first creating the workspace.

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/171670#

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @sonialiap (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[aman-atg]

Proposal

• This can be easily fixed by using constant strings inside menuItems
  

  App/src/pages/workspace/WorkspaceSidebar.js

  Line 54 in b08f80f

  isActive: Navigation.isActive(ROUTES.getWorkspaceCardRoute(policy.id)),

  
  We can change this line with isActive: Navigation.isWorkspaceActive(CONST.WORKSPACE.CARD)

• And a new isWorkspaceActive function can be use inside Navigation.js

function isWorkspaceActive(menuItemName) {
    const name = navigationRef.current.getCurrentRoute().name;
    return name === menuItemName;
}

• Constants

 WORKSPACE: {
       CARD: 'WorkspaceCard',
       PEOPLE: 'WorkspacePeople',
   },

• Suggestions are welcome as to where should these constants go and Proper naming for better readability and usability.

Maybe naming can be better

[rushatgabhane]

And a new isWorkspaceActive function can be use inside Navigation.js

@aman-atg That seems like a hack. Because Navigation.isActive() not working is the root cause.

[aman-atg]

@rushatgabhane IMO selecting sidebar items after parsing and comparing the routes is just an overkill for a simple task.

[rushatgabhane]

I mean, with your proposal we'll have to create a new constant and function every time we want to check if a route is active.
Does that make sense?

Anyway, check this out, getCurrentRoute().path is undefined the first time we visit the url.
(But gets defined on refresh/ clicking Expensify Card)

App/src/libs/Navigation/Navigation.js

Line 133 in b08f80f

const path = navigationRef.current && navigationRef.current.getCurrentRoute().path

[aman-atg]

new constant and function every time we want to check if a route is active

• I raised that concern by saying

Maybe naming can be better

• it could be isRouteName or something

Anyway, check this out, getCurrentRoute().path is undefined the first time we visit the url.
(But gets defined on refresh/ clicking Expensify Card)

And yeah I know that. What are ya trying to say here?
Also, just a point to note isActive isn't being used anywhere else

[mananjadhav]

While I feel this issue is an issue with react-navigation at the first glance.

** Proposal **

We can modify the isActive function that'll look something like this

function isActive(routePath) {
	let path = '';

	if (navigationRef.current && navigationRef.current.getCurrentRoute().path) {
		path = navigationRef.current.getCurrentRoute().path.substring(1)
	} else {
		let pathFromState = parsePathFromPathString(getPathFromState(navigationRef.current.getState()));
		if(pathFromState) {
			path = pathFromState;
		} 
	}

	return path === routePath;
}

function parsePathFromPathString(pathString) {

	// Parse path value from the following path string returned state
	// /WorkspaceSettings?initial=true&screen=WorkspacePeople&params=%5Bobject%20Object%5D&path=%2Fworkspace%2F51594D80CA931BBB%2Fpeopl
}

[rushatgabhane]

@mananjadhav We maintain currentURL as an ONYX key.

isActive() function can be a one-liner. What do you think?

[mananjadhav]

That’s the first thing that I checked and it works too.

I thought we would be using isActive later for internal screen navigation too. It it is going to be route based then Onyx works.

[mananjadhav]

Approach with Onyx:

Navigation.js

function isActive(routePath) {
  // We remove First forward slash from the URL before matching

  let currentUrl = getCurrentURL();
  const path =
    navigationRef.current && navigationRef.current.getCurrentRoute().path
      ? navigationRef.current.getCurrentRoute().path.substring(1)
      : currentUrl
      ? currentUrl.substring(1)
      : '';
  return path === routePath;
}

actions/App.js

let currentUrl;
Onyx.connect({
  key: ONYXKEYS.CURRENT_URL,
  callback: (val) => (currentUrl = val),
  initWithStoredValues: false,
});

function getCurrentURL() {
  return currentUrl;
}

While I tested and it worked fine. I am just concerned about one thing. Onyx uses a persistence layer with localStorage on the web. This is generally shared by the domain/application. So any updates on one open tab shouldn't affect the other.