WordPress / wporg-mu-plugins

Development of the Global Header and other mu-plugins used on WordPress.org.
63 stars 29 forks source link

Header: in-page link targets are covered by header #219

Closed joyously closed 9 months ago

joyously commented 2 years ago

Now that the new header had fixed position, links to targets are hidden under it.

On a Make site, click on a sidebar link to a comment, for example, Matt's comment. Note that you have to scroll to find the start of the comment.

You can add CSS for this (use the height of the header) [id] { scroll-margin-top: 2em; }

iandunn commented 2 years ago

We do have something like that already:

https://github.com/WordPress/wporg-mu-plugins/blob/01e6ca252a72186aa783ec3551f2fa8f750b9ad2/mu-plugins/blocks/global-header-footer/postcss/common.pcss#L29

It works for me when I'm logged out, but not when logged in. Both have admin bars. It may be that the code assumes that logged-in sites don't have the admin bar, which recently changed on some sites.

joyously commented 2 years ago

That's not the same thing, since it's on html and not on [id], and in a media query.

iandunn commented 2 years ago

It's in a media query because the header isn't fixed on mobile.

https://github.com/WordPress/wporg-mu-plugins/blob/433d57f3b7f679c0585836ac81375814472bcd1e/mu-plugins/blocks/global-header-footer/postcss/header/header.pcss#L27-L28

I haven't looked into targeting html vs an ID, though, that might be part of it 👍🏻

iandunn commented 2 years ago

Disabling the logged-out-admin-bar plugin doesn't fix it. IIRC it worked fine at launch, so if it's not the html vs id issue, it could be a recent change in Gutenberg.

ryelle commented 2 years ago

I think the issue here is limited to Make sites, and there's some kind of scrollTo JS in p2 that overrides the scroll-padding-top offset that works on other sites.

For example, this anchor on developer.w.org works, but this one on another make site doesn't.

joyously commented 2 years ago

I tried both of your links and they both have the problem. The developer one was worse for me than the Make one. Clicking the first link shows me (Firefox): PHP Coding Standards Coding Standards Handbook WordPress Developer Resources-38

Scrolling up a little shows how far off it was: PHP Coding Standards Coding Standards Handbook WordPress Developer Resources-30

The second link shows me: Miscellaneous editor changes for WordPress 6 1 – Make WordPress Core-53

Scrolling up (less than the other one): Miscellaneous editor changes for WordPress 6 1 – Make WordPress Core-07

ryelle commented 2 years ago

Oh interesting, that's correct — in-page anchors work, so on Developer, if you use the side table of contents to navigate to section, it works as expected. Same with the list of links in the make dev note. But direct links to page content don't work.

adamwoodnz commented 9 months ago

This appears to be fixed now that the global header is no longer sticky. I've tested all the links in the comments on this issue.

Let me know if this is not the case.