ryan-roemer / sphinx-bootstrap-theme

Sphinx Bootstrap Theme
http://ryan-roemer.github.io/sphinx-bootstrap-theme/README.html
MIT License
586 stars 206 forks source link

Navbar hides class/function definitions when linked. #163

Open mrshannon opened 7 years ago

mrshannon commented 7 years ago

When linking to a class or function definition the navbar hides the definition. If watching closely, the page loads with the navbar covering the definition, then it scrolls to the proper location in order not to hide it, then finally scrolls back to the original location with the navbar hiding the definition again. Disabling javascript causes the page to load with the navbar covering up the definition. Therefore, it appears as though some javascript is fixing the issue and then some other javascript is undoing it.

I have tested this with no change in behavior with Google Chrome 59.0 and Firefox 54.0.

This could be related to #62 but since this only affects class/method/function definitions and is not fixed by a refresh (both in contrast to #62) I opened a separate issue.

I have found what is causing the issue. The problem is https://github.com/ryan-roemer/sphinx-bootstrap-theme/blob/master/sphinx_bootstrap_theme/bootstrap/static/bootstrap-sphinx.js_t#L54-L69

  $(window).load(function () {
    /*
     * Scroll the window to avoid the topnav bar
     * https://github.com/twbs/bootstrap/issues/1768
     */
    if ($("#navbar.navbar-fixed-top").length > 0) {
      var navHeight = $("#navbar").height(),
        shiftWindow = function() { scrollBy(0, -navHeight - 10); };

      if (location.hash) {
        setTimeout(shiftWindow, 1);
      }

      window.addEventListener("hashchange", shiftWindow);
    }
  });

is not the last run bit of code that scrolls the window. There are two that run after this. The first is in part of Sphinx and is https://github.com/sphinx-doc/sphinx/blob/master/sphinx/themes/basic/static/doctools.js_t#L184-L189

/**
   * workaround a firefox stupidity
   * see: https://bugzilla.mozilla.org/show_bug.cgi?id=645075
   */
  fixFirefoxAnchorBug : function() {
    if (document.location.hash)
      window.setTimeout(function() {
        document.location.href += '';
      }, 10);
  },

the second is in MathJax and is https://github.com/mathjax/MathJax/blob/master/unpacked/MathJax.js#L2711-L2716

      if (target) {
        while (!target.scrollIntoView) {target = target.parentNode}
        target = this.HashCheck(target);
        if (target && target.scrollIntoView)
          {setTimeout(function () {target.scrollIntoView(true)},1)}
      }

We need shiftWindow to be the last call that scrolls the window. Hopefully someone who knows more about Javascript than me can figure out a better way to handle it because the only solution I have found that reliably works is to extend the delay to 500 milliseconds.

svenevs commented 7 years ago

[redacted as I clearly misunderstood the actual problem]

svenevs commented 6 years ago

I revisited this, we can get away with putting it in {% block extrahead %}, since the parent class will (at least for now) load the other scripts such as mathjax before this. It seems to work very well, but there is one problem with it: if the page starts out with an anchor (not a hash change, but you start out on an anchor) the scrolling will not happen.

@ryan-roemer we just add it to layout.html:

{%- block extrahead %}
<meta charset='utf-8'>
<meta http-equiv='X-UA-Compatible' content='IE=edge,chrome=1'>
<meta name='viewport' content='width=device-width, initial-scale=1.0, maximum-scale=1'>
<meta name="apple-mobile-web-app-capable" content="yes">

+ <script type="text/javascript">
+   $(window).load(function () {
+     /*
+      * Scroll the window to avoid the topnav bar
+      * https://github.com/twbs/bootstrap/issues/1768
+      */
+     if ($("#navbar.navbar-fixed-top").length > 0) {
+       var navHeight = $("#navbar").height(),
+         shiftWindow = function() { scrollBy(0, -navHeight - 10); };
+ 
+       if (location.hash) {
+         setTimeout(shiftWindow, 1);
+       }
+ 
+       window.addEventListener("hashchange", shiftWindow);
+     }
+   });
+ </script>
{% endblock %}

This fixes clicking a link and the function executes after the other extensions (the problem @mrshannon is describing). If you click a link to a documented class it'll work. Refresh the page and the other scrolls we are trying to bypass will break it.

I think this is just a javascript thing, how do we make it happen on page load as well?