[$250] mWeb – Chat – Workspace switcher is with blue border after login

[lanitochka17]

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

---

Version Number: 1.4.83-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: N/A
Email or phone of affected tester (no customers): applausetester+jp_e_category_1@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Log out
3. Log in
4. Observe Workspace switcher in the left top part

Expected Result:

Workspace switcher is without blue border after login

Actual Result:

Workspace switcher is with blue border after login

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

Bug6512187_1718296681964.Blue_b.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0140196526c3a485da
• Upwork Job ID: 1803111339129290564
• Last Price Increase: 2024-07-09

Issue Owner

Current Issue Owner: @hoangzinh

[melvin-bot]

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

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

[lanitochka17]

@tylerkaraszewski FYI 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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[tylerkaraszewski]

I can't reproduce this.

RPReplay_Final1718305035.mp4

[luacmartins]

This is likely coming from #39520. It's really minor though

[luacmartins]

@lanitochka17 can you ask for a re-test? In the meantime, I'll demote this to NAB since it doesn't break any features.

[tsa321]

Proposal

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

Workspace switcher is with blue border after login

What is the root cause of that problem?

When the route is Home:

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

Lines 41 to 43 in 4ae133d

	<Tab.Screen
	name={SCREENS.HOME}
	component={SidebarScreen}

the SidebarScreen is displayed. The SidebarScreen is a component of Tab.Screen with the name SCREENS.HOME in bottomTabNavigator. The Tab is inside the createCustomBottomTabNavigator function:

App/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/index.tsx

Lines 49 to 54 in 4ae133d

	<ScreenWrapper
	testID={CustomBottomTabNavigator.displayName}
	shouldShowOfflineIndicator={false}
	shouldEnableKeyboardAvoidingView={false}
	shouldEnablePickerAvoiding={false}
	>

which includes ScreenWrapper.

So, when the route is Home, there are two screens: one from createCustomBottomTabNavigator and one from SidebarScreen, which is the component of the Tab.Screen for Home.

The focus trap is attached to ScreenWrapper, resulting in two focus traps. To disable the initial focus on the Workspace switcher, we must disable the initial focus for both of these screens.

The focus trap for Home is already disabled here:

App/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx

Lines 22 to 25 in 4ae133d

	const isActive = useMemo(() => {
	// Focus trap can't be active on bottom tab screens because it would block access to the tab bar.
	if (BOTTOM_TAB_SCREENS.find((screen) => screen === route.name)) {
	return false;

However, the trapFocus for BottomTabNavigator is not yet disabled.

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

We can include NAVIGATORS.BOTTOM_TAB_NAVIGATOR in BOTTOM_TAB_SCREENS

Alternative solution (1):

We can add NAVIGATORS.BOTTOM_TAB_NAVIGATOR in SCREENS_WITH_AUTOFOCUS:

App/src/components/FocusTrap/SCREENS_WITH_AUTOFOCUS.ts

Line 4 in 633e708

const SCREENS_WITH_AUTOFOCUS: string[] = [

So it won't autofocus on Workspace switcher after login

Alternative solution (2):

alternatively we can disable focusTrap on small screen:

App/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx

Lines 42 to 44 in 2a641d9

	return (
	<FocusTrap
	active={isActive}

Above that line, add this code to return children, the code could be:

if (isSmallScreenWidth) {return children;} Or similar cleaner and more efficient code.

[lanitochka17]

Issue is still reproducible on the mWeb/Chrome

Screenrecorder-2024-06-14-17-16-41-363.mp4

[melvin-bot]

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

[tylerkaraszewski]

I am going to mark this external and get a C+ for it.

[melvin-bot]

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

[hoangzinh]

ah right, sorry, but NAVIGATORS.BOTTOM_TAB_NAVIGATOR also doesn't have any auto-focus element, does it? I mean it does not make sense to put it into SCREENS_WITH_AUTOFOCUS.

[tsa321]

It's exclusively, in case if we want focusTrap on other screens elsewhere

@hoangzinh Are there any concerns about completely disabling the focus trap for users with small screens for all SCREENS, considering their keyboards lack a tab key?

[hoangzinh]

Focus-trap is not officially tested on any mobile browsers or devices.

According to focus trap doc, I think we can disable on mweb, also because it doesn't have tab key. But we might have a reason why we haven't disabled it on mWeb since the beginning, we need to clarify it in Slack. Before we do it, do you have any other solutions that can solve this issue?

[melvin-bot]

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

[melvin-bot]

@tylerkaraszewski, @hoangzinh Huh... This is 4 days overdue. Who can take care of this?

[tylerkaraszewski]

Still no good proposals for this I guess.

[tylerkaraszewski]

It's not overdue, it's been commented on.

[hoangzinh]

Edit: it is not SCREEN HOME but it is NAVIGATORS.BOTTOM_TAB_NAVIGATOR

@tsa321 sorry I just read your proposal carefully again. Can you explain why it's NAVIGATORS.BOTTOM_TAB_NAVIGATOR but not SCREENS.HOME? Because the screen we're fixing is SCREENS.HOME. Thank you.

[melvin-bot]

@tylerkaraszewski, @hoangzinh Huh... This is 4 days overdue. Who can take care of this?

[tsa321]

@hoangzinh because the WorkspaceSwitcherButton is in here:

App/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/TopBar.tsx

Line 46 in 15e67a0

<WorkspaceSwitcherButton policy={policy} />

in the BottomTabNavigator region. I have tried screen home and it doesn't work.

[hoangzinh]

because the WorkspaceSwitcherButton is in here in the BottomTabNavigator region

Right, but eventually, it's rendered in the SCREENS.HOME, isn't it?

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

Lines 27 to 30 in 8547850

	<Tab.Screen
	name={SCREENS.HOME}
	component={SidebarScreen}
	/>

I have tried screen home and it doesn't work.

If it's true, we need to find out why SCREENS.HOME doesn't work. To ensure we're fixing the root cause.

[melvin-bot]

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

[tsa321]

@hoangzinh the initialRouteName is NAVIGATORS.BOTTOM_TAB_NAVIGATOR as shown in the linkingConfig:

App/src/libs/Navigation/linkingConfig/config.ts

Line 10 in c973e62

initialRouteName: NAVIGATORS.BOTTOM_TAB_NAVIGATOR,

[hoangzinh]

It's correct. Sorry, but @tsa321, can you elaborate on which question/concern is answered by above? Thank you

[tsa321]

@hoangzinh Essentially, I am console logging the route name in the focus trap for the screen. The route names displayed are "navigators bottom tab" and "screen home." However, "screen home" doesn't prevent initial focus on that page. Therefore, the correct route name should be "navigators bottom tab."

[hoangzinh]

The route names displayed are "navigators bottom tab" and "screen home."

Nice explore @tsa321. Is it a bug or behavior of navigation? Because we're on the "screen home", why do we have "navigators bottom tab" in the console log?

[tsa321]

@hoangzinh, no, it's not a bug with navigation.

If the route is home:

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

Lines 41 to 43 in 4ae133d

	<Tab.Screen
	name={SCREENS.HOME}
	component={SidebarScreen}

the SidebarScreen is displayed. The SidebarScreen is a component of Tab.Screen with the name SCREENS.HOME in bottomTabNavigator. The Tab is inside the createCustomBottomTabNavigator function:

App/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/index.tsx

Lines 49 to 54 in 4ae133d

	<ScreenWrapper
	testID={CustomBottomTabNavigator.displayName}
	shouldShowOfflineIndicator={false}
	shouldEnableKeyboardAvoidingView={false}
	shouldEnablePickerAvoiding={false}
	>

which includes ScreenWrapper.

So, when the route is Home, there are two screens: one from createCustomBottomTabNavigator and one from SidebarScreen, which is the component of the Tab.Screen for Home.

The focus trap is attached to ScreenWrapper, resulting in two focus traps. To disable the initial focus on the Workspace switcher, we must disable the initial focus for both of these screens.

The focus trap for Home is already disabled here:

App/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx

Lines 22 to 25 in 4ae133d

	const isActive = useMemo(() => {
	// Focus trap can't be active on bottom tab screens because it would block access to the tab bar.
	if (BOTTOM_TAB_SCREENS.find((screen) => screen === route.name)) {
	return false;

However, the trapFocus for BottomTabNavigator is not yet disabled.

Therefore, the solution is the same as my proposal or you could include the BottomTabNavigator inside BOTTOM_TAB_SCREENS.

[hoangzinh]

There is a PR #44932 that would remove SCREENS_WITH_AUTOFOCUS list, therefore I think we can include the BottomTabNavigator inside BOTTOM_TAB_SCREENS, as you said, what do you think?

[tsa321]

@hoangzinh yes, we can do that.

[hoangzinh]

Cool, can you update your proposal (alternative solutions or something) so internal engineer can review. Especially the root cause, please

[tsa321]

Proposal

updated

@hoangzinh I have updated my proposal.

[hoangzinh]

Thanks @tsa321. I think we can go with @tsa321's proposal #43713 (comment), with the main solution

🎀👀🎀 C+ reviewed

[melvin-bot]

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