alsoscotland / react-super-select

MIT License
95 stars 33 forks source link

ReactSuperSelect eats mouse scroll and keyboard events #77

Closed reintroducing closed 8 years ago

reintroducing commented 8 years ago

I found these two issues that I wanted to bring to your attention.

Firstly, when the dropdown is open and i'm scrolling through the page, as soon as my mouse scrolls into the dropdown the scrolling stops and I can no longer scroll. This seems problematic because picture a dropdown at the bottom of a page. As you scroll down all the way and open the dropdown, I assume you won't be able to access the dropdown contents that are out of view since you can't scroll down.

Second, if I have the dropdown open (and mouse isn't even in it) and I try to run some kind of keyboard event (let's just say a user command+r to reload the page which is what I was trying to do at some point), the keyboard events dont get fired and react-super-select just seems to swallow them up. For any keyboard events that don't have to do with accessibility of the open dropdown the events should just pass through so the user can use the keyboard as they'd like, even when the dropdown is open.

alsoscotland commented 8 years ago

@reintroducing I will look into this as well.

alsoscotland commented 8 years ago

I have a fix for he keyboard event issue.

As far as the scrolling is concerned. I made the behavior that way intentionally. otherwise if you scroll to the bottom of the results set, particularly in an infinitely paging result set, the page continues to scroll and it is not as usable.

alsoscotland commented 8 years ago

https://github.com/alsoscotland/react-super-select/pull/79

reintroducing commented 8 years ago

Thanks, perhaps an option to enable/disable this? That way if someone wants that functionality they can have it but if not then they can opt out?

alsoscotland commented 8 years ago

@reintroducing I was thinking something along the same lines. It is a heavy handed choice so disabling via prop might make sense. I will look into it. Thank you for all the great feedback by the way

reintroducing commented 8 years ago

Absolutely, and thanks for all your work on this, really appreciate it.

alsoscotland commented 8 years ago

no worries

alsoscotland commented 8 years ago

added forceDefaultBrowserScrolling prop in https://github.com/alsoscotland/react-super-select/pull/79/commits/77ab3464c9d26716f721d5af1b8b71b5fc67d940

alsoscotland commented 8 years ago

@reintroducing https://github.com/alsoscotland/react-super-select/commit/1ef30d53ec02aacba1bfd5941eb2fbfa9cfc9c20 your idea of using componentDidMount for the onClose/onOpen callbacks ended up being better. thanks!

reintroducing commented 8 years ago

Glad to hear it, thanks again!

alsoscotland commented 8 years ago

@reintroducing if you use these new props please let me know if you run into any issues

reintroducing commented 8 years ago

To be 100% honest I ended up using react-select in the end. If you want my reasoning I can certainly provide it but it boiled down to getting this done a few days ago :\