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

[AutoComplete] DropDown Positioning #4393

Closed
alexprice1 opened this issue May 31, 2016 · 19 comments
Closed

[AutoComplete] DropDown Positioning #4393

alexprice1 opened this issue May 31, 2016 · 19 comments
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module!

Comments

@alexprice1
Copy link

Problem description

Autocomplete has 4 major usability problems:

  • Only "opens" down (if you are really far down the page, cannot see any results)
  • Results are not scrollable
  • The dropdown does not stay fixed to the element if you scroll from an outer-container
  • When you click into the search box, no results come up (if there is searchText, I think results should come up)

Steps to reproduce

Place an autocomplete within a scrollable div where it does not have enough height to render all results.

Versions

  • Material-UI: 0.15.0
  • React: 15.0.2
  • Browser: Chrome

autocomplete

@dominicxcliu
Copy link

dominicxcliu commented Jun 6, 2016

You can actually restrict the menu height and make the results scrollable by using the menuStyle property, like this:

menuStyle = {{maxHeight: '300px'}}

Also, setting openOnFocus makes the full list of results come up when you just click into the field.

openOnFocus = {true}

But yeah, +1 for the other issues...

@liesislukas
Copy link
Contributor

having param something like 'position' would help. E.g. right now i need to figure how to push all the list left from input, i have small space on right and list is 600px in width.

  • Results are not scrollable
    ...
    menuProps={{maxHeight: 400}}
    maxSearchResults={300}
    ...
  • The dropdown does not stay fixed to the element if you scroll from an outer-container
    seems like fixed, i use Material-UI: 0.15.1
  • When you click into the search box, no results come up (if there is searchText, I think results should come up)
    works fine with even in v0.15.0 i didn't noticed this issue
    ...
    openOnFocus = {true}
    ...

@mpontikes mpontikes mentioned this issue Aug 5, 2016
13 tasks
@fmilani
Copy link

fmilani commented Sep 6, 2016

Only "opens" down (if you are really far down the page, cannot see any results)

I managed to make autocomplete to "open up" by using:

anchorOrigin={{ vertical: 'top', horizontal: 'left' }}

and

targetOrigin={{ vertical: 'bottom', horizontal: 'left' }}

But I couldn't figured out exactly why this worked (setting targetOrigin to "bottom" didn't make sense to me).

Besides, when the first letter is typed, the menu opens down. Only after I type the second letter it opens up. Does anybody know why is that?

@patotoma
Copy link

+1 still an issue

@snackycracky
Copy link

this is ridiculous. It's not a bug, it's actually a feature which is essential for me to release such a component.

@liamcmitchell
Copy link

To allow scrolling of the menu when it goes off the bottom of the screen:

popoverProps={{
  style: {
    bottom: 0,
    overflowY: 'auto'
  }
}}

@NetworkAndSoftware
Copy link

liamcmitchell, that works great when you have a large number of items in the list, but awful if your list is small: the dropdown will always extend to the bottom.

@NetworkAndSoftware
Copy link

This bug is compounded when the AutoComplete is used in a Dialog, where the page can not scroll and the items that are outside the current window are entirely unreachable.

Perhaps the way to fix this is to modify the PopOver component to have a prop that allows css style max-height of its first child div to window.innerHeight - the divs .top

@subk
Copy link

subk commented May 10, 2017

Looks like Popover option canAutoPosition which is true by default is set to false by AutoComplete.
See https://github.com/callemall/material-ui/blob/master/src/AutoComplete/AutoComplete.js#L541

This should do the trick :

popoverProps={{
  canAutoPosition: true
}}

@MayurKasar321
Copy link

when i open modal my page is auto scroll up how can i solve this problem

@lechup
Copy link

lechup commented Jul 3, 2017

Hm... it looks like when AutoComplete is scrolled inside body tag everything is working ok (like on demo/docs page), when You want to have it scrolled inside any other container it does not - it stays in the same place (fixed position calculated from body tag).

Here is jsfiddle using v0.18.5 showing the problem https://jsfiddle.net/k52fdwm5/14/

Any clues how it should be handled?

My idea is to change default placement of Popover in DOM, as a last child of parent of anchorEl.

@lechup
Copy link

lechup commented Jul 4, 2017

Hm... After some debugging I've end up with propagating scroll event to window from container that is scrolled:
https://jsfiddle.net/lechup/k52fdwm5/18/

@oliviertassinari do You have any idea how to do it properly inside AutoComplete?

My first idea would be scrollWrapperRef or scrollWrapperEl prop + attach event listener propagating scroll event.

PS: Code should be IE 9.0+ compatible: http://caniuse.com/#search=event

@slavab89
Copy link
Contributor

slavab89 commented Jul 6, 2017

The issue is probably with Popover itself and not with auto-complete and such..
Also, i like the workaround of propagating the scroll event but couldnt it cause like a double scroll event because onScroll you trigger another scroll?
I think this partially happens because of this where an event listener is created for the window itself, so if scrolling happens on something more deep inside the DOM tree, it wont trigger that event (not 100% its the case though).

On v1 branch, the popover is internal and seems to be using a Modal currently therefore the screen cant be scrolled when it opens - An example for it is the component demo of Menu.
So i think this issue wasnt really tackled for v1.
There is #5937 which should cover it

@lechup
Copy link

lechup commented Jul 6, 2017

@slavab89 window.attachListener("scroll", () => {}) won't get any scroll events from any node inside - scroll events are not propagated (bubbled up). So it is working when we scroll body but does not when we scroll anything inside body.

My workaround just fire new scroll event on window, I'm not sure whether it could scroll window/body somehow, on chrome it looks like it does not - probably beacuse I do not set any variables connected with scrollY, scrollX on the Event()?

@slavab89
Copy link
Contributor

slavab89 commented Jul 7, 2017

Not sure as well. In any case i am too using your workaround for now. To make it a bit better i've wrapped the onScroll callback in a throttle from lodash so that it will fire every 50/100 milis instead of every actual scroll even (Same thing is done in the Popover itself actually)

@gorkemcnr
Copy link

Basically, the problem is in Popover.js which uses the EventListener and target has been set as 'window'. So, if a prop is provided to set the target of EventListener (it could be an object or string like id of element), it will work.

But, there is another problem which is in setPlacement method of Popover.js. While setting the top position of target element, it uses Math.max to get the positive values only as targetEl.style.top = "${Math.max(0, targetPosition.top)}px";

If we changed that part as targetEl.style.top = "${targetPosition.top}px"; it will work as expected.

@lechup
Copy link

lechup commented Jul 18, 2017

I think we need someone from the core to shed some light what is current status of autocomplete - for now it is not available in 1.0 branch...

@gorkemcnr Your idea is basically what I've proposed already:

My first idea would be scrollWrapperRef or scrollWrapperEl prop + attach event listener propagating scroll event.

If You have some time You can try to prepare PR for this...

@oliviertassinari any update on this?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 18, 2017

@lechup The progress on the 1.0 branch is tracked on this issue #4783. Long story short, I think we would better off by not porting the component. Instead, documenting how to use external libraries like react-autosuggest or providing an adapter. That goes into the strategy of having the v1 less opinionated and closer to our core business. The 50+ issues show that building such component is hard.

gorkemcnr pushed a commit to gorkemcnr/material-ui that referenced this issue Jul 19, 2017
- As discussed on mui#4393 and mui#4783 I have provided a fix for popover position issue on autocomplete.
@oliviertassinari
Copy link
Member

Closed by #4783

oliviertassinari pushed a commit that referenced this issue Jul 21, 2017
* #4783
- As discussed on #4393 and #4783 I have provided a fix for popover position issue on autocomplete.

* Update Popover.js

* -Commented PR changes are applied
-Prop name is changed to a more meaningful name

* Update Popover.js

* Prop order is changed as alphabetical order

* redundant space removed

* redundant space is removed
@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Apr 29, 2020
chowdream added a commit to chowdream/Material-UI that referenced this issue Aug 17, 2023
* #4783
- As discussed on mui/material-ui#4393 and mui/material-ui#4783 I have provided a fix for popover position issue on autocomplete.

* Update Popover.js

* -Commented PR changes are applied
-Prop name is changed to a more meaningful name

* Update Popover.js

* Prop order is changed as alphabetical order

* redundant space removed

* redundant space is removed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests