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

Local Navigation Bar: Update scroll offset to prevent local nav bar overlapping content #597

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Apr 4, 2024

Fixes #586. The local nav bar overlaps content when using anchor links. There is some code in the parent theme that used to work for the sticky header, but since we unstuck the header in favor of the local nav bar, this doesn't apply anymore.

I've taken the same approach here, but put it in the Local Navigation Bar's CSS, so that it will load whenever that block is used. This adds scroll-padding-top to html, to account for the local nav bar + admin bar (if it exists).

Some child themes have added scroll-margin-top to individual elements, this will add extra space until it's removed, but I don't think that needs to block rolling this fix out.

Screenshots

Developer

With scroll-margin-top on TOC headings Without scroll-margin-top
Screen Shot 2024-04-04 at 13 00 34 Screen Shot 2024-04-04 at 13 00 49

Forums, with the local scroll-margin-top disabled:

forum

Plugin FAQs

plugins

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

LGTM

Visually I think it's nice to have a little more space above the linked element, but maybe it's up to individual themes to set that. For example the Dev Resources title looks better with 20px space to the local nav so that the content is vertically aligned. Modifying the local scroll-margin-top achieves this.

developer wordpress org_block-editor_(2 1366x768)

@ryelle ryelle force-pushed the update/scroll-offset-localnav branch from 5bf921b to 0a774b8 Compare April 15, 2024 20:24
@ryelle
Copy link
Contributor Author

ryelle commented Apr 15, 2024

Yeah, I think that should be up to the individual themes. I've updated the ToC heading offset to 20px, since that's part of the mu-block, and left the regular offset to exactly the heading height. I'm going to merge this so that the plugins FAQs are offset correctly, and then the margin can be removed from wporg-support-2024.

@ryelle ryelle merged commit fa5e370 into trunk Apr 15, 2024
2 checks passed
@ryelle ryelle deleted the update/scroll-offset-localnav branch April 15, 2024 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Local Nav: Should adjust scroll positioning
2 participants