Fix anchor tag padding on link

[hoetmaaiers]

This was a difficult one. I though am glad the solution remained clean and simple.

I was able to narrow down the bug defined in #147 to defining .section with overflow: auto; to allow scrolling large tables and other horizontally wide elements. This breaks the height computation of the .section element.

After tinkering around, and trying out different solution paths, I wrapped ;the "scrolling" content inside an extra wrapper. This allows the padding-top on body to be replaced with a calculated height.

[hoetmaaiers]

Any remarks or feedback on this fix PR?

EDIT: after looking into this again the CircleCI build seems not to be what it should. TBC

[choldgraf]

it is a little hard for me to figure out what changes here are directly related to the anchor link because it seems there are many lines changed that are just whitespace updates etc...could we keep things like whitespace changes to a different PR so this is easier to review?

[jorisvandenbossche]

I suppose the problem here might be that the parent layout (https://github.com/sphinx-doc/sphinx/blob/3.x/sphinx/themes/basic/layout.html) puts this footer block not inside the content block. So that way, it might not be properly overriding the parent footer block.

Not sure how this works exactly with inheritance and blocks (didn't directly see anything about it at https://jinja.palletsprojects.com/en/2.11.x/templates/#template-inheritance)

[hoetmaaiers]

The problem is harder then I thought (since I am struggling for a while with it), but a neat solution is possible...

I have a solution in mind (and tested), which would obsolete this PR but requires the addition of a new div to the section structure.

Current html structure:

<div class="section" id="giant-tables">
  <h4>Giant Tables ...</h4>
  <table class="table">
    ...
  </table>
</div>

Requested html structure:

<div class="section" id="giant-tables">
  <h4>Giant Tables ...</h4>
  <div class="section-content">
    <table class="table">
      ...
    </table>
  </div>  
</div>

This we could simply define .section-content{ overflow: scroll; } which makes large sections scrollable and keeps anchor links to function as expected.

Is it possible to add the section content element to the rendered output @jorisvandenbossche, @choldgraf ?

[jorisvandenbossche]

@hoetmaaiers Is that needed specifically for tables?
I am a bit hesitant to start changing the output of sphinx for the content too much, but we actually already modify the output for table: https://github.com/pandas-dev/pydata-sphinx-theme/blob/4e28e6e8d0f19ea9a9bd281e5c225d8218ffe08d/pydata_sphinx_theme/bootstrap_html_translator.py#L74

So something like this:

--- a/pydata_sphinx_theme/bootstrap_html_translator.py
+++ b/pydata_sphinx_theme/bootstrap_html_translator.py
@@ -75,6 +75,7 @@ class BootstrapHTML5Translator(HTML5Translator):
         # type: (nodes.Element) -> None
         # copy of sphinx source to *not* add 'docutils' and 'align-default' classes
         # but add 'table' class
+        self.body.append(self.starttag(node, "div", CLASS="section-content"))
         self.generate_targets_for_table(node)
 
         self._table_row_index = 0
@@ -85,3 +86,7 @@ class BootstrapHTML5Translator(HTML5Translator):
         #     classes.append('align-%s' % node['align'])
         tag = self.starttag(node, "table", CLASS=" ".join(classes))
         self.body.append(tag)
+
+    def depart_table(self, node):
+        super().depart_table(node)
+        self.body.append('</div>\n')

does add an extra div around the tables.

For my understanding: how is the overflow of tables related to the offset in anchor links?

[choldgraf]

Is there a reason that the original solution you had shown in #147 doesn't work here? Just a note that we do still have that code in our CSS: https://github.com/pandas-dev/pydata-sphinx-theme/blob/master/src/scss/_base.scss#L52

It works well for me in the sphinx-book-theme, so not sure why it isn't working here...

[hoetmaaiers]

Yikes, I missed the answers from above, I am sorry.

The solution as shown in #147 works, but it is conflict with the smaller width solution to allow overflowing main content to have a scroll option.

Scrolling the full body is possible without any extra work (but not the desired solution as @jorisvandenbossche also gave feedback on this).

To allow scrolling on only the overflowing .secton a simple css overflow: auto add scroll options to a single section. The problem with overflow: auto is that the ":before heading elements offset solution" breaks. With overflow auto the height of the before element is not taken into account for the whole section height.

To make it more clear, I think a short call with some video sharing could clarify it much better.