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

Extra whitespace for rendered footnotes/citations #312

Closed
N-Wouda opened this issue Jan 27, 2024 · 10 comments · Fixed by #313
Closed

Extra whitespace for rendered footnotes/citations #312

N-Wouda opened this issue Jan 27, 2024 · 10 comments · Fixed by #313

Comments

@N-Wouda
Copy link
Contributor

N-Wouda commented Jan 27, 2024

Hello! I'm not sure if this is an issue, or works as desired. In any case, sections with explicit markup (for e.g. footnotes, citations) render with an additional newline between the label and the note. This can be seen in the documentation:
image

I half hoped these to render as in the docutils examples, with the label and note on the same line:
image

Other than this super minor thing I'm very happy using sphinx-immaterial - big kudos for such great work!

@2bndy5
Copy link
Collaborator

2bndy5 commented Jan 27, 2024

Good catch. I would expect no new line between footnote back link and footnote content. This might be expected behavior for footnotes that are referenced more than once.

@2bndy5
Copy link
Collaborator

2bndy5 commented Jan 28, 2024

Looking at the generated HTML output,

docutils uses a table structure

<table>
          <tbody><tr><td colspan="2"><hr>
          <!-- <tr><td colspan="2">Footnotes: -->
          </td></tr><tr><td><a name="5"><strong>[5]</strong></a></td><td> A numerical footnote.
          Note there's no colon after the <samp>]</samp>.
          </td></tr></tbody>
</table>

mkdocs-material uses a div->ol structure

<div class="footnote">
  <hr> <ol> <li id="fn:1"> <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit.&nbsp;<a class="footnote-backref" href="#fnref:1" title="Jump back to footnote 1 in the text"></a></p> </li> <li id="fn:2"> <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla et euismod nulla. Curabitur feugiat, tortor non consequat finibus, justo purus auctor massa, nec semper lorem quam in massa.&nbsp;<a class="footnote-backref" href="#fnref:2" title="Jump back to footnote 2 in the text"></a></p> </li> </ol> 
</div>

sphinx-immaterial uses a aside->aside structure

<aside class="footnote-list brackets">
<aside class="footnote brackets" id="id6" role="doc-footnote">
<span class="label"><span class="fn-bracket">[</span><a role="doc-backlink" href="#id5">5</a><span class="fn-bracket">]</span></span>
<p>A numerical footnote. Note
there’s no colon after the <code class="docutils literal notranslate"><span class="pre">]</span></code>.</p>
</aside>
</aside>

Personally, I'd prefer the footnotes be generated as they are in mkdoc-material, but I haven't looked at sphinx-immaterial source code yet.

@N-Wouda
Copy link
Contributor Author

N-Wouda commented Jan 28, 2024

The mkdoc-material notes look beautiful!

@2bndy5
Copy link
Collaborator

2bndy5 commented Jan 28, 2024

I think we need to override the visit_footnote() and depart_footnote() from docutils' HTML writer. Sphinx doesn't seem to reimplement those methods.

    def visit_footnote(self, node):
        # No native HTML element: use <aside> with ARIA role
        # (html4css1 uses tables).
        # Wrap groups of footnotes for easier styling.
        label_style = self.settings.footnote_references  # brackets/superscript
        if not isinstance(node.previous_sibling(), type(node)):
            self.body.append(f'<aside class="footnote-list {label_style}">\n')
        self.body.append(self.starttag(node, 'aside',
                                       classes=[node.tagname, label_style],
                                       role="doc-footnote"))

    def depart_footnote(self, node):
        self.body.append('</aside>\n')
        if not isinstance(node.next_node(descend=False, siblings=True),
                          type(node)):
            self.body.append('</aside>\n')

@2bndy5
Copy link
Collaborator

2bndy5 commented Jan 31, 2024

I did some local testing and adapted the aforementioned visit/depart methods to generate mkdocs-material's div->ol->li structure. However, the blank line persisted, so I think we need to look at CSS inherited from upstream.

@jbms
Copy link
Owner

jbms commented Jan 31, 2024

I did some local testing and adapted the aforementioned visit/depart methods to generate mkdocs-material's div->ol->li structure. However, the blank line persisted, so I think we need to look at CSS inherited from upstream.

The aside element seems like a more appropriate element, but that may indeed make it more difficult to keep the styling identical.

As far as the blank line, there is some css from the basic theme that deals with it:

https://github.com/sphinx-doc/sphinx/blob/fa290049515c38e68edda7e8c17be69b8793bb84/sphinx/themes/basic/static/basic.css_t#L616

Essentially the footnote identifier is in a span element and the content is in a p element. The p element has display: block so it starts a new paragraph. To fix it we can make the span elements float left.

@2bndy5 2bndy5 changed the title Extra whitespace for sections with explicit markup Extra whitespace for rendered footnotes/citations Feb 1, 2024
@2bndy5
Copy link
Collaborator

2bndy5 commented Feb 1, 2024

I copied over the CSS from sphinx' basic theme. I altered the paragraph indent to be slightly less than basic theme's CSS.

Rendered results for #313 can be observed on RTD. I think this should resolve the problem, yes?

@2bndy5 2bndy5 added the pending release has been resolved but is pending a new release label Mar 30, 2024
@N-Wouda
Copy link
Contributor Author

N-Wouda commented Mar 30, 2024

Thanks @2bndy5 @jbms! 🎉

@2bndy5
Copy link
Collaborator

2bndy5 commented Jun 13, 2024

Released in v0.11.12

@2bndy5 2bndy5 removed the pending release has been resolved but is pending a new release label Jun 13, 2024
@N-Wouda
Copy link
Contributor Author

N-Wouda commented Jun 13, 2024

Fantastic! I'm going to pull this in immediately. Thanks again @2bndy5!

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 a pull request may close this issue.

3 participants