pydata-sphinx-theme icon indicating copy to clipboard operation
pydata-sphinx-theme copied to clipboard

Avoid replacing classes for Sphinx blocks

Open choldgraf opened this issue 5 years ago • 4 comments

In another project we are running into issues because readthedocs doesn't consistently apply the pydata bootstrap theme's "re-classing" of blocks in order to map onto the bootstrap docs (e.g. making admonition become alert like here: https://github.com/pandas-dev/pydata-bootstrap-sphinx-theme/blob/master/pandas_sphinx_theme/bootstrap_html_translator.py#L44).

It got me thinking, manually re-classing a bunch of Sphinx objects may not be the most robust solution. If people have CSS customizations etc, they'd probably break if the user changed themes, and any extensions that depend on the basic Sphinx classes wouldn't work because the would have been replaced.

Is there any other way we can easily apply default bootstrap themeing to Sphinx classes? Maybe this is one way to do it: https://www.sitepoint.com/sass-semantically-extend-bootstrap/

choldgraf avatar Feb 20 '20 05:02 choldgraf

I don't really understand why you got a difference between building the docs on circle ci or on readthedocs (both are using this theme, right?). That should mean it is not using the custom HTMLTranslator.

jorisvandenbossche avatar Feb 20 '20 23:02 jorisvandenbossche

Hmm, maybe it is because their build format is not "html", but "readthedocs" (sphinx-build -b readthedocs), but I don't directly find in their source how this is defined.

Now, the alerts can probably be solved differently as well. But in general, I think this custom translator class is one of the ways that gives some control over the html that sphinx generates (for cases that are otherwise not easy to solve with only css and the classes that sphinx adds to the html; eg when you need to add additional classes to be able to style something).

jorisvandenbossche avatar Feb 20 '20 23:02 jorisvandenbossche

ooh that's an interesting point...I didn't realize that they used a different translator...

choldgraf avatar Feb 21 '20 03:02 choldgraf

But we should still make sure the theme works on readthedocs, though. That's pretty important

So I suppose this "readthedocs" build target is coming from https://github.com/readthedocs/readthedocs-sphinx-ext

In https://github.com/readthedocs/readthedocs-sphinx-ext/blob/master/readthedocs_ext/readthedocs.py, they are registering their builders. And it seems that the "readthedocs" builder is a subclass from the StandaloneHTMLBuilder (which is the default "html" builder in sphinx). So not directly sure why that would not use the registered translator from our theme.

jorisvandenbossche avatar Feb 21 '20 08:02 jorisvandenbossche

In our current implementation, we overwrite the readthedoc builder https://github.com/pydata/pydata-sphinx-theme/blob/f4ef6381207b1b69cce57fe1ccb582a2cc13b955/src/pydata_sphinx_theme/init.py#L910

Can we consider this as resolved ?

12rambau avatar Oct 04 '22 11:10 12rambau

I think the proposal in this issue is to stop overwriting the builder with our own translator, and instead to make style modifications that we want purely using CSS. Basically the goal is to reduce the complexity of our implementation by not doing clever things with over-riding Sphinx.

choldgraf avatar Oct 05 '22 08:10 choldgraf