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: new CSS approach to navbar-covers-target-anchor problem #318

Merged
merged 2 commits into from
Feb 26, 2021

Conversation

drammock
Copy link
Collaborator

closes #315

This attempts to use JavaScript instead of CSS to handle the problem of page anchors being hidden under the navbar. It's not perfect, but I'm hoping someone here with better webdev / javascript skills than mine can spot ways to improve it, and in any event at least it gets rid of the horrible handling of inline targets described in #315.

Known shortcomings:

  • clicking an href that targets an anchor, scrolling away from it, than clicking another href that targets the same anchor seems to always fail (target anchor ends up under the navbar). This is easy to test on demo.html#targets because there are several hrefs that all target that anchor. Probably due to the JS getting triggered by hashchange --- in this situation the hash isn't changing --- maybe there's a better way to trigger it that I'm not aware of?
  • clicking back and forth between various footnotes and their backreference links, sometimes they end up aligned just perfectly and other times not (they're always close, at least); I can't figure out if this is non-deterministic or if I just can't spot the pattern.

PS: I'm also no yarn / node expert so I may have messed up the precommit hook / hashed assets thing... LMK if it looks wrong and any tips on how to fix it.

@jorisvandenbossche
Copy link
Member

@drammock Thanks for the PR! WIll try to give this a look in the weekend

@choldgraf
Copy link
Collaborator

This tweet seemed like a promising option. @drammock wanna see if this fixed the problem?

https://mobile.twitter.com/JoshWComeau/status/1332015868725891076

@drammock drammock changed the title JS approach to navbar-covers-target-anchor problem new CSS approach to navbar-covers-target-anchor problem Feb 25, 2021
@drammock
Copy link
Collaborator Author

@choldgraf Thanks! I wasn't able to get scroll-margin-top on the targeted anchors to work properly, but it eventually led me to scroll-padding-top which can be set once on the root and it magically fixes everything!

@choldgraf
Copy link
Collaborator

amazing! this LGTM. @jorisvandenbossche ?

@jorisvandenbossche
Copy link
Member

CSS magic .. ;-)
Wonderful, from testing this indeed seems to work!

@jorisvandenbossche jorisvandenbossche changed the title new CSS approach to navbar-covers-target-anchor problem FIX: new CSS approach to navbar-covers-target-anchor problem Feb 26, 2021
@jorisvandenbossche jorisvandenbossche merged commit 93b5115 into pydata:master Feb 26, 2021
@jorisvandenbossche
Copy link
Member

Thanks both!

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.

setting display: block on inline targets breaks things badly
3 participants