Skip to content

Move rustdoc header out of rustdoc container (Fixes #935) #982

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

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

cynecx
Copy link
Contributor

@cynecx cynecx commented Aug 18, 2020

Closes #935.

@cynecx cynecx marked this pull request as ready for review August 18, 2020 16:15
@cynecx
Copy link
Contributor Author

cynecx commented Aug 18, 2020

Uh, this is wrong. Nvm.

@cynecx cynecx closed this Aug 18, 2020
@cynecx cynecx reopened this Aug 18, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 18, 2020

For the record:

$ git show HEAD~:templates/rustdoc/body.html | diff - templates/rustdoc/header.html 
238,270d237
< 
< <script type="text/javascript" src="/menu.js?{{ docsrs_version() | slugify }}"></script>
< <script type="text/javascript" src="/index.js?{{ docsrs_version() | slugify }}"></script>
< <script>
<       // Reset the scroll offset on browsers that don't support
<       // scroll-padding-top (Desktop & Mobile Safari):
<       const maybeFixupViewPortPosition = function() {
<         if (window.location.hash) {
<           const anchorElement = document.getElementById(window.location.hash.substr(1));
<           const navBarHeight = document.getElementsByClassName("nav-container-rustdoc")[0].offsetHeight;
<           if (anchorElement &&
<               anchorElement.getBoundingClientRect().top <= navBarHeight &&
<               Math.abs(anchorElement.getBoundingClientRect().top) >= 0) {
<             // It's just overlapped by the nav bar. This can't be a coincidence, scroll it into view:
<             requestAnimationFrame(function() {
<               scrollBy(0, -navBarHeight);
<             });
<           }
<         }
<       };
<       window.addEventListener("hashchange", maybeFixupViewPortPosition, false);
<       // Fix up the scroll position if this was a direct visit to the page
<       // (not back/forward navigation):
<       if (window.performance) {
<         const navEntries = window.performance.getEntriesByType('navigation');
<         const usedBack = navEntries.length > 0 && navEntries[0].type === 'back_forward' ||
<               (window.performance.navigation &&
<                window.performance.navigation.type == window.performance.navigation.TYPE_BACK_FORWARD);
<         if (!usedBack && window.location.hash) {
<           window.addEventListener("scroll", maybeFixupViewPortPosition, {"once": true});
<         }
<       }
< </script>

@cynecx is there a reason you added the extra script?

@cynecx
Copy link
Contributor Author

cynecx commented Aug 18, 2020

@jyn514 Uh. I didn't add anything script related. I've just moved the html from one place to a new template file? Am I missing something?

@jyn514
Copy link
Member

jyn514 commented Aug 18, 2020

Oh I misread the diff - you kept the script in body.html and didn't add it to header.html. Yeah, that seems fine.

@cynecx
Copy link
Contributor Author

cynecx commented Aug 18, 2020

@jyn514 Well, I wasn't sure whether changing the location of the script had any semantic difference, so I've just left it there :)

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind posting a screenshot of what this looks like after? My styles have been broken for quite some time. You can run the website with cargo run start-web-server on linux or docker-compose up on other platforms.

@jyn514
Copy link
Member

jyn514 commented Aug 18, 2020

🤦 I finally figured out the issue with my styles, I never ran a build with the new styles. Don't worry about getting a screenshot yourself, here's one:

image

@jyn514
Copy link
Member

jyn514 commented Aug 18, 2020

So this did fix the issue, nice :) looks good to me after nit.

@jyn514 jyn514 added A-frontend Area: Web frontend A-backend Area: Webserver backend C-bug Category: This is a bug S-waiting-on-author Status: This PR is incomplete or needs to address review comments and removed A-frontend Area: Web frontend labels Aug 18, 2020
Co-authored-by: Joshua Nelson <[email protected]>
@cynecx
Copy link
Contributor Author

cynecx commented Aug 18, 2020

@jyn514 God damn it. You beat me. I was just running docker-compose xD.

@jyn514 jyn514 merged commit 536807d into rust-lang:master Aug 18, 2020
@cynecx cynecx deleted the move-rustdoc-header branch August 18, 2020 19:33
@jyn514
Copy link
Member

jyn514 commented Aug 18, 2020

@jyn514 God damn it. You beat me. I was just running docker-compose xD.

Well, I did already have the full build cached :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend Area: Webserver backend C-bug Category: This is a bug S-waiting-on-author Status: This PR is incomplete or needs to address review comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Header bar font is different on front page and rustdoc files
2 participants