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

Add custom and collapsible admonition functionality #1124

Closed
choldgraf opened this issue Jan 21, 2023 · 14 comments · Fixed by #1155
Closed

Add custom and collapsible admonition functionality #1124

choldgraf opened this issue Jan 21, 2023 · 14 comments · Fixed by #1155
Labels
kind: enhancement New feature or request

Comments

@choldgraf
Copy link
Collaborator

There are two ways that people often request changing admonitions:

  1. They want to be able to define their own admonitions (e.g., "I want to be able to write .. example:: and have it re-use the tip style")
  2. They want to be able to collapse admonitions so they don't take up as much vertical space.

There are a few examples of each in the wild, but I recently discovered that the Sphinx Immaterial theme has nice support for both "custom admonitions" and for "collapsible admonitions".

I think it would be great if this theme supported the same functionality, ideally by following the exact same patterns they do so that we do not introduce too many new ways for people to do things. (note that these patterns are also both based on mkdocs-material so this particular API / syntax / etc is already supported in many places.

I opened an issue asking if they'd be interested in turning this into a standalone extension, but I also think it would be easy to just copy their admonitions module, give them credit, and implement it in this theme.

@choldgraf
Copy link
Collaborator Author

choldgraf commented Jan 21, 2023

If we want to do this, I wonder if this would be possible to use with our new HTML translatormixin stuff @12rambau ? It's a slightly different way of doing it than immaterial does, but seems more in-line with our setup here.

@12rambau
Copy link
Collaborator

I think we could, but we can also override the Admonition directive entirely (maybe easier). That being said isn't the collapsible admonition already part of sphinx-design ? I'm already using it here: https://docs.sepal.io/en/latest/

.. note:: 
    :class: dropdown

    toto

Also I would advocate in favor of a small extension, that would be beneficial to any sphinx theme.

@2bndy5
Copy link

2bndy5 commented Jan 22, 2023

isn't the collapsible admonition already part of sphinx-design ?

In regards to sphinx-immaterial theme (which is a port of the mkdocs-material theme), the details/summary elements are styled under the assumption that they are indeed admonitions. This restraint is actually born from the mkdocs' plugin that implements admonitions; support for details/summary elements was patched into that implementation later. So, in sphinx-immaterial, we could not use CSS specific to details/summary elements that weren't used for admonitions because it resulted in visual bugs due to conflicting CSS, and altering our theme's CSS would deviate too much from the mkdocs-material theme's CSS (which we inherit).

Consequently, this led to monkeypatching sphinxcontrib-details-directive (which doesn't respect the :class: option properly) because it didn't ship with any custom CSS like sphinx-design. Eventually, we abandoned that approach because it used sphinx post transforms (which can be costly during build time) and had limited sphinx builder support...

@2bndy5
Copy link

2bndy5 commented Jan 22, 2023

.. note::
    :class: dropdown

@12rambau This is a clever approach that isn't explicitly mentioned in sphinx-design docs. I feel like it was sheer coincidence that this worked for the admonitions without visual bugs. Luckily, the dev cycle for sphinx-design is sparsely periodic, so there's little risk that future changes to sphinx-design CSS would break this approach.

EDIT: This uses sphinx-togglebutton, not sphinx-design.

@choldgraf
Copy link
Collaborator Author

choldgraf commented Jan 22, 2023

A few quick thoughts:

How their implementation works (I think)

It looks like they accomplish this by defining their own admonition directive that allows for extra options like :collapsible:

https://github.com/jbms/sphinx-immaterial/blob/18bc88342a5eb95aaa286801f6b14885d7383750/sphinx_immaterial/custom_admonitions.py#L153C7-L173

and then they override the Sphinx admonition python class that is returned somewhere around here:

https://github.com/jbms/sphinx-immaterial/blob/18bc88342a5eb95aaa286801f6b14885d7383750/sphinx_immaterial/custom_admonitions.py#L225-L251

and that lets them add the extra options + monkeypatch the HTML admonition writers etc without raising errors for "unknown options" in Sphinx.

Sphinx-togglebutton

Yep - right now the support of :class: dropdown is via sphinx-togglebutton. That uses pure JS to accomplish the behavior, but IMO it is a little bit hacky and requires JS so may not work in some cases. sphinx-design does accomplish this but it requires people to use a new kind of directive rather than re-use the admonitions (which IMO is preferred). I feel like <details> was made for this so it would be cleaner. I'd also love a way to reduce "one extra very specific sphinx extensions" (the togglebutton one) if possible so that's why I'm exploring the idea of integrating it at the theme (or Sphinx) level

I think that sphinx-immaterial gets the UX pretty close to right (because mkdocs-material also does a great job of this!) so trying to think of ways we can standardize UX and behavior across themes when the right decisions are made. Upstreaming would be great1, but if need be I'm also happy to just implement it and document that we are trying to mimic sphinx-immaterial as much as possible.

Footnotes

  1. One challenge we might have with upstreaming this is that admonitions are defined in docutils and the docutils maintainers can be difficult to interact with unless they already agree with you ahead of time...also their code is still written in subversion + hosted on sourceforge so it might be non-trivial to upstream an improvement.

@2bndy5
Copy link

2bndy5 commented Jan 22, 2023

Yep, you nailed it. I only have 1 note though:

monkeypatch the HTML admonition writers

This happens when the module is imported. So, technically it doesn't matter if the users actually utilize the feature or not (we have options to disable the default overridden admonitions); the monkeypatch is still applied. That's why I wrote the the monkeypatch to still behave as originally intended when no custom-added options are used.


A few less commented dev tribulations:

  1. Titles for the Sphinx builtin admonitions are specially handled in the visit_admonition() to accommodate for translating the hardcoded titles ("Note", "Tip", etc). Thus, the unusual extra parameter name = None in the visit_admonition() signature.
  2. The todo admonition required an explicit target for the todo-list directive to reference them.
  3. A big problem I had was making sure I didn't break napoleon usage of admonitions (which only supports the Sphinx builtin admonitions and does not allow for directive options). This is why the idea for a :title: option was born. If we made a directive argument optional for admonitions that originally didn't expect/accept arguments, then it broke the napoleon style docstrings that use admonitions because having a blank line between the block start and the block content goes against the fundamental purpose of the Napoleon docstring style (human readability).

PS - Your footnote is the reason I didn't bother suggesting the admonition directive's required argument could be made optional.

@drammock
Copy link
Collaborator

regarding the collapsible admonitions: sphinx-design (which we already depend on) has "dropdowns" with built-in support for the standard semantic color names: https://sphinx-design.readthedocs.io/en/pydata-theme/dropdowns.html#dropdown-options

An example from MNE-Python docs (near the bottom of the page):

https://mne.tools/stable/install/check_installation.html

Done like this:

.. dropdown:: If you get an error...
    :color: danger
    :icon: alert-fill

    Content goes here

re: defining their own admonitions, I can see the value there, the "legacy" example in the sphinx-immaterial docs is convincing. But I'd consider this low priority (vs, say, sphinx 6 compatibility), since it's already possible in plain sphinx to write

.. admonition:: custom title
   :class: whatever

    Content goes here

and then write a little custom CSS to style the whatever class.

@2bndy5
Copy link

2bndy5 commented Jan 24, 2023

Sometimes, forcing the user to learn CSS may be a bit demanding.

@drammock
Copy link
Collaborator

Sometimes, forcing the user to learn CSS may be a bit demanding.

like I said, the "legacy" example in your docs is convincing as a use case. I'm on board that it would be nice to provide this. What I'm arguing is that this should be back-burner for now; it doesn't seem as urgent as some other things currently on our plate.

@2bndy5
Copy link

2bndy5 commented Jan 25, 2023

I agree with your priority preference. Thankfully, for us the sphinx v6 support was a mere matter of typing. Heads up about favicon & logo template vars getting renamed with _url suffix. EDIT: you already addressed #1094

@12rambau
Copy link
Collaborator

12rambau commented Feb 4, 2023

I worked around the requirements set-up by @choldgraf and my conclusion is that we should do nothing but a documentation update if people are really wanting to do it. everything needed is already in the wild:

collapsible

add sphinx-togglebutton and follow their documentation and magic, this

.. note:: 
   :class: dropdown

   Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum sit amet augue vitae metus egestas fermentum. Nam hendrerit in purus in scelerisque. Sed auctor neque lacus, at porttitor lectus ornare vitae. Vestibulum pretium vel turpis a placerat. Nullam mattis mi dapibus, malesuada mauris ac, luctus lectus. Morbi ac tincidunt velit. Quisque porta leo egestas ex bibendum, in dignissim dui fringilla. Nulla facilisi. Ut non mollis urna. Mauris felis ante, malesuada et tellus nec, accumsan condimentum eros. Fusce commodo placerat elit, in bibendum massa. Donec quam tellus, lacinia a libero non, viverra luctus risus.

becomes this:
Capture d’écran 2023-02-04 à 11 47 42

change title

to change the title, as explained in Sphinx doc, use the standard admonition:

.. admonition:: Aknowledgement
   :class: dropdown

   Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum sit amet augue vitae metus egestas fermentum. Nam hendrerit in purus in scelerisque. Sed auctor neque lacus, at porttitor lectus ornare vitae. Vestibulum pretium vel turpis a placerat. Nullam mattis mi dapibus, malesuada mauris ac, luctus lectus. Morbi ac tincidunt velit. Quisque porta leo egestas ex bibendum, in dignissim dui fringilla. Nulla facilisi. Ut non mollis urna. Mauris felis ante, malesuada et tellus nec, accumsan condimentum eros. Fusce commodo placerat elit, in bibendum massa. Donec quam tellus, lacinia a libero non, viverra luctus risus.

to get this:

Capture d’écran 2023-02-04 à 11 50 17

change the color and the title

all th coloring and iconing of the admonitions is piloted from their classes so add the admonition classname you want to mimic:

.. admonition:: Aknowledgement
   :class: dropdown warning

   Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum sit amet augue vitae metus egestas fermentum. Nam hendrerit in purus in scelerisque. Sed auctor neque lacus, at porttitor lectus ornare vitae. Vestibulum pretium vel turpis a placerat. Nullam mattis mi dapibus, malesuada mauris ac, luctus lectus. Morbi ac tincidunt velit. Quisque porta leo egestas ex bibendum, in dignissim dui fringilla. Nulla facilisi. Ut non mollis urna. Mauris felis ante, malesuada et tellus nec, accumsan condimentum eros. Fusce commodo placerat elit, in bibendum massa. Donec quam tellus, lacinia a libero non, viverra luctus risus.

to get this:

Capture d’écran 2023-02-04 à 11 52 12

change title, color and icon

In short you want to customize everything. You are a very advanced user, don't take me for a fool you know where the custom.css file is so please go there and add this vibrant olive color from your dreams and let's be crazy copy/paste the unicode of this incredible balance logo that you found on fontawesome website:

div.admonition.toto {border-color: olive;}
div.admonition.toto > .admonition-title:before {background-color: olive;}
div.admonition.toto > .admonition-title:after {color: olive; content: "\f24e"}

this create a new .toto class that you can use:

 .. admonition:: Aknowledgement
   :class: dropdown toto 

   Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum sit amet augue vitae metus egestas fermentum. Nam hendrerit in purus in scelerisque. Sed auctor neque lacus, at porttitor lectus ornare vitae. Vestibulum pretium vel turpis a placerat. Nullam mattis mi dapibus, malesuada mauris ac, luctus lectus. Morbi ac tincidunt velit. Quisque porta leo egestas ex bibendum, in dignissim dui fringilla. Nulla facilisi. Ut non mollis urna. Mauris felis ante, malesuada et tellus nec, accumsan condimentum eros. Fusce commodo placerat elit, in bibendum massa. Donec quam tellus, lacinia a libero non, viverra luctus risus.

tadaaam:

Capture d’écran 2023-02-04 à 11 55 29

theming

If you want to change the color of the icon, and its value and makes it different for the light and dark theme, set up some css variables. be aware that you go in poor design territory but you can do something like:

html[data-theme=dark]{
    --color-crazy: pink;
    --icon-color-crazy: blue;
    --icon-crazy: "\f6be"; /* fa-cat */
    
}
html[data-theme=light]{
    --color-crazy: blue;
    --icon-color-crazy: pink;
    --icon-crazy: "\f6d3"; /* fa-dog */
}

div.admonition.crazy {border-color: var(--color-crazy);}
div.admonition.crazy > .admonition-title:before {background-color:var(--color-crazy);}
div.admonition.crazy > .admonition-title:after {
    color: var(--icon-color-crazy); 
    content: var(--icon-crazy);
}

and light will look different from dark:
Capture d’écran 2023-02-04 à 12 33 02

Capture d’écran 2023-02-04 à 12 33 19

conclusion

to fix or not fix ?

@drammock
Copy link
Collaborator

drammock commented Feb 4, 2023

👍🏻 for a docs-only solution to this. Maybe we need a new docs page/section called "extending the theme" and this would have a part called "adding custom admonitions". Providing people a copy-pastable CSS example will go a long way I think

@choldgraf
Copy link
Collaborator Author

I'm +1 on a docs solution as well. I do wish that the UX around admonitions would improve in sphinx, but I guess that is not our problem to solve :⁠-⁠\

I still think it's nuts that I can't do

.. note:: My note title

   My note body 

😭

@12rambau
Copy link
Collaborator

12rambau commented Feb 5, 2023

agreed but we'll need to wait for sphinx 7 or that ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants