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

Open React-Select does not follow "window" scroll #433

Closed
wwqrd opened this issue Mar 26, 2019 · 7 comments · Fixed by #662
Closed

Open React-Select does not follow "window" scroll #433

wwqrd opened this issue Mar 26, 2019 · 7 comments · Fixed by #662
Assignees
Labels
Architect Architect
Milestone

Comments

@wwqrd
Copy link
Contributor

wwqrd commented Mar 26, 2019

Can be seen when editing an EditableList item, such as a prompt.

Could be:

  • Intended behaviour
  • Issue with our implementation

If issue with our implementation we should update to fix.

Screenshot 2019-03-26 at 18 04 11

@jthrilly jthrilly added this to the Beta milestone Apr 5, 2019
@jthrilly jthrilly added the blocker blocker label Apr 8, 2019
@wwqrd
Copy link
Contributor Author

wwqrd commented Apr 26, 2019

I did a little bit of investigation with this:

  • The anchoring problem disappears if we remove: menuPortalTarget={document.body}, which positions the div relatively.
  • When the menu is positioned relatively, we run into zIndex issues - which I wasn't able to resolve.

One further area of investigation might be to piggyback the portal providing class used by modals, this would mean that we could insert the menus relative to the scrolling div.

@jthrilly
Copy link
Member

https://codesandbox.io/s/43n7z1v374?from-embed

This shows a potential approach.

@jthrilly jthrilly added the Architect Architect label May 29, 2019
@jthrilly
Copy link
Member

jthrilly commented Jun 3, 2019

I am removing this from the beta release for now, as there don't seem to be any easy fixes.

@jthrilly jthrilly removed the blocker blocker label Jun 3, 2019
@jthrilly jthrilly modified the milestones: Beta, Beta-stretch Jun 3, 2019
@jthrilly
Copy link
Member

Could perhaps use overscroll-behavior: contain?

https://codepen.io/aaroniker/pen/ZEzmzxj

@jthrilly jthrilly modified the milestones: Beta-stretch, stable Mar 23, 2020
@wwqrd
Copy link
Contributor Author

wwqrd commented Apr 3, 2020

React-select already does something similar to overscroll-behavior: contain I think?

Something similar which seems like it would solve the issue would be to use the menuShouldBlockScroll option. This disables scrolling outside the selector whilst it is open. However this feature is currently broken in react-select: JedWatson/react-select#3929 Suggest we wait until it's fixed there and try again.

@wwqrd
Copy link
Contributor Author

wwqrd commented Jun 19, 2020

The react select issue has moved to: JedWatson/react-select#4088

@jthrilly
Copy link
Member

One additional consideration here is window height. Native select elements are able to overlap window boundaries:

Screen Shot 2020-06-19 at 10 31 40 AM

This isn't ever going to be possible with the current approach as far as I can tell, which means we will always have issues where the list is longer than the remaining offset from the bottom of the select element. Perhaps we are making a headache for ourselves?

Two solutions:

  • Use a native <select> and abandon the "rich" element of this.
  • Design a different interaction for choosing an input control.

My view is we should implement the first one in the short term, to resolve this issue. We can then earnestly think about the second one, and revisit the intention of rich select items for choosing an input component. The original design was some sort of full screen "chooser" with a summary component.

Alternatively, we could go straight to number 2. Happy to hear what others think!

@wwqrd wwqrd self-assigned this Sep 7, 2020
@jthrilly jthrilly mentioned this issue Oct 16, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architect Architect
Development

Successfully merging a pull request may close this issue.

2 participants