jbms / sphinx-immaterial

Adaptation of the popular mkdocs-material material design theme to the sphinx documentation system
https://jbms.github.io/sphinx-immaterial/
Other
177 stars 28 forks source link

Resolve roles in navigation #56

Open mhostetter opened 2 years ago

mhostetter commented 2 years ago

I noticed that Sphinx roles are not applied in the navigation tab or content tab headings. Is there anything we can do about that? Is there a way to run a Sphinx role processor on the the titles or captions?

Content tabs

Here I tried to add an inline literal to a content tab heading. I also tried to add an octicon from the sphinx-design extension. Potentially the octicon did get converted to a SVG but isn't displaying as such?

    Examples
    --------
    .. md-tab-set::

        .. md-tab-item:: n = 14

            Content one...

        .. md-tab-item:: ``n = 15``

            Content two...

        .. md-tab-item:: :octicon:`graph` Graphs

            Content three...

renders as

image

HTML ```html
```

Navigation tabs

Here I tried to add an octicon to a navigation tab, but it didn't render as expected.

.. toctree::
   :caption: Getting Started
   :hidden:

   getting-started.rst

.. toctree::
   :caption: :octicon:`info` Basic Usage
   :hidden:

   basic-usage/galois-field-classes.rst
   basic-usage/compilation-modes.rst
   basic-usage/field-element-representation.rst
   basic-usage/array-creation.rst
   basic-usage/array-arithmetic.rst
   basic-usage/linear-algebra.rst
   basic-usage/poly-creation.rst
   basic-usage/poly-arithmetic.rst

.. toctree::
   :caption: Tutorials
   :hidden:

renders as

image

HTML ```html ```
jbms commented 2 years ago

@2bndy5 can probably address the content tabs better than I can.

As far as the toc, I'm not sure if the :caption: is normally supposed to contain rST or if it is intended to be plain text. If the latter we might not want to change the meaning, at least by default, to avoid an incompatibility with other sphinx themes. We might instead want a way to explicitly opt in.

Separately, though, page/section titles can contain rST and normally in sphinx that does result in styling of the sidebar toc. I intentionally converted all titles to plain text when generating the navigation because I think in many cases it is preferable not to have special styling in the toc. For example if you have a literal block in a title, I think it can look strange to have the literal styling in the toc. It is also tricky to define css rules so that things like literal styles display in a reasonable way inside the yoc. But I can see how icons could be desired and wouldn't have as many difficulties. I'm certainly open to suggestions for how to improve things.

mhostetter commented 2 years ago

I agree that literals in a TOC (eg, classes/functions in an API reference) could be ugly. I'm not so worried about the inline literal case. When discovering I couldn't inline n = 14 in the content tab, I was ok with just using normal text styling.

However, the octicons would be nice to add IMHO. I would be happy with adding an exception (or special processing) to only enable :octicon: from sphinx-design or another extension. I think those icons could add a little flair. 🎉

2bndy5 commented 2 years ago

I can try to fix the content tabs so that inline RST roles should work. There may be some roles that you shouldn't need to use in the labels for a content tab (like a ref). I'm surprised the octicon didn't work already because it does interpret the label as inline text (I know literals work in content tab labels).

If they ever get around to publishing the next release, sphinx-design will have 10000+ inline icons from the Google material icon fonts repo. I submitted this feature hoping we could use it abusively, like using an inline icon in a page's header title (that would hopefully show in the main nav menu also).

2bndy5 commented 2 years ago

I was able to get the roles parsed properly when treating the raw text as a separate paragraph. This results in a automatic <p> element that triggers builtin CSS: image

current working patch from https://github.com/jbms/sphinx-immaterial/blob/f925a444a9ca724d46cbacdf909e376ee2c68364/sphinx_immaterial/content_tabs.py#L92-L98 into ```py from docutils.statemachine import StringList # add tab label tab_label = nodes.rubric(self.arguments[0], classes=["tabbed-label"]) label_text = StringList(initlist=[self.arguments[0]]) self.state.nested_parse(label_text, self.content_offset, tab_label) self.add_name(tab_label) tab_item += tab_label ```

This new CSS rule fixed the <p> tag problem:

label.tabbed-label > p {
  margin: 0;
}
jbms commented 2 years ago

Instead of using nested_parse you should use inline_text as in md_admonition.py:

https://github.com/jbms/sphinx-immaterial/blob/3b1a1a097d2923c7cd8736eb6509a0aca7d96aba/sphinx_immaterial/md_admonition.py#L24

2bndy5 commented 2 years ago

That's what it is using now; it doesn't seem to handle parsing roles. unless I appended the children improperly somehow.

2bndy5 commented 2 years ago

I'm also looking into a way to strip the <p> element from tab label output (which better suites upstream CSS).

jbms commented 2 years ago

I do believe inline_text is intended to parse rST --- I'll try to figure out what is happening.

jbms commented 2 years ago

Actually I'm rather confused --- roles already seem to work fine with content tabs:

        .. md-tab-set::

            .. md-tab-item:: Customized content ``n = 4``
                :class: custom-tab-item-style

                This content could be styled differently from other page content.

            .. md-tab-item:: Custom CSS :math:`n = 3`

                .. literalinclude:: _static/extra_css.css
                    :language: css
                    :start-at: /* *********************** custom-tab-item-style
                    :end-before: /* ************************* inline icon stuff

content-tab-roles

2bndy5 commented 2 years ago

That's what I thought as well, but I was able to reproduce the issue (not using main branch right now though).

jbms commented 2 years ago

Maybe it depends on the sphinx or docutils version?

jbms commented 2 years ago

I was using Sphinx 4.4.0 and docutils 0.17.1

2bndy5 commented 2 years ago

I didn't know sphinx allowed anything newer than docutils v16

2bndy5 commented 2 years ago

I'm using docutils 0.17.1 with sphinx v4.4.0. I'm switching to main to see if it reproduces there.

2bndy5 commented 2 years ago

using the main branch, I was able to partially fix the issue by making sure the label's text started with literal text (not a inline interpreted text role). But notice how it still doesn't parse the octicon role: image Rather the octicon:graph acts like a cross-reference to the tab-set (I think)


Examples
--------

.. md-tab-set::

    .. md-tab-item:: n = 14

        Content one...

    .. md-tab-item:: when ``n = 15``

        Content two...

    .. md-tab-item:: A :octicon:`graph` Graphs

        Content three...
2bndy5 commented 2 years ago

The math role works also, but I think that's getting transformed at page load. image

jbms commented 2 years ago

If you add "sphinx_design" to the list of extensions in conf.py then octicon works.

There does indeed appear to be a bug though in the case that the label starts out with a special role.

2bndy5 commented 2 years ago

If you add "sphinx_design" to the list of extensions in conf.py then octicon works.

yep. just realized that as well.

I wonder if we pad the text with whitespace, then we might get the proper structure.

jbms commented 2 years ago

We shouldn't have to pad with whitespace --- I think the first docutils node is getting converted to a string, which is why we are seeing the docutils markup

2bndy5 commented 2 years ago
        textnodes, _ = self.state.inline_text(" " + self.arguments[0], self.lineno)

this works: image

jbms commented 2 years ago

The bug is actually in these two lines:

https://github.com/jbms/sphinx-immaterial/blob/3b1a1a097d2923c7cd8736eb6509a0aca7d96aba/sphinx_immaterial/content_tabs.py#L95

https://github.com/jbms/sphinx-immaterial/blob/3b1a1a097d2923c7cd8736eb6509a0aca7d96aba/sphinx_immaterial/content_tabs.py#L155

For docutils nodes that inherit from TextElement, the first argument to __init__ is the rawsource, and the second argument is the element text --- after that come the children.

Therefore, the first child is getting interpreted as text in both places.

Adding an extra "" argument fixes that.

2bndy5 commented 2 years ago

oh, I didn't notice which line you were talking about with the "", with

#L95
        tab_label = nodes.rubric(
            "", "", *textnodes, classes=["tabbed-label"]
        )

#L154-157
        label_node = content_tab_label(
            "",
            "",
            *tab_label.children,

renders as expected: image

2bndy5 commented 2 years ago

I took a peek at nav_adapt.py (the code that assembles the ToCs), and it looks like most of the code was designed around using plain text for section titles. I don't think it will be an easy 1-2 line fix.

[EDIT] I was able to hack it with raw HTML as the title's text (_TocVisitor._render() instead of _TocVisitor._render_title()), but I had to forego the <wbr> insertion tactic we implemented for wrapping long API names into multiple lines (to avoid the automatic ellipsis).

BTW, inline literals don't look that weird. I'm rather used to seeing a mix of monospace fonts with regular fonts in the side nav of other themes, namely the rtd-theme. image "Additional Samples" is 2 separate inline literals

jbms commented 2 years ago

Yeah, render_partial is the way that Sphinx uses to render snippets like this. However, when profiling documentation generation I discovered a problem with render_partial: it is actually quite expensive because it sets up a fresh docutils Publisher for every call, and renders a full document, only to extract a small portion.

In normal use Sphinx ends up calling render_partial ~5 times for every page just to render titles in a few places (e.g. for the "Prev" and "Next" page links, the ancestor pages, etc.). For my API documentation where every symbol is a separate page, I have several hundred pages, and every call to render_partial adds about 1.5 milliseconds. That doesn't seem too slow, but when you multiply that by 5 calls * 500 pages you are spending 3.5 seconds just generating a few tiny titles per page. The total build was around 60 seconds (without using parallel build), so 3.5 seconds is a pretty significant amount of time to spend on such a tiny thing. If we call render_partial for every single TOC entry per page it would be much, much slower.

I worked around that issue by monkey patching render_partial to check if the input node is just a pure text node, in which case it just renders it directly without spinning up docutils. For the TOC, we could potentially do similar checking, and handle text-only nodes specially --- we could do the <wbr> injection only in that case. Then in the general non-text-only case, we could ideally combine all of the titles together into a single document, render that document, then extract back the titles, so that we only require a single call to render_partial.

In fact I didn't even notice the render_partial issue until after fixing a larger performance issue: currently in nav_adapt.py we generate the TOC separately for each page, by first asking Sphinx to generate a complete global TOC (as docutils nodes) for all pages, then pruning it to just include what we want for the single page, and converting it for use by the HTML templates. This ends up having quadratic cost in the number of pages, since for every page we first generate a complete TOC covering all pages --- therefore with a large number of pages a large amount of time is spent just generating the TOCs.

The solution I implemented was to ask Sphinx for the TOC just once, convert it to an internal format, and then for each page quickly extract just the portion we need. One tricky thing is that normally Sphinx converts the page references to relative URLs as part of the TOC generation, but that is problematic since we will be re-using the same TOC docutils representation for all pages. That requires manually fixing up the relative URLs in nav_adapt.py.

We would have to see how we can make rendering the title as HTML fit with this optimization --- in most cases the HTML would be identical, but it is possible that the title itself could contain a link to another page, which would get rendered as a relative URL. Though maybe we can ignore that issue since it is kind of weird to have a separate link within a TOC title.

I will try to bring over these fixes as PRs today so that they aren't just piling up on my end.

2bndy5 commented 2 years ago

If a ref link is present in a toc title, then I would be surprised if clicking the link within the displayed toc will actually have an effect.

jbms commented 2 years ago

I don't think there is anything that prevents those referenced being turned into <a> elements, but yeah agreed that we don't really want them to do that. Probably we could solve that with some preprocessing of the docutils nodes before rendering them.

There is a better example than a reference node, though --- if you have a node that references an image file (e.g. an icon), then there is the same issue with a relative URL. This issue only applies if the image is referenced by URL in generated HTML, though --- if it is embedded then it wouldn't be an issue.

2bndy5 commented 1 year ago

I took a swing at letting docutils.nodes.raw get partially rendered and then only injecting <wbr> tags in the title's text data... So, we should be able to use sphinx-design's inline icons (which instantiate as a docutils.nodes.raw node) in the page titles.

@jbms Can you think of an objection to using render_partial() for raw nodes in the nav menu?

This feels hacky because it is focused solely on letting SVG glyphs be part of the title in the nav menu.

example

..  in admonitions.rst

:material-regular:`warning` Admonitions
=======================================

renders like so: image

I had to use python's html.parser.HTMLParser to extract only the text data from a title's text and inject the <wbr> appropriately. Clearly, <wbr> isn't used in such a short title, but it didn't break the word wrapping in all the other section titles/headers.

jbms commented 1 year ago

One question is whether it is always desirable to allow rST styling to apply to the navigation as well --- for example, if some portions use a literal or code role, do we want those styles (e..g font and possibly color) to also apply to the navigation, and if so, how do we ensure that the CSS rules properly interoperate with the CSS rules for navigation items.

The other question is for the specific case of icons, is it actually desired to have the icon present in the header (e.g. <hN> element), or is it just desired to have it in the navigation, as with the mkdocs-material feature: https://squidfunk.github.io/mkdocs-material/reference/#setting-the-page-icon ?

Regarding the implementation of using render_partial:

We should just test to see how expensive this is --- when generating API documentation, the total number of navigation items can be quite large, and we don't want to spend an excessive time rendering the navigation. Sphinx 5 significantly optimized render_partial by allowing the docutils publisher to be reused, but render_partial still does a lot of work, because it uses the docutils publisher interface that was really designed to process an entire document (but that is the only interface available in docutils). If it is too expensive, we could special case nodes that contain only text.

I think rather than parsing the HTML to add back in wbr, it would be better to modify the HTMLTranslator to just add wbr when translating text nodes in the first place.

Another alternative would be to use a custom HTML translator that only supports a limited set of docutils nodes, and just converts everything else to text.

2bndy5 commented 1 year ago

I'll push my changes to a test branch for experimentation/review.

My primary goal is to allow icons in the nav menu (not concerned about supporting all other roles). Yes, putting the inline icon role in the header/section title also makes it show on the page, and I'm ok with that since it is expected behavior. But if the goal is to only use icons in the nav menu (like the upstream insider feature), then we'd first have to support the metadata tags feature that was merged in from v8.5.6.

I'm on the fence about how to approach this. As an experienced Sphinx user, my first instinct is to use the header/section title. But I imagine those migrating from mkdocs might first try the metadata tags approach. From a dev standpoint, it would be more cohesive to do it like upstream does (even though that's still a premium feature).

FWIW, the inline icon in the main section of the page seems a bit strange to me because I'm just not used to seeing it. (switched to octicons/alert since last preview image) image

2bndy5 commented 1 year ago

I just realized the metadata tag approach won't work with toctree captions. Doesn't feel like a deal breaker, but it would be easy to support if using the inline icon role approach.

jbms commented 1 year ago

Another option would be to introduce a custom role for specifying the nav icons that generates a docutils node type that we will ignore when rendering the section headings but is handled when generating the nav labels.

Could also be a custom directive to specify an icon for the current section.

2bndy5 commented 1 year ago

I kinda like that idea because then users could put it in other places like paragraphs. Compared to sphinx-design, maybe we could support a CSS class (optional and separated by a semi-colon), so they could further adjust the color or animated it.

I'm picturing something like:

:md-icon:`material/document`
:md-icon:`material/document;custom-class`

Where the role arguments would be a path to the icon (required) [and custom CSS class].

2bndy5 commented 1 year ago

We may have to regex the icon in toctree captions because the directive's option accepts only text.

2bndy5 commented 1 year ago

I think rather than parsing the HTML to add back in wbr, it would be better to modify the HTMLTranslator to just add wbr when translating text nodes in the first place.

Seems to me that this is the only way to support icons in the toctree caption because Sphinx' visitor for Text nodes (inherited by title nodes) does its own filtering (based on astext() output). This idea also feels more like a refactor than a patch to nav_adapt.py. I'm not against it, but it will take me some time to write a working draft (lots going on in nav_adapt already).

mhostetter commented 1 year ago

Hey guys. Just wanted to check in on this issue. Any chance we can make this possible? I think it would add a lot of flair to the websites! ✨

As always, very appreciative of your work here!

2bndy5 commented 1 year ago

I've become focused on only allowing an icon (like our new si-icon role) into the toc. I stumped a while back on how to actually implement this though (because of the toctree directive's :caption: in which sphinx' src treats as text only).

jbms commented 1 year ago

I've become focused on only allowing an icon (like our new si-icon role) into the toc. I stumped a while back on how to actually implement this though (because of the toctree directive's :caption: in which sphinx' src treats as text only).

That is a good point --- I did not think about that.

For page titles, Sphinx by default preserves markup and this theme specifically strips it and renders them as text only, unlike other themes. However, toctree captions are indeed text only in Sphinx. I think there are a few possible solutions:

Also of note, mkdocs-material-insiders has some new features related to this, including the "typeset plugin" (https://squidfunk.github.io/mkdocs-material/reference/#built-in-typeset-plugin) which is exactly the feature requested here. While this plugin would not be relevant to Sphinx and we can't make use of the mkdocs-material-insiders code, I think it could make sense to take it into account in coming up with a more comprehensive plan for what options we want to provide in sphinx immaterial.

2bndy5 commented 1 year ago

I was aiming for option 2 a while ago, but the millennial in me would settle for option 1 (for a more instant gratification).

jbms commented 1 year ago

I was aiming for option 2 a while ago, but the millennial in me would settle for option 1 (for a more instant gratification).

Seems fine since it is a subset of the work for option 2. Option 2 should also be pretty easy to implement.

2bndy5 commented 1 year ago
  • Support this functionality only for page titles, not toctree captions.

I was able to do this by supporting roles in the section headings while maintaining the needed <wbr> injection for wrapping long words (like API names). My new attempt is on the nav-roles branch.

  • Monkey patch Sphinx's handling of toctree caption to treat it as rST rather than plain text

I tried doing this without monkey patching the toctree directive. Instead, I tried wrapping the original directive in a derivative used to overload the toctree... This seemed to fail, but I'm not sure the failure was exclusive to the wrapping approach. It'd probably better to address this in a separate issue later.

mkdocs-material-insiders has some new features related to this

I also tried implementing the icon metadata approach used in upstream to only prefix the ToC entries with an icon (using our si-icon role's mechanisms), but the html-page-context event that I used was triggered too late for the icons' SVG data to get added to the generated CSS (in env-check-consistency event). Or maybe, the env from separate threads that parse the doc srcs didn't contain the added ToC icons... It doesn't seem too difficult to solve this, I just need a better understanding of all the involved event triggers concerning inline icons' SVGs.

In a side note, I think the support for roles in section headers is a more stable approach due to the way the ToCs are extracted (seems deep-copied) from a cache when nav_adapt.py handles the html-page-context event. Not to mention, roles in section headers is how this is supported for every other Sphinx theme I've ever used.

2bndy5 commented 10 months ago

Upstream seems to have added page icons in v9.2.0 (no longer an insiders feature).