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.

Sign up for GitHub

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

Jump to bottom

support tests may cause page to scroll #60

Closed
rodneyrehm opened this issue Nov 20, 2015 · 2 comments
Closed

support tests may cause page to scroll #60

rodneyrehm opened this issue Nov 20, 2015 · 2 comments
Labels
bug focus
Milestone
1.1.0 - second wave

Comments

@rodneyrehm
Copy link
Member

When ally.js is loaded for the first time (or does not have a cache yet), it runs a batch of supports tests to identify browser compatibility. It caches the results in localStorage so it doesn't have to re-run the tests over and over again. That's why this issue hasn't cropped up before.

As part of the tests a few DOM elements are created, injected into the document and focused. Of course they're removed and the previously focussed element becomes the activeElement again. But those focus() calls make the browser scroll the element into view.

That means a first-time visitor may end up at the end of the page without having scrolled there. Or in allyjs.io's case, the JSBin iframe containing the example is scrolled into view.

We can solve the scroll position of the document ally.js is loaded in, but I don't see how that would (easily, seamlessly) work with cross-origin iframes.

Anyway, saving the scrollPosition when saving the currently focused element and restoring to that position after the test should do the trick. Since we're appending to <body> we should not need to care about nested scrollable containers.
@ginader
Copy link

ginader commented Nov 20, 2015

👍

@rodneyrehm rodneyrehm modified the milestone: 1.1.0 - second wave Nov 30, 2015
@rodneyrehm rodneyrehm mentioned this issue Dec 3, 2015
Closed
@rodneyrehm
Copy link
Member Author

while touching this component we should also transform all tests to return a function instead of the test result. this allows running the tests when they're actually needed, not when the ally.js source is loaded (identified by #85)

rodneyrehm added a commit that referenced this issue Dec 6, 2015
@rodneyrehm
fix(supports): restore scroll position after tests - #60
fa9a740
rodneyrehm added a commit that referenced this issue Dec 6, 2015
@rodneyrehm
refactor(supports): return callbacks instead of values for lazy evalu…
33f75e0 
…ation - #60
@rodneyrehm rodneyrehm mentioned this issue Dec 6, 2015
Merged
rodneyrehm added a commit that referenced this issue Dec 12, 2015
@rodneyrehm
fix(supports): restore scroll position after tests - #60
7a0c9a2
rodneyrehm added a commit that referenced this issue Dec 12, 2015
@rodneyrehm
refactor(supports): return callbacks instead of values for lazy evalu…
af16bc4 
…ation - #60
rodneyrehm added a commit that referenced this issue Dec 12, 2015
@rodneyrehm
docs(CHANGELOG): adding notes on #68, #60
22d452f
@rodneyrehm rodneyrehm removed the ready label Dec 12, 2015
rodneyrehm added a commit that referenced this issue Jan 28, 2016
@rodneyrehm
fix(supports): reset scroll position of <body> element - #60
c32930c
rodneyrehm added a commit that referenced this issue Feb 9, 2016
@rodneyrehm
fix(supports): detect focus support off-screen - #60
42677a6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug focus
Projects
None yet
Milestone
1.1.0 - second wave
Development

No branches or pull requests
2 participants
@ginader @rodneyrehm