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

Fix scrolling on some browsers #510

Closed
wants to merge 1 commit into from
Closed

Fix scrolling on some browsers #510

wants to merge 1 commit into from

Conversation

Kestrer
Copy link
Contributor

@Kestrer Kestrer commented Dec 7, 2019

By making the body of rustdoc position: relative instead of fixed, scrolling on many browsers (like qutebrowser and iOS Safari) works like it should. This change has no impact on how the page looks (although you might want to test it some more).

@Kestrer
Copy link
Contributor Author

Kestrer commented Dec 7, 2019

I just realized that this attempts to fix the same problem as 498, but this fixes the actual problem instead of adding a workaround.

@jyn514
Copy link
Member

jyn514 commented Dec 7, 2019

I just realized that this attempts to fix the same problem as 498, but this fixes the actual problem instead of adding a workaround.

That's ok, I like solutions without JavaScript better :)

@jyn514
Copy link
Member

jyn514 commented Dec 7, 2019

r? @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

Fixing it this way would bring back the issue that when going to an anchor, the anchor will be under the top navbar and therefore invisible.

@Kestrer
Copy link
Contributor Author

Kestrer commented Dec 9, 2019

@GuillaumeGomez We could add the following JavaScript that gets the best of both worlds (although this is very hacky):

const links = Array.from(document.getElementsByTagName("a"))
    .filter(link => link.href.includes("#"));
for link of links {
    link.addEventListener("click", _ =>
        requestAnimationFrame(_ => scrollBy(0, -50))
    );
}
if (location.hash) {
    scrollBy(0, -50);
}

This means that people with JS disabled would still get the above mentioned issue, and it's also bad practice in general to use JS for hacks like this, but I think there aren't many better alternatives.

@GuillaumeGomez
Copy link
Member

That's worse from my point of view... However I know that we have another thing going on which will have as side-effect to fix this issue.

@jyn514
Copy link
Member

jyn514 commented Feb 2, 2020

Closing in favor of #579

@jyn514 jyn514 closed this Feb 2, 2020
@Kestrer
Copy link
Contributor Author

Kestrer commented Feb 6, 2020

@GuillaumeGomez I still think that my solution (with the JS) is the best solution to the problem - #579 doesn't actually work for me. I (and probably many other Rust users) use a keyboard-based browser, and on most webpages I can use my regular keyboard shortcuts to navigate the main content, but not docs.rs as it is position: fixed. Maybe I should file an issue with the browser instead, but also from a CSS-philosophy and web accessibility point of view, having the main content be position: fixed is not good practice. Moreover on some browsers auto-focusing the content gives an ugly yellow border around it.

Sorry about bringing this issue back, but I just really want to use my keyboard shortcuts again!

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

Successfully merging this pull request may close these issues.

3 participants