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

Feature to scroll the viewport to display the menu #632

Merged
merged 2 commits into from Dec 10, 2015

Conversation

Projects
None yet
4 participants
@azaharakis

Our project required the menu of an enabled dropdown to always be in view when the menu is activated. This PR introduces this feature. The viewport will only be shifted if the menu isn't in focus.

We discussed different implementations such as the dropdown menu displaying above the select box. This implementation required the least amount of change to the code.

The implementation also uses a prop to calculate how much space to place below the menu and the viewport base. We discussed whether or not this should be calculated dynamically from css rather then a prop (incorporating any margin-bottom thats applied to .Select-menu) as this is arguably a styling concern but this semantically might not make sense as margins don't apply to elements taken out of document flow.

If you are interested in this feature but would like some changes please let us know!

Alexander Zaharakis
Feature to scroll the viewport to display the menu
If a select box was activated near the bottom of the viewport, the
viewport will now shift the minimum amount to display the full contents
of the menu.
@JedWatson

This comment has been minimized.

Show comment
Hide comment
@JedWatson

JedWatson Nov 29, 2015

Owner

Looks good @azaharakis but I think it would be best if we make this optional, and enable the behaviour with a prop, maybe scrollMenuIntoView?

Owner

JedWatson commented Nov 29, 2015

Looks good @azaharakis but I think it would be best if we make this optional, and enable the behaviour with a prop, maybe scrollMenuIntoView?

Alexander Zaharakis
Add a flag to enable viewport shift feature
as well as adding the feature to the README
@JedWatson

This comment has been minimized.

Show comment
Hide comment
Owner

JedWatson commented Dec 10, 2015

Thanks @azaharakis!

JedWatson added a commit that referenced this pull request Dec 10, 2015

Merge pull request #632 from azaharakis/show-fullmenu-in-viewport
Feature to scroll the viewport to display the menu

@JedWatson JedWatson merged commit e8b6bd0 into JedWatson:master Dec 10, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+12.8%) to 94.211%
Details
@fritz-c

This comment has been minimized.

Show comment
Hide comment
@fritz-c

fritz-c Jan 14, 2016

This is a really handy feature, however it doesn't seem to work when the select element is inside a scrolling container within the page. Something like:
scroll
Perhaps offer an option to specify the parent to scroll?
(Related to #637)

fritz-c commented Jan 14, 2016

This is a really handy feature, however it doesn't seem to work when the select element is inside a scrolling container within the page. Something like:
scroll
Perhaps offer an option to specify the parent to scroll?
(Related to #637)

@xabikos

This comment has been minimized.

Show comment
Hide comment
@xabikos

xabikos Jan 21, 2016

The auto scroll could be a solution but as I mentioned in the issue I created using a library like tether is probably better choice as it makes sure the drop down menu is always shown in the correct place

xabikos commented Jan 21, 2016

The auto scroll could be a solution but as I mentioned in the issue I created using a library like tether is probably better choice as it makes sure the drop down menu is always shown in the correct place

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment