[HOLD for payment 2024-03-29] IOS - Workspace - Offline indicator in workspace page overlaps with navigation bar

[kbecciv]

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

Action Performed:

1. Launch New Expensify app.
2. Go to Profile > Workspaces.
3. Select any workspace.
4. Go offline.

Expected Result:

Offline indicator will not overlap with navigation bar.

Actual Result:

Offline indicator overlaps with navigation bar.

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

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @abekkala (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]

@abekkala 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

[allgandalf]

Proposal

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

What is the root cause of that problem?

We set style prop to have bottom padding :

App/src/pages/workspace/WorkspaceInitialPage.tsx

Lines 249 to 252 in 5770e23

	<ScreenWrapper
	testID={WorkspaceInitialPage.displayName}
	includeSafeAreaPaddingBottom={false}
	style={[styles.pb0]}

This in overturns the below style for bottom padding:

App/src/components/ScreenWrapper.tsx

Lines 224 to 226 in 5770e23

	if (includeSafeAreaPaddingBottom || (isOffline && shouldShowOfflineIndicator)) {
	paddingStyle.paddingBottom = paddingBottom;
	}

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

Remove the style={[styles.pb0]} prop as this sets the bottom padding to 0.

What alternative solutions did you explore? (Optional)

[rmm-fl]

Proposal

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

ios - Offline indicator overlaps with navigation bar.

What is the root cause of that problem?

ScreenWrapper has styles.pb0 here:

App/src/pages/workspace/WorkspaceInitialPage.tsx

Lines 249 to 252 in 5770e23

	<ScreenWrapper
	testID={WorkspaceInitialPage.displayName}
	includeSafeAreaPaddingBottom={false}
	style={[styles.pb0]}

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

Remove padding style and replace it with this:

     <ScreenWrapper
            testID={WorkspaceInitialPage.displayName}
            shouldEnablePickerAvoiding={false}
            shouldShowOfflineIndicatorInWideScreen
        >

Similar usage here:

App/src/pages/workspace/WorkspacesListPage.tsx

Lines 390 to 394 in 5770e23

	<ScreenWrapper
	shouldEnablePickerAvoiding={false}
	shouldShowOfflineIndicatorInWideScreen
	testID={WorkspacesListPage.displayName}
	>

Result

What alternative solutions did you explore? (Optional)

[allgandalf]

This bug is also visible for Collect Workspaces and my proposal fixes it for both the views :)

[allgandalf]

hello @rmm-fl , shouldShowOfflineIndicatorInWideScreen is not used to determine the style of the indicator, it is used as a boolean condition to show content, I think you must have confused yourself with the intention to use shouldShowOfflineIndicatorInWideScreen :)

App/src/components/ScreenWrapper.tsx

Line 264 in 5770e23

{!isSmallScreenWidth && shouldShowOfflineIndicatorInWideScreen && (

[mkhutornyi]

This is follow-up of ideal nav v2
cc: @kosmydel

[ShridharGoel]

Proposal

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

IOS - Workspace - Offline indicator in workspace page overlaps with navigation bar.

What is the root cause of that problem?

Padding is being set as 0.

App/src/pages/workspace/WorkspaceInitialPage.tsx

Lines 249 to 253 in 34fa987

	<ScreenWrapper
	testID={WorkspaceInitialPage.displayName}
	includeSafeAreaPaddingBottom={false}
	style={[styles.pb0]}
	>

This is happening only on iOS because we don't have this horizontal bottom bar on other platforms. If we use fullscreen mode in Android, then we might be able to see the same issue there as well.

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

We should add a condition which only uses 0 padding if user is online. If user is offline, then the style will be set to undefined and padding would be set inside ScreenWrapper.

<ScreenWrapper
    testID={WorkspaceInitialPage.displayName}
    includeSafeAreaPaddingBottom={false}
    style={!isOffline ? styles.pb0 : undefined}
>

Then, padding would be taken from here:

App/src/components/ScreenWrapper.tsx

Lines 223 to 226 in 34fa987

	// We always need the safe area padding bottom if we're showing the offline indicator since it is bottom-docked.
	if (includeSafeAreaPaddingBottom || (isOffline && shouldShowOfflineIndicator)) {
	paddingStyle.paddingBottom = paddingBottom;
	}

In the above condition {!isOffline ? styles.pb0 : undefined}, we don't need check for includeSafeAreaPaddingBottom because if it always false.
Also, we don't need check for shouldShowOfflineIndicator because if it always true.

[WojtekBoman]

@mkhutornyi Because it's follow-up to ideal-nav-v2, I can look into it :) As far as I know, removing style prop from the ScreenWrapper in WorkspaceInitialPage.tsx should solve this issue. I'll prepare a PR :)

[melvin-bot]

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

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.55-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Remove style from ScreenWrapper in WorkspaceInitialPage #38562

If no regressions arise, payment will be issued on 2024-03-29. 🎊

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@marcaaron] The PR that introduced the bug has been identified. Link to the PR:
• [@marcaaron] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@marcaaron] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@marcaaron] Determine if we should create a regression test for this bug.
• [@marcaaron] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[situchan]

Please assign me here

[abekkala]

PAYMENT SUMMARY FOR MAR 29

• Fix: @WojtekBoman SWM - no payment required
• PR Review: @situchan [$500, if no regressions] Offer

[abekkala]

@slafortune will be handling payment for me while I'm ooo

[slafortune]

Paid