[HOLD for payment 2025-01-02] [$250] Desktop- The search filter shifts to the window border on scroll

[m-natarajan]

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: 9.0.71-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @s77rt
Slack conversation (hyperlinked to channel name): expensify_bugs

Action Performed:

1. Resize window to a small width
2. Go to Search
3. The search filter is not fully visible
4. Scroll down
5. The search filter shifts to the window border

Expected Result:

On step 3, the search filter should be fully visible. On step 5, the search filter shouldn't shift to the window border

Actual Result:

On step 3, the search filter is not fully visible. On step 5, the search filter shifts to the window border

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Screen.Recording.2024-12-04.at.5.01.28.PM.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021865070532106078592
• Upwork Job ID: 1865070532106078592
• Last Price Increase: 2024-12-13
• Automatic offers:
• gijoe0295 | Contributor | 105371761

Issue Owner

Current Issue Owner: @lschurr

[melvin-bot]

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[gijoe0295]

Edited by proposal-police: This proposal was edited at 2024-12-05 20:23:08 UTC.

Proposal

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

On step 3, the search filter is not fully visible. On step 5, the search filter shifts to the window border

What is the root cause of that problem?

SearchPageBottomTab is inside a BottomTabNavigator which is wrapped by a ScreenWrapper.

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

Line 22 in 1408a2d

NavigationContentWrapper: BottomTabNavigationContentWrapper,

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

Line 11 in 1408a2d

<ScreenWrapper

SearchPageBottomTab also has its own ScreenWrapper so we have 2 nested ScreenWrappers in this page.

The problem is from PR #53250 we added the HeaderGap to ScreenWrapper which adds a small top gutter for desktop app:

App/src/components/ScreenWrapper.tsx

Line 293 in aeac8bf

<HeaderGap styles={headerGapStyles} />

So we have duplicated HeaderGaps causing the top gutter gap to double (from 12 > 24px), and thus the SearchPageBottomTab is partially covered (by that redundant 12 px gutter) and when we scroll up, there's a huge gap above the top bar.

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

We should also apply the same fix here:

App/src/components/ScreenWrapper.tsx

Lines 153 to 154 in aeac8bf

	// since Modals are drawn in separate native view hierarchy we should always add paddings
	const ignoreInsetsConsumption = !useContext(ModalContext).default;

which checks if we're already in a ScreenWrapperStatusContext (is nested inside another ScreenWrapper) to avoid showing the HeaderGap.

const shouldAddHeaderGap = !useContext(ScreenWrapperStatusContext);
...
{shouldAddHeaderGap && <HeaderGap styles={headerGapStyles} />}

What alternative solutions did you explore? (Optional)

Or just simply introduce a new param shouldAddHeaderGap. It's true by default. Only set it to false to pages that are nested inside many (>1) ScreenWrappers.

[melvin-bot]

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

[melvin-bot]

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

[jayeshmangwani]

Thanks for the proposal @gijoe0295; it looks good.

However, we were already using HeaderGap to ScreenWrapper before this PR, so how is this issue arising now after the PR?
Additionally, why is it occurring on narrow desktop layouts but not on the wider ones?

[melvin-bot]

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

[jayeshmangwani]

Awaiting proposals here

[jayeshmangwani]

@gijoe0295 bump for this comment #53638 (comment)

[gijoe0295]

Edited by proposal-police: This proposal was edited at 2024-12-12 20:36:09 UTC.

Updated Proposal

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

1. SearchTypeMenu is overlapped by TopBar initially
2. SearchTypeMenu is visible above the TopBar while scrolling down

What is the root cause of that problem?

SearchPageBottomTab is wrapped by 2 ScreenWrappers which come with 2 HeaderGaps. The first one belongs to the parent BottomTabNavigator and the latter is inside the page itself.

1. We initialize the topBarOffset to be 80 which is the height of the TopBar:

App/src/pages/Search/SearchPageBottomTab.tsx

Line 38 in 9169920

const topBarOffset = useSharedValue<number>(variables.searchHeaderHeight);

But this initial offset does not account for the 12 gap of HeaderGap 2 causing SearchTypeMenu to be covered 12 px at its top.

2. We have the logic to hide the SearchTypeMenu behind the TopBar when scrolling the search list on small screens, by moving it up to -26 top offset; and set TopBar's zIndex to be higher.

App/src/pages/Search/SearchPageBottomTab.tsx

Line 53 in 9169920

topBarOffset.set(clamp(topBarOffset.get() - distanceScrolled, variables.minimalTopBarOffset, variables.searchHeaderHeight));

On web or native apps, this negative top offset will move SearchTypeMenu out of the viewport. But on desktop, since we have a small gap above the TopBar, part of the SearchTypeMenu will be visible there.

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

1. Create platform-specific searchHeaderHeight variable which accounts for the 12px header gap on desktop only.
2. SearchTypeMenu has zIndex: 9, so while scrolling, it will overlap HeaderGap 2:

App/src/pages/Search/SearchPageBottomTab.tsx

Lines 117 to 118 in 9169920

	<Animated.View style={[styles.searchTopBarStyle, topBarAnimatedStyle]}>
	<SearchTypeMenu

So we need to set headerGap style here to have zIndex: 10 and backgroundColor: theme.appBG to make sure that it:

• lies on top of
• cover the SearchTypeMenu.

[gijoe0295]

@jayeshmangwani

how is this issue arising now after the PR?

After careful investigation, I think this issue has existed for quite a long time before that PR. And we can not just remove the second HeaderGap because it will narrow down the necessary header gap on desktop.

why is it occurring on narrow desktop layouts but not on the wider ones

We have the logic to hide the SearchTypeMenu behind the TopBar when scrolling the search list on small screens. I explained this in the (2) point within RCA section in my updated proposal.

Please take your time to review my second proposal. Thank you!

[melvin-bot]

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

[jayeshmangwani]

Please take your time to review my second proposal. Thank you!

Interesting, @gijoe0295 , Is it possible for you to push your changes to a local branch? It’s not generally needed, but since it’s a UI issue, I want to make sure I can visually test different scenarios on desktop before we move forward.

[gijoe0295]

@jayeshmangwani Here's the test branch main...gijoe0295:App:gijoe/53638. Feel free to provide feedbacks.

[jayeshmangwani]

Thanks for the branch @gijoe0295,

I tested it on desktop, and it successfully resolves the bug at hand. We can proceed with @gijoe0295's Proposal of desktop platform specific searchHeaderHeight.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[pecanoro]

Sounds good! Assigning @gijoe0295 to the issue

[melvin-bot]

📣 @gijoe0295 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

@pecanoro @lschurr @jayeshmangwani @gijoe0295 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[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 9.0.78-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: Desktop - The search filter shifts to the window border on scroll #54359

If no regressions arise, payment will be issued on 2025-01-02. 🎊

For reference, here are some details about the assignees on this issue:

• @jayeshmangwani requires payment through NewDot Manual Requests
• @gijoe0295 requires payment automatic offer (Contributor)
• @s77rt requires payment through NewDot Manual Requests

[melvin-bot]

@jayeshmangwani @lschurr @jayeshmangwani The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[s77rt]

#53638 (comment)

I'm not due payment here

[lschurr]

Payment summary:

• Contributor: @gijoe0295 $250 (paid via Upwork)
• Reviewer: $250 @jayeshmangwani (please request via NewDot)

[garrettmknight]

$250 approved for @jayeshmangwani