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 weird layout workflow issues on firefox #358

Merged

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented May 20, 2019

Currently, on firefox, on a big enough page, when you scroll and press 'S', the page scrolls back to the search bar but it's still hidden under the top navbar. It's because firefox considers that we're scrolling the body and not the content, which is an issue fixed by this PR.

I need to check on chrome if it doesn't break anything and works as expected.

A little screenshot:

Screenshot from 2019-05-20 16-52-57

Fixes #356

@GuillaumeGomez
Copy link
Member Author

I confirm that it works as expected on chrome as well!

@GuillaumeGomez GuillaumeGomez changed the title [WIP] Fix weird layout workflow issues on firefox Fix weird layout workflow issues on firefox May 20, 2019
@GuillaumeGomez
Copy link
Member Author

cc @QuietMisdreavus @onur

@GuillaumeGomez
Copy link
Member Author

Thanks to @onur, I was able to see that the right scrollbar wasn't where it was supposed to be. The second commit fixes this issue.

@onur
Copy link
Member

onur commented Jun 5, 2019

@GuillaumeGomez it's really a nasty hack and unfortunately didn't work. Here is the result for different screen resolutions, click them to see in full resolution:

Note: My rustc is really old therefore using an old version of css from rustdoc. I don't think its gonna make any difference but I'll try again tomorrow with a newest version of rustc.

Screenshot_2019-06-06 rand - Rust
Screenshot_2019-06-06 rand - Rust(1)
Screenshot_2019-06-06 rand - Rust(2)
Screenshot_2019-06-06 rand - Rust(3)

@GuillaumeGomez
Copy link
Member Author

Didn't think that the resolution would change that... I need to investigate further. Thanks again for the bug reports! :3

@GuillaumeGomez GuillaumeGomez force-pushed the layout-workflow-issues branch from ba4c4c5 to 1f86ee2 Compare June 6, 2019 09:08
@GuillaumeGomez
Copy link
Member Author

I found an even easier way to fix the problem: removing the limit on the top element to let sub elements handle it themselves. :)

@GuillaumeGomez
Copy link
Member Author

ping @onur

@@ -141,14 +143,13 @@ div.container {
}

div.container-rustdoc {
max-width: 1200px;
Copy link
Member

Choose a reason for hiding this comment

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

What does removing this rule do?

Copy link
Member

Choose a reason for hiding this comment

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

After talking with Guillaume on Discord, it seems like this was the thing causing the bug Onur had seen, with the scrollbar appearing next to the content rather than on the edge of the viewpoint. However, removing this also causes the "crate search" box in the docs.rs navbar to move all the way to the side:

image

Maybe we can move this rule to div.nav-container-rustdoc instead, rather than deleting it entirely? The value itself may need to be changed, since the rustdoc content is a little wider than 1200px.

@GuillaumeGomez GuillaumeGomez force-pushed the layout-workflow-issues branch from 1f86ee2 to d727476 Compare June 25, 2019 16:10
@@ -140,15 +142,15 @@ div.container {
text-align: left;
}

div.container-rustdoc {
div.nav-container-rustdoc {
Copy link
Member

Choose a reason for hiding this comment

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

When i said "move this rule to div.nav-container-rustdoc, i meant to use the existing block on line 265. By renaming the block like this, the div loses its text-align: left rule:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

If I did that, the width of the navbar would be maximum 1200px, which wouldn't be nice. I changed something else instead. :)

@GuillaumeGomez GuillaumeGomez force-pushed the layout-workflow-issues branch from d727476 to 598810a Compare June 26, 2019 08:22
@GuillaumeGomez
Copy link
Member Author

It seemed to have fix the issue (at least locally). I'll merge it for now but don't hesitate to come back to me if any other issue appears!

@GuillaumeGomez GuillaumeGomez merged commit 8b53e64 into rust-lang:master Jul 22, 2019
@GuillaumeGomez GuillaumeGomez deleted the layout-workflow-issues branch July 22, 2019 19:16
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.

Content is hidden by the top navbar
3 participants