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

Position of select menu not based on scrollable parent #3822

Open
prateekgarcha opened this issue Oct 21, 2019 · 9 comments · May be fixed by #3830
Open

Position of select menu not based on scrollable parent #3822

prateekgarcha opened this issue Oct 21, 2019 · 9 comments · May be fixed by #3830
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet issue/has-pr Issue has a PR attached to it that may solve the issue issue/reviewed Issue has recently been reviewed (mid-2020) menu-bug Addresses menu positioning, scrolling, or general interactions

Comments

@prateekgarcha
Copy link

prateekgarcha commented Oct 21, 2019

As mentioned in PR 3531,

It does not solve all use cases, as there is still code that checks for available view height on window (viewSpaceBelow >= menuHeight), and not inside the container.

I'm facing the similar issue when the menu is present near the bottom of the view area inside a scrollable div, in my case a slide panel, the menu's positioning is being based on the window height and not the scrollable parent.
The following are the issues that I'm facing:-

  1. The menu is being truncated if there is some other container at the bottom of the page but the menu can fit inside the window and is being placed at the bottom.
  2. The scrollable div is not being scrolled to bring the menu into the view.
  3. If I use menuPosition = "fixed", I'm able to remove the truncation problem, but that brings another issue as the position of menu is not being changed if the parent div is scrolled up or down.

I'm working on a fix for the getMenuPlacement function inside Menu.js, and trying to change the calculations based on the scrollable parent instead of using the window every time. If there is no scrollable parent, then the default thing will be working as is.
If anyone else is working on this issue, do let me know.

@prateekgarcha
Copy link
Author

@JedWatson @mitchellhamilton do you guys have any plan of adding this functionality if I send a PR? I'm still working on a generic fix that works for a scrollable div as well as the default html element but it would not make sense if it won't be merge. Will be waiting for your reply.

@prateekgarcha prateekgarcha linked a pull request Oct 24, 2019 that will close this issue
@bzalasky
Copy link

This is pretty similar to the issue I described in 3810 (not quite the same though). We're using fixed menu positioning and configured the dropdown to close on scroll.

@prateekgarcha
Copy link
Author

@bzalasky The prob with "fixed" menu position is that, if we scroll the page, the position of menu remains fixed to where it was rendered first and the view doesn't look good with some weird behavior as well (try to hover the mouse over the menu after scrolling the page and then the menu will "try" to move to its desired position, but that also breaks in some cases).
Can you try the fix in #3830 and see if it helps your cause?

@bzalasky
Copy link

bzalasky commented Nov 8, 2019

The prob with "fixed" menu position is that, if we scroll the page, the position of menu remains fixed to where it was rendered first

This is why we opted to close the dropdown when the page is scrolled. It's not perfect, but solved our problem. We actually need fixed positioning in our case because of how the dropdown is rendered inside of a responsive table (to avoid clipping issues for rows near the bottom of the table).

I can take a look at your patch.

@pelmorex-yyamaguchi
Copy link

@bzalasky I guess if the scrolling is happening inside the menu, then this approach wouldn't work? If both inside/outside menu scroll hides the menu, users won't be able to select options within scrolled down menu.

@IanVS
Copy link
Contributor

IanVS commented Nov 22, 2019

Does #3669 solve your problem, @prateek0227 ?

@bladey bladey added the issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet label Jun 1, 2020
@bladey bladey added the issue/reviewed Issue has recently been reviewed (mid-2020) label Jun 18, 2020
@bladey
Copy link
Contributor

bladey commented Jun 23, 2020

Hi @prateek0227,

I wanted to check in with you and the above issue to see if @IanVS's PR suggestion would have solved your issue.

@bladey bladey added the awaiting-author-response Issues or PRs waiting for more information from the author label Jun 23, 2020
@mattaningram
Copy link

mattaningram commented Jun 23, 2020

@bladey I can't speak for @prateek0227 but it appears the PR would address this same issue I am having. I am not dealing with the scroll-to issue the PR also deals with, but the positioning not being based on nearest scrollable parent as @prateek0227 describes is causing me a lot of problems.

@prateekgarcha
Copy link
Author

My apologies @IanVS I guess I missed your previous comments.
@bladey I spent my whole day today trying to check if Ian's changes work for me but unfortunately they are not working and I'm able to reproduce the main issue. If the changes I made in packages/react-select/src/utils.js are also added to Ian's changes, then they seem to work a little bit but not for every case.
I have also made a tiny update to my PR and now every issue seems to fixed.

@bladey bladey removed the awaiting-author-response Issues or PRs waiting for more information from the author label Jun 24, 2020
@bladey bladey added the issue/has-pr Issue has a PR attached to it that may solve the issue label Jul 6, 2020
@ebonow ebonow added the menu-bug Addresses menu positioning, scrolling, or general interactions label Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet issue/has-pr Issue has a PR attached to it that may solve the issue issue/reviewed Issue has recently been reviewed (mid-2020) menu-bug Addresses menu positioning, scrolling, or general interactions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants