Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2024-09-21][$250] [Search v2.1] - Dropdown Menu Misalignment After Page Minimize and Maximize #46532

Closed
2 of 6 tasks
lanitochka17 opened this issue Jul 30, 2024 · 37 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 30, 2024

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.14-2
Reproducible in staging?: Y
Reproducible in production?: Y
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): abebemiherat@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. Navigate to Workspace Chat > Click Plus Sign > Submit Expense
  2. Add a merchant and submit it
  3. Go to Search > Minimize the page > Select checkbox next to expense > Click on Chevron (next to "x selected" button top right) > Dropdown appears
  4. Maximize the page and observe that the dropdown appears in the middle instead of aligning with the chevron

Expected Result:

When an expense is selected in search, minimized, and the chevron is clicked to open the dropdown, maximizing the page should align the dropdown with the chevron

Actual Result:

When an expense is selected in search, minimized, and the chevron is clicked to open the dropdown, maximizing the page causes the dropdown to appear in the middle instead of aligning with the chevron

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

Bug6557251_1722340339427.Screen_Recording_2024-07-30_at_4.29.53_AM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01697c80b209631150
  • Upwork Job ID: 1819349098199446682
  • Last Price Increase: 2024-08-23
  • Automatic offers:
    • situchan | Reviewer | 103654124
Issue OwnerCurrent Issue Owner: @sonialiap
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 30, 2024
Copy link

melvin-bot bot commented Jul 30, 2024

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

@lanitochka17
Copy link
Author

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

@abzokhattab
Copy link
Contributor

abzokhattab commented Jul 30, 2024

Unable to reproduce it on the latest main.

Screen.Recording.2024-08-02.at.7.57.08.AM.mov

@melvin-bot melvin-bot bot added the Overdue label Aug 1, 2024
@daledah
Copy link
Contributor

daledah commented Aug 2, 2024

Edited by proposal-police: This proposal was edited at 2024-08-13 03:17:25 UTC.

Proposal

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

The position of dropdown menu isn't updated after resize the screen

What is the root cause of that problem?

We calculate left and bottom property of Dropdown Menu from popoverAnchorPosition, which is updated here:

dropdownAnchor.current.measureInWindow((x, y, w, h) => {
setPopoverAnchorPosition({
horizontal: x + w,
vertical:
anchorAlignment.vertical === CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.TOP
? y + h + CONST.MODAL.POPOVER_MENU_PADDING // if vertical anchorAlignment is TOP, menu will open below the button and we need to add the height of button and padding
: y - CONST.MODAL.POPOVER_MENU_PADDING, // if it is BOTTOM, menu will open above the button so NO need to add height but DO subtract padding
});
});
}
}, [windowWidth, windowHeight, isMenuVisible, anchorAlignment.vertical]);

When windowWidth changes, we update the popover position based on the dropdownAnchor position (in this case, dropdownAnchor is the button). But when this useEffect is triggered, x and y params haven't been updated. It means the new value of x, y is updated after the useEffect is triggered. I took a deep dive into this and see that this bug is from measureInWindow (in react native).

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

We handled this bug before in some other places:

anchorPosition={shouldUseStyleUtilityForAnchorPosition ? styles.popoverMenuOffset(windowWidth) : popoverPosition}

App/src/styles/index.ts

Lines 3620 to 3624 in 420c2a1

popoverMenuOffset: (windowWidth: number) =>
({
...getPopOverVerticalOffset(180),
horizontal: windowWidth - 355,
} satisfies AnchorPosition),

In the above places, we are using another way to adjust the popover's position. We adjust the position of the popover relying on windowWidth directly. And I suggest we should do the same thing in this case

  1. Create a new style
popoverButtonDropdownMenuOffset: (windowWidth: number) =>
            ({
                ...getPopOverVerticalOffset(70),
                horizontal: windowWidth - 20,
            } satisfies AnchorPosition),
  1. To be safe, in ButtonWithDropdownMenu we only apply the style above in the case the current logic fails. We can create a new prop called shouldUseStyleUtilityForAnchorPosition like here and apply new style when this new prop is true:
anchorPosition={shouldUseStyleUtilityForAnchorPosition ? styles.popoverButtonDropdownMenuOffset(windowWidth) : popoverAnchorPosition}
  1. Pass shouldUseStyleUtilityForAnchorPosition to ButtonWithDropdownMenu in SearchPageHeader. We can check if this bug also happens on other places where we also use ButtonWithDropdownMenu and maybe apply the same solution by passing shouldUseStyleUtilityForAnchorPosition prop

What alternative solutions did you explore? (Optional)

We can pass this new style from SearchPageHeader directly via a new prop, and using this style prop in ButtonWithDropdownMenu.

Copy link
Contributor

github-actions bot commented Aug 2, 2024

true

@sonialiap sonialiap added the External Added to denote the issue can be worked on by a contributor label Aug 2, 2024
@melvin-bot melvin-bot bot changed the title Search - Dropdown Menu Misalignment After Page Minimize and Maximize [$250] Search - Dropdown Menu Misalignment After Page Minimize and Maximize Aug 2, 2024
Copy link

melvin-bot bot commented Aug 2, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 2, 2024
Copy link

melvin-bot bot commented Aug 2, 2024

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

Copy link

melvin-bot bot commented Aug 5, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Aug 5, 2024
@sonialiap
Copy link
Contributor

Reproducible on Chrome

image

@sonialiap
Copy link
Contributor

@situchan what do you think of the proposal #46532 (comment) ?

@situchan
Copy link
Contributor

situchan commented Aug 6, 2024

@daledah please update proposal based on latest codebase. ThreeDotsMenu no longer exists in HeaderView. (after #44025)

@melvin-bot melvin-bot bot removed the Overdue label Aug 6, 2024
Copy link

melvin-bot bot commented Aug 9, 2024

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

@situchan
Copy link
Contributor

Awaiting more proposals

@daledah
Copy link
Contributor

daledah commented Aug 13, 2024

@situchan Updated proposal

ThreeDotsMenu no longer exists in HeaderView

There is no impact on my proposal, I mention ThreeDotsMenu because this bug also occurs in ThreeDotsMenu.

@situchan
Copy link
Contributor

@daledah thanks. can you please share test branch?

Copy link

melvin-bot bot commented Aug 13, 2024

@sonialiap @situchan 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!

@daledah
Copy link
Contributor

daledah commented Aug 14, 2024

@situchan Sorry for the delay. Here's the test branch: https://github.com/daledah/App/tree/fix/46532

@sonialiap sonialiap removed the Bug Something is broken. Auto assigns a BugZero manager. label Aug 16, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Aug 26, 2024
@daledah
Copy link
Contributor

daledah commented Aug 26, 2024

@situchan This PR is ready to review.

@luacmartins luacmartins changed the title [$250] Search - Dropdown Menu Misalignment After Page Minimize and Maximize [$250] [Search v2.1] - Dropdown Menu Misalignment After Page Minimize and Maximize Sep 4, 2024
@RachCHopkins
Copy link
Contributor

@situchan any update here with your review?

@situchan
Copy link
Contributor

The PR is on hold for #48511 - #47987 (comment)

@luacmartins
Copy link
Contributor

#48862 is merged

@luacmartins luacmartins self-assigned this Sep 11, 2024
@RachCHopkins
Copy link
Contributor

@sonialiap you can have this one back too! 😛

@RachCHopkins RachCHopkins removed their assignment Sep 18, 2024
@luacmartins luacmartins changed the title [$250] [Search v2.1] - Dropdown Menu Misalignment After Page Minimize and Maximize [HOLD for payment 2024-09-21][$250] [Search v2.1] - Dropdown Menu Misalignment After Page Minimize and Maximize Sep 19, 2024
@luacmartins luacmartins added Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 and removed Reviewing Has a PR in review Weekly KSv2 labels Sep 19, 2024
@luacmartins luacmartins moved this from Polish to Done in [#whatsnext] #expense Sep 19, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 23, 2024
@sonialiap
Copy link
Contributor

Payment summary

@melvin-bot melvin-bot bot removed the Overdue label Sep 23, 2024
@sonialiap
Copy link
Contributor

waiting for @daledah to accept the offer in upwork 💰

@daledah
Copy link
Contributor

daledah commented Sep 25, 2024

@sonialiap Done that thanks

@melvin-bot melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels Sep 25, 2024
@sonialiap
Copy link
Contributor

All paid ✔️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Archived in project
Development

No branches or pull requests

8 participants