Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.34k stars 2.77k forks source link

[$50] Saved search - Long list causes overlap with status bar on native, and unscrollable on mweb #49217

Closed IuliiaHerets closed 1 day ago

IuliiaHerets commented 3 days ago

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: 9.0.35-0 Reproducible in staging?: Y Reproducible in production?: N/A - new feature, doesn't exist in prod Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  1. Launch New Expensify app.
  2. Go to Search.
  3. Tap on the dropdown.
  4. Note that the selected Expenses tab overlaps with the status bar.
  5. Go to staging new dot mobile web.
  6. Go to Search.
  7. Tap on the dropdown.
  8. Note that the page cannot be scrolled on mobile web.

Expected Result:

In Step 4, the selected Expenses tab will not overlap with the status bar. In Step 8, the list is scrollable on mobile web.

Actual Result:

In Step 4, the selected Expenses tab overlaps with the status bar. In Step 8, the list is not scrollable on mobile web.

Workaround:

Unknown

Platforms:

Screenshots/Videos

Bug6603226_1726299984918!IMG_5007

https://github.com/user-attachments/assets/cfd6bea5-28dc-4e7a-9fc6-80db46a42bed

View all open jobs on GitHub

melvin-bot[bot] commented 3 days ago

Triggered auto assignment to @trjExpensify (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.

melvin-bot[bot] commented 3 days ago

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

github-actions[bot] commented 3 days ago

:wave: 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.
ishpaul777 commented 3 days ago

Edited by proposal-police: This proposal was edited at 2024-09-14 21:40:49 UTC.

Proposal

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

Saved search - Long list causes overlap with status bar on native, and unscrollable on mweb

What is the root cause of that problem?

In https://github.com/Expensify/App/commit/c04e7a1f2e4b0d04c935aafd8cf44c10b7a7ce0f, we added a scrollview in PopoverMenu to make it scrollable, but height for menu is not capped, so it can go beyond the screen height and overlap with status bar on native, and be unscrollable on mweb

https://github.com/Expensify/App/blob/0c618acc8b5a0b77eb27dfa049a18e3135c51c52/src/components/PopoverMenu.tsx#L259

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

we need to set a max height for the menu so it doesn't go beyond the screen height. (we'll add this in a new style), we'll also need to confirm the upper margin value from design team. for now, its 200 as a placeholder.

    const {windowHeight} = useWindowDimensions();

    <ScrollView style={isSmallScreenWidth ? {maxHeight: windowHeight - 200} : createMenuContainer}>
Mweb

https://github.com/user-attachments/assets/a67da04d-03ac-4c82-b60b-40072c585bed

ios Native

https://github.com/user-attachments/assets/bdc3fa5a-8c44-4540-8a29-91c317492616

luacmartins commented 2 days ago

@lakchote will address this in a follow up from his PR

luacmartins commented 2 days ago

Fixed on staging.

luacmartins commented 2 days ago

Reopening to compensate @ishpaul777 $50 since his solution was used to solve this issue.

melvin-bot[bot] commented 1 day ago

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

trjExpensify commented 1 day ago

Offer sent to Ishpaul for $50 for the proposal.

trjExpensify commented 1 day ago

Paid!