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

Add CSS scroll-margin-top to headings which contain link targets. #1077

Merged
merged 2 commits into from
Oct 26, 2019

Conversation

mattheww
Copy link
Contributor

This addresses #1040 "Menu bar covers section header on anchor links".

I haven't attempted to make the new margin not apply in cases where the top menu bar won't be visible.

I've tested it on desktop Firefox and Chromium.

The second commit adds a CSS variable for the menu-bar height, to make it less likely that the scroll-margin-top value will become outdated if someone changes the design.

This means when the link is followed, the page scrolls in such a way as to
leave space for the fixed menu bar.

Fixes rust-lang#1040
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks! This has really been annoying me.

Unfortunately it doesn't seem to work on Safari. Initially I thought it was because it still uses the old name of scroll-snap-margin, but using that didn't help. I was also wondering if maybe it was because the scroll-snap-type is none for the container, but nothing I did seemed to get it to work.

I also went down the rabbit hole of looking at alternatives (1, 2, 3, 4), but none of them seem to work very well. Why do these things have to be so difficult?

Anyway, I'm fine with this until someone can come up with a better solution. I'm also fine with the padding even when the menu bar isn't visible (I kinda prefer a little padding).

@ehuss ehuss merged commit e5f77aa into rust-lang:master Oct 26, 2019
Ruin0x11 pushed a commit to Ruin0x11/mdBook that referenced this pull request Aug 30, 2020
Add CSS `scroll-margin-top` to headings which contain link targets.
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.

2 participants