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

Scroll Into View doesn't work for Element Click cases where element is bigger than viewport #1223

Closed
InstyleVII opened this issue Feb 6, 2018 · 2 comments

Comments

@InstyleVII
Copy link
Contributor

Sample gist here

Going by the spec's current writing Element Click would fail as all browsers when calling ScrollIntoView on the button would scroll to the bottom of the element (not the middle) and don't scroll horizontally whatsoever.

@andreastt
Copy link
Member

What do you mean by ‘fail’? The way I read the spec it isn’t meant to scroll at all in this case, because scroll into view is defined like this (my highlighting):

To scroll into view an element perform the following steps only if the element is not already in view:

In other words, if the in-view centre point of the element can be reached already there is no call to scroll it into view.

That said, I think the desired scrolling behaviour in cases where scrolling is necessary would be to try to reach the middle of the element. Last I checked the center option for ScrollIntoViewOptions given to Element.scrollIntoView() wasn’t widely supported.

We chose bottom as a compromise because it is more common for display: fixed elements such as menus to overlap the top. We currently don’t handle scrolling to elements hidden behind fixed elements at all, and I’m not sure there’s any way we can get around that. Of course, the middle of the element would be better if UAs supported it.

@InstyleVII
Copy link
Contributor Author

Ah I see I missed the in-view center point definition which actually accounts for viewport using innderWidth/innerHeight. We'll need to re-define that in our code. Agreed that scrolling to the center of the button is more preferable, and will test to see if we support center for scrollIntoView.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants