sphinx_rtd_theme icon indicating copy to clipboard operation
sphinx_rtd_theme copied to clipboard

Table handling for sphinx-extensions (incl. proposal)

Open danwos opened this issue 4 years ago • 4 comments

Problem

The rtd theme wrapps all tables in a div with class wy-table-responsive via javascript. Unfortunately some html-tables in sphinx-projects are not coming from "normal" rst-tables, but are created by sphinx-extensions, which provide their own table setup/manipulation.

An example is the Sphinx-Needs extension, and there especially the directives need and needtable. See also bug report #767 from the past.

Here a table comparison for needtable in the themes alabaster and rtd: image

The first table is the important one, which gets handled by the JS-lib DataTables. With rtd, head and body are separated and the columns are not aligned.

Reproducible Project

You can take a look into the documentation on rtd. I have activated a branch which is using the rtd theme:

needtable docs with alabaster needtable docs with rtd

Sphinx-Needs doc sources

Problem source

As I'm the maintainer of Sphinx-Needs, I tried to provide my own css-configuration, which contains rules for RTD only. This works in some cases, but not in all.

Problem is also that rtd changes the DOM by inserting a div-wrapper with class wy-table-responsive. Which may confuse other JS-code.

Proposal / Fix

For sure rtd-theme can not handle all the exceptions needed by other extensions in its code. But maybe there is a way to exclude certain table-configurations from the default rtd table-handling.

theme.js is already doing this for classes like citation or footnote. See: https://github.com/readthedocs/sphinx_rtd_theme/blob/3532ffc89bbf89ef7d2ac62d65b3683f588a9673/src/theme.js#L102

Maybe we can extend this line by a specific class-name .rtd_exclude, which can be set by sphinx-extensions to avoid the rtd-specific handling for these tables:

$("table.docutils:not(.field-list,.footnote,.citation,.rtd_exclude)")
            .wrap("<div class='wy-table-responsive'></div>");

There may be some more positions in the code, where such an exception-handling is needed (haven't checked everything yet).

Would this be a proper solution? I would be happy to create a PR, but first please give me your feedback :)

danwos avatar Jul 21 '21 07:07 danwos

@agjohnson or @Blendify or @nienn: Any thoughts on this concept?

Would we lucky to support here with an PR.

danwos avatar Aug 17 '21 08:08 danwos

I'm 👍 on adding some exclude class. It can be an useful feature for other extensions or custom settings to use and I don't see any drawbacks.

I would select a more specific class to this specific purpose thought, as the same class should not be used to any other unrelated styling. Maybe .rtd-exclude-wy-table?

nienn avatar Aug 17 '21 16:08 nienn

This issue is really getting critical for us. The fix is available from @danwos. Any chance of merging this soon? We are working with a fork of RTD now because of this.

twodrops avatar Oct 27 '21 07:10 twodrops

I will say that wrapping my HTML imported tables with a div with class wy-table-responsive allowed me to properly horizontally scroll long tables in Sphinx-generated HTMLs.

jimhavrilla avatar Apr 11 '22 16:04 jimhavrilla