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

anchor links dont work in desktop safari #268

Closed
iamstarkov opened this issue Jan 31, 2023 · 8 comments
Closed

anchor links dont work in desktop safari #268

iamstarkov opened this issue Jan 31, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@iamstarkov
Copy link

iamstarkov commented Jan 31, 2023

What browser are you using?

Safari on macOS

Other browser name

No response

Describe the bug

when you open or click on https://docs.renovatebot.com/configuration-options/#updateinternaldeps in desktop safari page opens but doesn't scroll to the "update internal deps" heading/title like it does in chrome
Screenshot 2023-01-31 at 13 20 22

Steps to reproduce

  1. Open https://docs.renovatebot.com/configuration-options/#updateinternaldeps
  2. Observe that scroll posisiton is on top of the page, and not being scrolled to the relevant header

Additional context

Console is empty of errors in Safari and anchor links not working not limited to just this one anchor link, im sure none of the left sidebar anchor links work

@iamstarkov iamstarkov added the bug Something isn't working label Jan 31, 2023
@HonkingGoose
Copy link
Collaborator

Thank you for reporting the problem. ❤️

It sounds like this upstream issue is related:

The Material for MkDocs maintainer has this to say about the upstream issue: 1

So, a heads up: I'm aware of this issue, but solving it turns out to be very complex. I'm going to take this as an opportunity to refactor instant loading, which definitely needs to be simplified, also solving the issue at hand. However, I need to find enough time to do this, so I currently can't put an estimate on when this will be fixed.

In the meantime, disabling instant loading is a good workaround.

Here's the relevant config in our repository:

# Use instant loading for internal links
# https://squidfunk.github.io/mkdocs-material/setup/setting-up-navigation/?h=instant#instant-loading
- navigation.instant

Instant loading is the killer feature of Material for MkDocs, and it does speed up clicking around in the docs a lot. So I don't really want to disable instant loading. I'll let the maintainers weigh the pros and cons, and let them decide. 😄

Footnotes

  1. https://github.com/squidfunk/mkdocs-material/issues/3797#issuecomment-1107816067

@viceice
Copy link
Member

viceice commented Jan 31, 2023

i see wrong scrolling from search regularly on Android mobile chrome. it's gotten better with latest release.

that page is probably too big and should be split into smaller pages

@HonkingGoose
Copy link
Collaborator

Hi @iamstarkov! 👋

Can you try to reproduce the problem again? Material for MkDocs has gotten some fixes, so maybe the bug is fixed now.

If the new patches didn't fix the bug, then I'd like to test things by:

  1. Disabling the navigation.instant feature for a short while
  2. Let the bug reporter test Safari on macOS again
  3. Based on the feedback: decide if we want to disable the instant loading, or keep it and accept that Safari on macOS is a bit broken

@HonkingGoose
Copy link
Collaborator

Looks like version 9.1.1 of Material for MkDocs 1 fixes this:

All we have to do now is wait until we reach the stabilityDays threshold for Renovate to automatically update us. Or a maintainer can manually approve the update earlier (if they want).

Footnotes

  1. https://github.com/squidfunk/mkdocs-material/releases/tag/9.1.1

@viceice
Copy link
Member

viceice commented Mar 6, 2023

@iamstarkov can you verify that this is working now?

@HonkingGoose
Copy link
Collaborator

@viceice Our production deployment is still using the broken version. The fixed version is awaiting the stabilityDays before it auto-merges, but you can speed things up manually, if you want.

@viceice
Copy link
Member

viceice commented Mar 6, 2023

@HonkingGoose it's now manually merged.

@iamstarkov
Copy link
Author

@viceice it works now indeed!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants